Author: markt Date: Wed May 16 14:53:21 2018 New Revision: 1831726 URL: http://svn.apache.org/viewvc?rev=1831726&view=rev Log: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=62343 Make CORS filter defaults more secure. This is the fix for CVE-2018-8014.
Modified: tomcat/trunk/java/org/apache/catalina/filters/CorsFilter.java tomcat/trunk/java/org/apache/catalina/filters/LocalStrings.properties tomcat/trunk/test/org/apache/catalina/filters/TestCorsFilter.java tomcat/trunk/test/org/apache/catalina/filters/TesterFilterConfigs.java tomcat/trunk/webapps/docs/changelog.xml Modified: tomcat/trunk/java/org/apache/catalina/filters/CorsFilter.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/filters/CorsFilter.java?rev=1831726&r1=1831725&r2=1831726&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/filters/CorsFilter.java (original) +++ tomcat/trunk/java/org/apache/catalina/filters/CorsFilter.java Wed May 16 14:53:21 2018 @@ -256,17 +256,14 @@ public class CorsFilter extends GenericF // Section 6.1.3 // Add a single Access-Control-Allow-Origin header. - if (anyOriginAllowed && !supportsCredentials) { - // If resource doesn't support credentials and if any origin is - // allowed - // to make CORS request, return header with '*'. + if (anyOriginAllowed) { + // If any origin is allowed, return header with '*'. response.addHeader( CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN, "*"); } else { - // If the resource supports credentials add a single - // Access-Control-Allow-Origin header, with the value of the Origin - // header as value. + // Add a single Access-Control-Allow-Origin header, with the value + // of the Origin header as value. response.addHeader( CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN, origin); @@ -764,6 +761,10 @@ public class CorsFilter extends GenericF // For any value other then 'true' this will be false. this.supportsCredentials = Boolean.parseBoolean(supportsCredentials); + if (this.supportsCredentials && this.anyOriginAllowed) { + throw new ServletException(sm.getString("corsFilter.invalidSupportsCredentials")); + } + try { if (!preflightMaxAge.isEmpty()) { this.preflightMaxAge = Long.parseLong(preflightMaxAge); @@ -1073,7 +1074,7 @@ public class CorsFilter extends GenericF /** * By default, all origins are allowed to make requests. */ - public static final String DEFAULT_ALLOWED_ORIGINS = "*"; + public static final String DEFAULT_ALLOWED_ORIGINS = ""; /** * By default, following methods are supported: GET, POST, HEAD and OPTIONS. @@ -1089,7 +1090,7 @@ public class CorsFilter extends GenericF /** * By default, support credentials is turned on. */ - public static final String DEFAULT_SUPPORTS_CREDENTIALS = "true"; + public static final String DEFAULT_SUPPORTS_CREDENTIALS = "false"; /** * By default, following headers are supported: Modified: tomcat/trunk/java/org/apache/catalina/filters/LocalStrings.properties URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/filters/LocalStrings.properties?rev=1831726&r1=1831725&r2=1831726&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/filters/LocalStrings.properties (original) +++ tomcat/trunk/java/org/apache/catalina/filters/LocalStrings.properties Wed May 16 14:53:21 2018 @@ -14,6 +14,8 @@ # limitations under the License. addDefaultCharset.unsupportedCharset=Specified character set [{0}] is not supported + +corsFilter.invalidSupportsCredentials=It is not allowed to configure supportsCredentials=[true] when allowedOrigins=[*] corsFilter.invalidPreflightMaxAge=Unable to parse preflightMaxAge corsFilter.nullRequest=HttpServletRequest object is null corsFilter.nullRequestType=CORSRequestType object is null Modified: tomcat/trunk/test/org/apache/catalina/filters/TestCorsFilter.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/filters/TestCorsFilter.java?rev=1831726&r1=1831725&r2=1831726&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/catalina/filters/TestCorsFilter.java (original) +++ tomcat/trunk/test/org/apache/catalina/filters/TestCorsFilter.java Wed May 16 14:53:21 2018 @@ -55,8 +55,7 @@ public class TestCorsFilter { corsFilter.doFilter(request, response, filterChain); Assert.assertTrue(response.getHeader( - CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals( - "https://www.apache.org")); + CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals("*")); Assert.assertTrue(((Boolean) request.getAttribute( CorsFilter.HTTP_REQUEST_ATTRIBUTE_IS_CORS_REQUEST)).booleanValue()); Assert.assertTrue(request.getAttribute( @@ -88,8 +87,7 @@ public class TestCorsFilter { corsFilter.doFilter(request, response, filterChain); Assert.assertTrue(response.getHeader( - CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals( - "https://www.apache.org")); + CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals("*")); Assert.assertTrue(((Boolean) request.getAttribute( CorsFilter.HTTP_REQUEST_ATTRIBUTE_IS_CORS_REQUEST)).booleanValue()); Assert.assertTrue(request.getAttribute( @@ -120,8 +118,7 @@ public class TestCorsFilter { corsFilter.doFilter(request, response, filterChain); Assert.assertTrue(response.getHeader( - CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals( - "https://www.apache.org")); + CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals("*")); Assert.assertTrue(((Boolean) request.getAttribute( CorsFilter.HTTP_REQUEST_ATTRIBUTE_IS_CORS_REQUEST)).booleanValue()); Assert.assertTrue(request.getAttribute( @@ -166,41 +163,15 @@ public class TestCorsFilter { } /* - * Tests the presence of the origin (and not '*') in the response, when - * supports credentials is enabled alongwith any origin, '*'. + * Tests the that supports credentials may not be enabled with any origin, + * '*'. * - * @throws IOException * @throws ServletException */ - @Test - public void testDoFilterSimpleAnyOriginAndSupportsCredentials() - throws IOException, ServletException { - TesterHttpServletRequest request = new TesterHttpServletRequest(); - request.setHeader(CorsFilter.REQUEST_HEADER_ORIGIN, - TesterFilterConfigs.HTTPS_WWW_APACHE_ORG); - request.setMethod("GET"); - TesterHttpServletResponse response = new TesterHttpServletResponse(); - + @Test(expected=ServletException.class) + public void testDoFilterSimpleAnyOriginAndSupportsCredentials() throws ServletException { CorsFilter corsFilter = new CorsFilter(); - corsFilter.init(TesterFilterConfigs - .getFilterConfigAnyOriginAndSupportsCredentials()); - corsFilter.doFilter(request, response, filterChain); - - Assert.assertTrue(response.getHeader( - CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals( - TesterFilterConfigs.HTTPS_WWW_APACHE_ORG)); - Assert.assertTrue(response.getHeader( - CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_CREDENTIALS) - .equals( - "true")); - Assert.assertTrue(((Boolean) request.getAttribute( - CorsFilter.HTTP_REQUEST_ATTRIBUTE_IS_CORS_REQUEST)).booleanValue()); - Assert.assertTrue(request.getAttribute( - CorsFilter.HTTP_REQUEST_ATTRIBUTE_ORIGIN).equals( - TesterFilterConfigs.HTTPS_WWW_APACHE_ORG)); - Assert.assertTrue(request.getAttribute( - CorsFilter.HTTP_REQUEST_ATTRIBUTE_REQUEST_TYPE).equals( - CorsFilter.CORSRequestType.SIMPLE.name().toLowerCase(Locale.ENGLISH))); + corsFilter.init(TesterFilterConfigs.getFilterConfigAnyOriginAndSupportsCredentials()); } /* @@ -261,8 +232,7 @@ public class TestCorsFilter { corsFilter.doFilter(request, response, filterChain); Assert.assertTrue(response.getHeader( - CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals( - "https://www.apache.org")); + CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals("*")); Assert.assertTrue(response.getHeader( CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_EXPOSE_HEADERS) .equals(TesterFilterConfigs.EXPOSED_HEADERS)); @@ -727,9 +697,8 @@ public class TestCorsFilter { }); corsFilter.doFilter(request, response, filterChain); - Assert.assertTrue(response.getHeader( - CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals( - "https://www.apache.org")); + Assert.assertNull(response.getHeader( + CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN)); Assert.assertTrue(((Boolean) request.getAttribute( CorsFilter.HTTP_REQUEST_ATTRIBUTE_IS_CORS_REQUEST)).booleanValue()); Assert.assertTrue(request.getAttribute( @@ -1412,7 +1381,7 @@ public class TestCorsFilter { Assert.assertTrue(corsFilter.getAllowedOrigins().size() == 0); Assert.assertTrue(corsFilter.isAnyOriginAllowed()); Assert.assertTrue(corsFilter.getExposedHeaders().size() == 0); - Assert.assertTrue(corsFilter.isSupportsCredentials()); + Assert.assertFalse(corsFilter.isSupportsCredentials()); Assert.assertTrue(corsFilter.getPreflightMaxAge() == 1800); } @@ -1448,9 +1417,9 @@ public class TestCorsFilter { Assert.assertTrue(corsFilter.getAllowedHttpHeaders().size() == 6); Assert.assertTrue(corsFilter.getAllowedHttpMethods().size() == 4); Assert.assertTrue(corsFilter.getAllowedOrigins().size() == 0); - Assert.assertTrue(corsFilter.isAnyOriginAllowed()); + Assert.assertFalse(corsFilter.isAnyOriginAllowed()); Assert.assertTrue(corsFilter.getExposedHeaders().size() == 0); - Assert.assertTrue(corsFilter.isSupportsCredentials()); + Assert.assertFalse(corsFilter.isSupportsCredentials()); Assert.assertTrue(corsFilter.getPreflightMaxAge() == 1800); } @@ -1554,8 +1523,7 @@ public class TestCorsFilter { corsFilter.doFilter(request, response, filterChain); Assert.assertTrue(response.getHeader( - CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals( - "https://www.apache.org")); + CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN).equals("*")); Assert.assertNull(request .getAttribute(CorsFilter.HTTP_REQUEST_ATTRIBUTE_IS_CORS_REQUEST)); Assert.assertNull(request Modified: tomcat/trunk/test/org/apache/catalina/filters/TesterFilterConfigs.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/filters/TesterFilterConfigs.java?rev=1831726&r1=1831725&r2=1831726&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/catalina/filters/TesterFilterConfigs.java (original) +++ tomcat/trunk/test/org/apache/catalina/filters/TesterFilterConfigs.java Wed May 16 14:53:21 2018 @@ -36,12 +36,13 @@ public class TesterFilterConfigs { public static final TesterServletContext mockServletContext = new TesterServletContext(); + // Default config for the test is to allow any origin public static FilterConfig getDefaultFilterConfig() { final String allowedHttpHeaders = CorsFilter.DEFAULT_ALLOWED_HTTP_HEADERS; final String allowedHttpMethods = CorsFilter.DEFAULT_ALLOWED_HTTP_METHODS; - final String allowedOrigins = CorsFilter.DEFAULT_ALLOWED_ORIGINS; + final String allowedOrigins = ANY_ORIGIN; final String exposedHeaders = CorsFilter.DEFAULT_EXPOSED_HEADERS; final String supportCredentials = CorsFilter.DEFAULT_SUPPORTS_CREDENTIALS; @@ -59,7 +60,7 @@ public class TesterFilterConfigs { CorsFilter.DEFAULT_ALLOWED_HTTP_HEADERS; final String allowedHttpMethods = CorsFilter.DEFAULT_ALLOWED_HTTP_METHODS + ",PUT"; - final String allowedOrigins = CorsFilter.DEFAULT_ALLOWED_ORIGINS; + final String allowedOrigins = ANY_ORIGIN; final String exposedHeaders = CorsFilter.DEFAULT_EXPOSED_HEADERS; final String supportCredentials = "true"; final String preflightMaxAge = @@ -77,7 +78,7 @@ public class TesterFilterConfigs { CorsFilter.DEFAULT_ALLOWED_HTTP_HEADERS; final String allowedHttpMethods = CorsFilter.DEFAULT_ALLOWED_HTTP_METHODS + ",PUT"; - final String allowedOrigins = CorsFilter.DEFAULT_ALLOWED_ORIGINS; + final String allowedOrigins = ANY_ORIGIN; final String exposedHeaders = CorsFilter.DEFAULT_EXPOSED_HEADERS; final String supportCredentials = "false"; final String preflightMaxAge = @@ -131,7 +132,7 @@ public class TesterFilterConfigs { CorsFilter.DEFAULT_ALLOWED_HTTP_HEADERS; final String allowedHttpMethods = CorsFilter.DEFAULT_ALLOWED_HTTP_METHODS; - final String allowedOrigins = CorsFilter.DEFAULT_ALLOWED_ORIGINS; + final String allowedOrigins = ANY_ORIGIN; final String exposedHeaders = EXPOSED_HEADERS; final String supportCredentials = CorsFilter.DEFAULT_SUPPORTS_CREDENTIALS; @@ -240,7 +241,7 @@ public class TesterFilterConfigs { CorsFilter.DEFAULT_ALLOWED_HTTP_HEADERS; final String allowedHttpMethods = CorsFilter.DEFAULT_ALLOWED_HTTP_METHODS; - final String allowedOrigins = CorsFilter.DEFAULT_ALLOWED_ORIGINS; + final String allowedOrigins = ANY_ORIGIN; final String exposedHeaders = CorsFilter.DEFAULT_EXPOSED_HEADERS; final String supportCredentials = CorsFilter.DEFAULT_SUPPORTS_CREDENTIALS; Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1831726&r1=1831725&r2=1831726&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Wed May 16 14:53:21 2018 @@ -92,6 +92,10 @@ usually performed during web application stop if that stop is triggered by a JVM shutdown. (markt) </fix> + <fix> + <bug>62343</bug>: Make CORS filter defaults more secure. This is the fix + for CVE-2018-8014. (markt) + </fix> </changelog> </subsection> <subsection name="Coyote"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org