Skip to content
This repository was archived by the owner on Dec 4, 2018. It is now read-only.

Commit 2c9d843

Browse files
markt-asfmarkt-asf
committed
Make CORS filter defaults more secure. This is the fix for CVE-2018-8014. git-svn-id: https://svn.apache.org/repos/asf/tomcat/tc8.0.x/trunk@1831729 13f79535-47bb-0310-9956-ffa450edef68
1 parent 0337a42 commit 2c9d843

5 files changed

Lines changed: 37 additions & 61 deletions

File tree

‎java/org/apache/catalina/filters/CorsFilter.java‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -278,17 +278,14 @@ protected void handleSimpleCORS(final HttpServletRequest request,
278278

279279
// Section 6.1.3
280280
// Add a single Access-Control-Allow-Origin header.
281-
if (anyOriginAllowed && !supportsCredentials) {
282-
// If resource doesn't support credentials and if any origin is
283-
// allowed
284-
// to make CORS request, return header with '*'.
281+
if (anyOriginAllowed) {
282+
// If any origin is allowed, return header with '*'.
285283
response.addHeader(
286284
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN,
287285
"*");
288286
} else {
289-
// If the resource supports credentials add a single
290-
// Access-Control-Allow-Origin header, with the value of the Origin
291-
// header as value.
287+
// Add a single Access-Control-Allow-Origin header, with the value
288+
// of the Origin header as value.
292289
response.addHeader(
293290
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN,
294291
origin);
@@ -804,6 +801,10 @@ private void parseAndStore(final String allowedOrigins,
804801
// For any value other then 'true' this will be false.
805802
this.supportsCredentials = Boolean.parseBoolean(supportsCredentials);
806803

804+
if (this.supportsCredentials && this.anyOriginAllowed) {
805+
throw new ServletException(sm.getString("corsFilter.invalidSupportsCredentials"));
806+
}
807+
807808
try {
808809
if (!preflightMaxAge.isEmpty()) {
809810
this.preflightMaxAge = Long.parseLong(preflightMaxAge);
@@ -1163,7 +1164,7 @@ protected enum CORSRequestType {
11631164
/**
11641165
* By default, all origins are allowed to make requests.
11651166
*/
1166-
public static final String DEFAULT_ALLOWED_ORIGINS = "*";
1167+
public static final String DEFAULT_ALLOWED_ORIGINS = "";
11671168

11681169
/**
11691170
* By default, following methods are supported: GET, POST, HEAD and OPTIONS.
@@ -1179,7 +1180,7 @@ protected enum CORSRequestType {
11791180
/**
11801181
* By default, support credentials is turned on.
11811182
*/
1182-
public static final String DEFAULT_SUPPORTS_CREDENTIALS = "true";
1183+
public static final String DEFAULT_SUPPORTS_CREDENTIALS = "false";
11831184

11841185
/**
11851186
* By default, following headers are supported:

‎java/org/apache/catalina/filters/LocalStrings.properties‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
# limitations under the License.
1515

1616
addDefaultCharset.unsupportedCharset=Specified character set [{0}] is not supported
17+
18+
corsFilter.invalidSupportsCredentials=It is not allowed to configure supportsCredentials=[true] when allowedOrigins=[*]
1719
corsFilter.invalidPreflightMaxAge=Unable to parse preflightMaxAge
1820
corsFilter.nullRequest=HttpServletRequest object is null
1921
corsFilter.nullRequestType=CORSRequestType object is null

‎test/org/apache/catalina/filters/TestCorsFilter.java‎

Lines changed: 15 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,7 @@ public void testDoFilterSimpleGET() throws IOException, ServletException {
5252
corsFilter.doFilter(request, response, filterChain);
5353

5454
Assert.assertTrue(response.getHeader(
55-
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals(
56-
"https://www.apache.org"));
55+
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals("*"));
5756
Assert.assertTrue(((Boolean) request.getAttribute(
5857
CorsFilter.HTTP_REQUEST_ATTRIBUTE_IS_CORS_REQUEST)).booleanValue());
5958
Assert.assertTrue(request.getAttribute(
@@ -85,8 +84,7 @@ public void testDoFilterSimplePOST() throws IOException, ServletException {
8584
corsFilter.doFilter(request, response, filterChain);
8685

8786
Assert.assertTrue(response.getHeader(
88-
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals(
89-
"https://www.apache.org"));
87+
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals("*"));
9088
Assert.assertTrue(((Boolean) request.getAttribute(
9189
CorsFilter.HTTP_REQUEST_ATTRIBUTE_IS_CORS_REQUEST)).booleanValue());
9290
Assert.assertTrue(request.getAttribute(
@@ -117,8 +115,7 @@ public void testDoFilterSimpleHEAD() throws IOException, ServletException {
117115
corsFilter.doFilter(request, response, filterChain);
118116

119117
Assert.assertTrue(response.getHeader(
120-
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals(
121-
"https://www.apache.org"));
118+
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals("*"));
122119
Assert.assertTrue(((Boolean) request.getAttribute(
123120
CorsFilter.HTTP_REQUEST_ATTRIBUTE_IS_CORS_REQUEST)).booleanValue());
124121
Assert.assertTrue(request.getAttribute(
@@ -163,41 +160,15 @@ public void testDoFilterSimpleSpecificHeader() throws IOException,
163160
}
164161

165162
/*
166-
* Tests the presence of the origin (and not '*') in the response, when
167-
* supports credentials is enabled alongwith any origin, '*'.
163+
* Tests the that supports credentials may not be enabled with any origin,
164+
* '*'.
168165
*
169-
* @throws IOException
170166
* @throws ServletException
171167
*/
172-
@Test
173-
public void testDoFilterSimpleAnyOriginAndSupportsCredentials()
174-
throws IOException, ServletException {
175-
TesterHttpServletRequest request = new TesterHttpServletRequest();
176-
request.setHeader(CorsFilter.REQUEST_HEADER_ORIGIN,
177-
TesterFilterConfigs.HTTPS_WWW_APACHE_ORG);
178-
request.setMethod("GET");
179-
TesterHttpServletResponse response = new TesterHttpServletResponse();
180-
168+
@Test(expected=ServletException.class)
169+
public void testDoFilterSimpleAnyOriginAndSupportsCredentials() throws ServletException {
181170
CorsFilter corsFilter = new CorsFilter();
182-
corsFilter.init(TesterFilterConfigs
183-
.getFilterConfigAnyOriginAndSupportsCredentials());
184-
corsFilter.doFilter(request, response, filterChain);
185-
186-
Assert.assertTrue(response.getHeader(
187-
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals(
188-
TesterFilterConfigs.HTTPS_WWW_APACHE_ORG));
189-
Assert.assertTrue(response.getHeader(
190-
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_CREDENTIALS)
191-
.equals(
192-
"true"));
193-
Assert.assertTrue(((Boolean) request.getAttribute(
194-
CorsFilter.HTTP_REQUEST_ATTRIBUTE_IS_CORS_REQUEST)).booleanValue());
195-
Assert.assertTrue(request.getAttribute(
196-
CorsFilter.HTTP_REQUEST_ATTRIBUTE_ORIGIN).equals(
197-
TesterFilterConfigs.HTTPS_WWW_APACHE_ORG));
198-
Assert.assertTrue(request.getAttribute(
199-
CorsFilter.HTTP_REQUEST_ATTRIBUTE_REQUEST_TYPE).equals(
200-
CorsFilter.CORSRequestType.SIMPLE.name().toLowerCase(Locale.ENGLISH)));
171+
corsFilter.init(TesterFilterConfigs.getFilterConfigAnyOriginAndSupportsCredentials());
201172
}
202173

203174
/*
@@ -258,8 +229,7 @@ public void testDoFilterSimpleWithExposedHeaders() throws IOException,
258229
corsFilter.doFilter(request, response, filterChain);
259230

260231
Assert.assertTrue(response.getHeader(
261-
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals(
262-
"https://www.apache.org"));
232+
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals("*"));
263233
Assert.assertTrue(response.getHeader(
264234
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_EXPOSE_HEADERS)
265235
.equals(TesterFilterConfigs.EXPOSED_HEADERS));
@@ -707,9 +677,8 @@ public void testInitDefaultFilterConfig() throws IOException,
707677
corsFilter.init(null);
708678
corsFilter.doFilter(request, response, filterChain);
709679

710-
Assert.assertTrue(response.getHeader(
711-
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals(
712-
"https://www.apache.org"));
680+
Assert.assertNull(response.getHeader(
681+
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN));
713682
Assert.assertTrue(((Boolean) request.getAttribute(
714683
CorsFilter.HTTP_REQUEST_ATTRIBUTE_IS_CORS_REQUEST)).booleanValue());
715684
Assert.assertTrue(request.getAttribute(
@@ -1392,7 +1361,7 @@ public void testWithFilterConfig() throws ServletException {
13921361
Assert.assertTrue(corsFilter.getAllowedOrigins().size() == 0);
13931362
Assert.assertTrue(corsFilter.isAnyOriginAllowed());
13941363
Assert.assertTrue(corsFilter.getExposedHeaders().size() == 0);
1395-
Assert.assertTrue(corsFilter.isSupportsCredentials());
1364+
Assert.assertFalse(corsFilter.isSupportsCredentials());
13961365
Assert.assertTrue(corsFilter.getPreflightMaxAge() == 1800);
13971366
}
13981367

@@ -1428,9 +1397,9 @@ public void testWithStringParserNull() throws ServletException {
14281397
Assert.assertTrue(corsFilter.getAllowedHttpHeaders().size() == 6);
14291398
Assert.assertTrue(corsFilter.getAllowedHttpMethods().size() == 4);
14301399
Assert.assertTrue(corsFilter.getAllowedOrigins().size() == 0);
1431-
Assert.assertTrue(corsFilter.isAnyOriginAllowed());
1400+
Assert.assertFalse(corsFilter.isAnyOriginAllowed());
14321401
Assert.assertTrue(corsFilter.getExposedHeaders().size() == 0);
1433-
Assert.assertTrue(corsFilter.isSupportsCredentials());
1402+
Assert.assertFalse(corsFilter.isSupportsCredentials());
14341403
Assert.assertTrue(corsFilter.getPreflightMaxAge() == 1800);
14351404
}
14361405

@@ -1534,8 +1503,7 @@ public void testDecorateRequestDisabled() throws IOException,
15341503
corsFilter.doFilter(request, response, filterChain);
15351504

15361505
Assert.assertTrue(response.getHeader(
1537-
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals(
1538-
"https://www.apache.org"));
1506+
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals("*"));
15391507
Assert.assertNull(request
15401508
.getAttribute(CorsFilter.HTTP_REQUEST_ATTRIBUTE_IS_CORS_REQUEST));
15411509
Assert.assertNull(request

‎test/org/apache/catalina/filters/TesterFilterConfigs.java‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,13 @@ public class TesterFilterConfigs {
3636
public static final TesterServletContext mockServletContext =
3737
new TesterServletContext();
3838

39+
// Default config for the test is to allow any origin
3940
public static FilterConfig getDefaultFilterConfig() {
4041
final String allowedHttpHeaders =
4142
CorsFilter.DEFAULT_ALLOWED_HTTP_HEADERS;
4243
final String allowedHttpMethods =
4344
CorsFilter.DEFAULT_ALLOWED_HTTP_METHODS;
44-
final String allowedOrigins = CorsFilter.DEFAULT_ALLOWED_ORIGINS;
45+
final String allowedOrigins = ANY_ORIGIN;
4546
final String exposedHeaders = CorsFilter.DEFAULT_EXPOSED_HEADERS;
4647
final String supportCredentials =
4748
CorsFilter.DEFAULT_SUPPORTS_CREDENTIALS;
@@ -59,7 +60,7 @@ public static FilterConfig getFilterConfigAnyOriginAndSupportsCredentials() {
5960
CorsFilter.DEFAULT_ALLOWED_HTTP_HEADERS;
6061
final String allowedHttpMethods =
6162
CorsFilter.DEFAULT_ALLOWED_HTTP_METHODS + ",PUT";
62-
final String allowedOrigins = CorsFilter.DEFAULT_ALLOWED_ORIGINS;
63+
final String allowedOrigins = ANY_ORIGIN;
6364
final String exposedHeaders = CorsFilter.DEFAULT_EXPOSED_HEADERS;
6465
final String supportCredentials = "true";
6566
final String preflightMaxAge =
@@ -77,7 +78,7 @@ public static FilterConfig getFilterConfigAnyOriginAndSupportsCredentials() {
7778
CorsFilter.DEFAULT_ALLOWED_HTTP_HEADERS;
7879
final String allowedHttpMethods =
7980
CorsFilter.DEFAULT_ALLOWED_HTTP_METHODS + ",PUT";
80-
final String allowedOrigins = CorsFilter.DEFAULT_ALLOWED_ORIGINS;
81+
final String allowedOrigins = ANY_ORIGIN;
8182
final String exposedHeaders = CorsFilter.DEFAULT_EXPOSED_HEADERS;
8283
final String supportCredentials = "false";
8384
final String preflightMaxAge =
@@ -131,7 +132,7 @@ public static FilterConfig getFilterConfigWithExposedHeaders() {
131132
CorsFilter.DEFAULT_ALLOWED_HTTP_HEADERS;
132133
final String allowedHttpMethods =
133134
CorsFilter.DEFAULT_ALLOWED_HTTP_METHODS;
134-
final String allowedOrigins = CorsFilter.DEFAULT_ALLOWED_ORIGINS;
135+
final String allowedOrigins = ANY_ORIGIN;
135136
final String exposedHeaders = EXPOSED_HEADERS;
136137
final String supportCredentials =
137138
CorsFilter.DEFAULT_SUPPORTS_CREDENTIALS;
@@ -240,7 +241,7 @@ public static FilterConfig getFilterConfigDecorateRequestDisabled() {
240241
CorsFilter.DEFAULT_ALLOWED_HTTP_HEADERS;
241242
final String allowedHttpMethods =
242243
CorsFilter.DEFAULT_ALLOWED_HTTP_METHODS;
243-
final String allowedOrigins = CorsFilter.DEFAULT_ALLOWED_ORIGINS;
244+
final String allowedOrigins = ANY_ORIGIN;
244245
final String exposedHeaders = CorsFilter.DEFAULT_EXPOSED_HEADERS;
245246
final String supportCredentials =
246247
CorsFilter.DEFAULT_SUPPORTS_CREDENTIALS;

‎webapps/docs/changelog.xml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,10 @@
8989
<bug>60490</bug>: Various formatting and layout improvements for the
9090
<code>ErrorReportValve</code>. Patch provided by Michael Osipov. (markt)
9191
</fix>
92+
<fix>
93+
<bug>62343</bug>: Make CORS filter defaults more secure. This is the fix
94+
for CVE-2018-8014. (markt)
95+
</fix>
9296
</changelog>
9397
</subsection>
9498
<subsection name="Coyote">

0 commit comments

Comments
 (0)