Skip to content

Commit 60f596a

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.5.x/trunk@1831728 13f79535-47bb-0310-9956-ffa450edef68
1 parent 188d54e commit 60f596a

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
@@ -267,17 +267,14 @@ protected void handleSimpleCORS(final HttpServletRequest request,
267267

268268
// Section 6.1.3
269269
// Add a single Access-Control-Allow-Origin header.
270-
if (anyOriginAllowed && !supportsCredentials) {
271-
// If resource doesn't support credentials and if any origin is
272-
// allowed
273-
// to make CORS request, return header with '*'.
270+
if (anyOriginAllowed) {
271+
// If any origin is allowed, return header with '*'.
274272
response.addHeader(
275273
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN,
276274
"*");
277275
} else {
278-
// If the resource supports credentials add a single
279-
// Access-Control-Allow-Origin header, with the value of the Origin
280-
// header as value.
276+
// Add a single Access-Control-Allow-Origin header, with the value
277+
// of the Origin header as value.
281278
response.addHeader(
282279
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN,
283280
origin);
@@ -781,6 +778,10 @@ private void parseAndStore(final String allowedOrigins,
781778
// For any value other then 'true' this will be false.
782779
this.supportsCredentials = Boolean.parseBoolean(supportsCredentials);
783780

781+
if (this.supportsCredentials && this.anyOriginAllowed) {
782+
throw new ServletException(sm.getString("corsFilter.invalidSupportsCredentials"));
783+
}
784+
784785
try {
785786
if (!preflightMaxAge.isEmpty()) {
786787
this.preflightMaxAge = Long.parseLong(preflightMaxAge);
@@ -1090,7 +1091,7 @@ protected enum CORSRequestType {
10901091
/**
10911092
* By default, all origins are allowed to make requests.
10921093
*/
1093-
public static final String DEFAULT_ALLOWED_ORIGINS = "*";
1094+
public static final String DEFAULT_ALLOWED_ORIGINS = "";
10941095

10951096
/**
10961097
* By default, following methods are supported: GET, POST, HEAD and OPTIONS.
@@ -1106,7 +1107,7 @@ protected enum CORSRequestType {
11061107
/**
11071108
* By default, support credentials is turned on.
11081109
*/
1109-
public static final String DEFAULT_SUPPORTS_CREDENTIALS = "true";
1110+
public static final String DEFAULT_SUPPORTS_CREDENTIALS = "false";
11101111

11111112
/**
11121113
* 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
@@ -55,8 +55,7 @@ public void testDoFilterSimpleGET() throws IOException, ServletException {
5555
corsFilter.doFilter(request, response, filterChain);
5656

5757
Assert.assertTrue(response.getHeader(
58-
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals(
59-
"https://www.apache.org"));
58+
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals("*"));
6059
Assert.assertTrue(((Boolean) request.getAttribute(
6160
CorsFilter.HTTP_REQUEST_ATTRIBUTE_IS_CORS_REQUEST)).booleanValue());
6261
Assert.assertTrue(request.getAttribute(
@@ -88,8 +87,7 @@ public void testDoFilterSimplePOST() throws IOException, ServletException {
8887
corsFilter.doFilter(request, response, filterChain);
8988

9089
Assert.assertTrue(response.getHeader(
91-
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals(
92-
"https://www.apache.org"));
90+
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals("*"));
9391
Assert.assertTrue(((Boolean) request.getAttribute(
9492
CorsFilter.HTTP_REQUEST_ATTRIBUTE_IS_CORS_REQUEST)).booleanValue());
9593
Assert.assertTrue(request.getAttribute(
@@ -120,8 +118,7 @@ public void testDoFilterSimpleHEAD() throws IOException, ServletException {
120118
corsFilter.doFilter(request, response, filterChain);
121119

122120
Assert.assertTrue(response.getHeader(
123-
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals(
124-
"https://www.apache.org"));
121+
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals("*"));
125122
Assert.assertTrue(((Boolean) request.getAttribute(
126123
CorsFilter.HTTP_REQUEST_ATTRIBUTE_IS_CORS_REQUEST)).booleanValue());
127124
Assert.assertTrue(request.getAttribute(
@@ -166,41 +163,15 @@ public void testDoFilterSimpleSpecificHeader() throws IOException,
166163
}
167164

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

206177
/*
@@ -261,8 +232,7 @@ public void testDoFilterSimpleWithExposedHeaders() throws IOException,
261232
corsFilter.doFilter(request, response, filterChain);
262233

263234
Assert.assertTrue(response.getHeader(
264-
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals(
265-
"https://www.apache.org"));
235+
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals("*"));
266236
Assert.assertTrue(response.getHeader(
267237
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_EXPOSE_HEADERS)
268238
.equals(TesterFilterConfigs.EXPOSED_HEADERS));
@@ -727,9 +697,8 @@ public String getFilterName() {
727697
});
728698
corsFilter.doFilter(request, response, filterChain);
729699

730-
Assert.assertTrue(response.getHeader(
731-
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals(
732-
"https://www.apache.org"));
700+
Assert.assertNull(response.getHeader(
701+
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN));
733702
Assert.assertTrue(((Boolean) request.getAttribute(
734703
CorsFilter.HTTP_REQUEST_ATTRIBUTE_IS_CORS_REQUEST)).booleanValue());
735704
Assert.assertTrue(request.getAttribute(
@@ -1412,7 +1381,7 @@ public void testWithFilterConfig() throws ServletException {
14121381
Assert.assertTrue(corsFilter.getAllowedOrigins().size() == 0);
14131382
Assert.assertTrue(corsFilter.isAnyOriginAllowed());
14141383
Assert.assertTrue(corsFilter.getExposedHeaders().size() == 0);
1415-
Assert.assertTrue(corsFilter.isSupportsCredentials());
1384+
Assert.assertFalse(corsFilter.isSupportsCredentials());
14161385
Assert.assertTrue(corsFilter.getPreflightMaxAge() == 1800);
14171386
}
14181387

@@ -1448,9 +1417,9 @@ public void testWithStringParserNull() throws ServletException {
14481417
Assert.assertTrue(corsFilter.getAllowedHttpHeaders().size() == 6);
14491418
Assert.assertTrue(corsFilter.getAllowedHttpMethods().size() == 4);
14501419
Assert.assertTrue(corsFilter.getAllowedOrigins().size() == 0);
1451-
Assert.assertTrue(corsFilter.isAnyOriginAllowed());
1420+
Assert.assertFalse(corsFilter.isAnyOriginAllowed());
14521421
Assert.assertTrue(corsFilter.getExposedHeaders().size() == 0);
1453-
Assert.assertTrue(corsFilter.isSupportsCredentials());
1422+
Assert.assertFalse(corsFilter.isSupportsCredentials());
14541423
Assert.assertTrue(corsFilter.getPreflightMaxAge() == 1800);
14551424
}
14561425

@@ -1554,8 +1523,7 @@ public void testDecorateRequestDisabled() throws IOException,
15541523
corsFilter.doFilter(request, response, filterChain);
15551524

15561525
Assert.assertTrue(response.getHeader(
1557-
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals(
1558-
"https://www.apache.org"));
1526+
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals("*"));
15591527
Assert.assertNull(request
15601528
.getAttribute(CorsFilter.HTTP_REQUEST_ATTRIBUTE_IS_CORS_REQUEST));
15611529
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
@@ -85,6 +85,10 @@
8585
<bug>50019</bug>: Add support for <code>&lt;lookup-name&gt;</code>.
8686
Based on a patch by Gurkan Erdogdu. (markt)
8787
</fix>
88+
<fix>
89+
<bug>62343</bug>: Make CORS filter defaults more secure. This is the fix
90+
for CVE-2018-8014. (markt)
91+
</fix>
8892
</changelog>
8993
</subsection>
9094
<subsection name="Coyote">

0 commit comments

Comments
 (0)