Moderate severityNVD Advisory· Published Mar 16, 2018· Updated Sep 16, 2024
CVE-2018-1199
CVE-2018-1199
Description
Spring Security (Spring Security 4.1.x before 4.1.5, 4.2.x before 4.2.4, and 5.0.x before 5.0.1; and Spring Framework 4.3.x before 4.3.14 and 5.0.x before 5.0.3) does not consider URL path parameters when processing security constraints. By adding a URL path parameter with special encodings, an attacker may be able to bypass a security constraint. The root cause of this issue is a lack of clarity regarding the handling of path parameters in the Servlet Specification. Some Servlet containers include path parameters in the value returned for getPathInfo() and some do not. Spring Security uses the value returned by getPathInfo() as part of the process of mapping requests to security constraints. In this particular attack, different character encodings used in path parameters allows secured Spring MVC static resource URLs to be bypassed.
AI Insight
LLM-synthesized narrative grounded in this CVE's description and references.
Spring Security and Spring Framework fail to handle URL path parameters consistently, allowing attackers to bypass security constraints and access protected resources.
Vulnerability
Spring Security versions 4.1.x before 4.1.5, 4.2.x before 4.2.4, and 5.0.x before 5.0.1, along with Spring Framework versions 4.3.x before 4.3.14 and 5.0.x before 5.0.3, do not properly consider URL path parameters when evaluating security constraints [1][2][3][4]. The root cause is a lack of clarity in the Servlet Specification regarding path parameter handling in getPathInfo(). Some Servlet containers include path parameters in the getPathInfo() value, while others do not; Spring Security uses this value for request-to-security-constraint mapping, leading to inconsistencies [4].
Exploitation
An attacker can craft a URL with special encodings in path parameters (e.g., using semicolon ; or encoded characters like %3b) to bypass security constraints [1][2][3][4]. The attack exploits the different interpretations of path parameters between the Servlet container and Spring Security, specifically targeting secured Spring MVC static resource URLs. No authentication or special privileges are required; the attacker only needs to send a specially crafted HTTP request [4].
Impact
Successful exploitation allows an attacker to bypass security constraints, gaining unauthorized access to protected static resources. This can lead to information disclosure of files that should have been restricted by Spring Security, potentially exposing sensitive configuration or data [4].
Mitigation
Users should upgrade to Spring Security 4.1.5, 4.2.4, or 5.0.1 (or later) and Spring Framework 4.3.14 or 5.0.3 (or later) where a StrictHttpFirewall was introduced to reject requests containing forbidden characters (e.g., semicolons and encoded path separators) [1][2][3][4]. For environments where upgrades are not immediately possible, administrators can apply the StrictHttpFirewall manually or restrict URL patterns to disallow path parameters. The vulnerability is not known to be listed in CISA's KEV as of publication [4].
References
AI Insight generated on May 22, 2026. Synthesized from this CVE's description and the cited reference URLs; citations are validated against the source bundle.
Affected packages
Versions sourced from the GitHub Security Advisory.
| Package | Affected versions | Patched versions |
|---|---|---|
org.springframework.security:spring-security-coreMaven | >= 4.2.0, < 4.2.4 | 4.2.4 |
org.springframework.security:spring-security-coreMaven | >= 5.0.0, < 5.0.1 | 5.0.1 |
org.springframework:spring-coreMaven | >= 4.3.0, < 4.3.14 | 4.3.14 |
org.springframework:spring-coreMaven | >= 5.0.0, < 5.0.3 | 5.0.3 |
org.springframework.security:spring-security-coreMaven | >= 4.1.0, < 4.1.5 | 4.1.5 |
Affected products
3- ghsa-coords2 versionspkg:maven/org.springframework.security/spring-security-corepkg:maven/org.springframework/spring-core
>= 4.2.0, < 4.2.4+ 1 more
- (no CPE)range: >= 4.2.0, < 4.2.4
- (no CPE)range: >= 4.3.0, < 4.3.14
- Dell EMC/Spring by Pivotalv5Range: spring Security 4.1.0 - 4.1.4, 4.2.0 - 4.2.3, 5.0.0
Patches
973a81f98d40eAllow interceptors to add existing header values
2 files changed · +39 −3
spring-web/src/main/java/org/springframework/web/client/RestTemplate.java+7 −2 modified@@ -922,6 +922,7 @@ public void doWithRequest(ClientHttpRequest httpRequest) throws IOException { Class<?> requestBodyClass = requestBody.getClass(); Type requestBodyType = (this.requestEntity instanceof RequestEntity ? ((RequestEntity<?>)this.requestEntity).getType() : requestBodyClass); + HttpHeaders httpHeaders = httpRequest.getHeaders(); HttpHeaders requestHeaders = this.requestEntity.getHeaders(); MediaType requestContentType = requestHeaders.getContentType(); for (HttpMessageConverter<?> messageConverter : getMessageConverters()) { @@ -930,7 +931,9 @@ public void doWithRequest(ClientHttpRequest httpRequest) throws IOException { (GenericHttpMessageConverter<Object>) messageConverter; if (genericConverter.canWrite(requestBodyType, requestBodyClass, requestContentType)) { if (!requestHeaders.isEmpty()) { - httpRequest.getHeaders().putAll(requestHeaders); + for (Map.Entry<String, List<String>> entry : requestHeaders.entrySet()) { + httpHeaders.put(entry.getKey(), new LinkedList<>(entry.getValue())); + } } if (logger.isDebugEnabled()) { if (requestContentType != null) { @@ -948,7 +951,9 @@ public void doWithRequest(ClientHttpRequest httpRequest) throws IOException { } else if (messageConverter.canWrite(requestBodyClass, requestContentType)) { if (!requestHeaders.isEmpty()) { - httpRequest.getHeaders().putAll(requestHeaders); + for (Map.Entry<String, List<String>> entry : requestHeaders.entrySet()) { + httpHeaders.put(entry.getKey(), new LinkedList<>(entry.getValue())); + } } if (logger.isDebugEnabled()) { if (requestContentType != null) {
spring-web/src/test/java/org/springframework/web/client/RestTemplateTests.java+32 −1 modified@@ -826,7 +826,7 @@ public void exchangeParameterizedType() throws Exception { } @Test // SPR-15066 - public void requestInterceptorCanAddExistingHeaderValue() throws Exception { + public void requestInterceptorCanAddExistingHeaderValueWithoutBody() throws Exception { ClientHttpRequestInterceptor interceptor = (request, body, execution) -> { request.getHeaders().add("MyHeader", "MyInterceptorValue"); return execution.execute(request, body); @@ -851,4 +851,35 @@ public void requestInterceptorCanAddExistingHeaderValue() throws Exception { verify(response).close(); } + @Test // SPR-15066 + public void requestInterceptorCanAddExistingHeaderValueWithBody() throws Exception { + ClientHttpRequestInterceptor interceptor = (request, body, execution) -> { + request.getHeaders().add("MyHeader", "MyInterceptorValue"); + return execution.execute(request, body); + }; + template.setInterceptors(Collections.singletonList(interceptor)); + + MediaType contentType = MediaType.TEXT_PLAIN; + given(converter.canWrite(String.class, contentType)).willReturn(true); + given(requestFactory.createRequest(new URI("http://example.com"), HttpMethod.POST)).willReturn(request); + String helloWorld = "Hello World"; + HttpHeaders requestHeaders = new HttpHeaders(); + given(request.getHeaders()).willReturn(requestHeaders); + converter.write(helloWorld, contentType, request); + given(request.execute()).willReturn(response); + given(errorHandler.hasError(response)).willReturn(false); + HttpStatus status = HttpStatus.OK; + given(response.getStatusCode()).willReturn(status); + given(response.getStatusText()).willReturn(status.getReasonPhrase()); + + HttpHeaders entityHeaders = new HttpHeaders(); + entityHeaders.setContentType(contentType); + entityHeaders.add("MyHeader", "MyEntityValue"); + HttpEntity<String> entity = new HttpEntity<>(helloWorld, entityHeaders); + template.exchange("http://example.com", HttpMethod.POST, entity, Void.class); + assertThat(requestHeaders.get("MyHeader"), contains("MyEntityValue", "MyInterceptorValue")); + + verify(response).close(); + } + }
e6e6b8f4adcaAllow interceptors to add existing header values
2 files changed · +39 −3
spring-web/src/main/java/org/springframework/web/client/RestTemplate.java+7 −2 modified@@ -852,14 +852,17 @@ public void doWithRequest(ClientHttpRequest httpRequest) throws IOException { Class<?> requestBodyClass = requestBody.getClass(); Type requestBodyType = (this.requestEntity instanceof RequestEntity ? ((RequestEntity<?>)this.requestEntity).getType() : requestBodyClass); + HttpHeaders httpHeaders = httpRequest.getHeaders(); HttpHeaders requestHeaders = this.requestEntity.getHeaders(); MediaType requestContentType = requestHeaders.getContentType(); for (HttpMessageConverter<?> messageConverter : getMessageConverters()) { if (messageConverter instanceof GenericHttpMessageConverter) { GenericHttpMessageConverter<Object> genericMessageConverter = (GenericHttpMessageConverter<Object>) messageConverter; if (genericMessageConverter.canWrite(requestBodyType, requestBodyClass, requestContentType)) { if (!requestHeaders.isEmpty()) { - httpRequest.getHeaders().putAll(requestHeaders); + for (Map.Entry<String, List<String>> entry : requestHeaders.entrySet()) { + httpHeaders.put(entry.getKey(), new LinkedList<String>(entry.getValue())); + } } if (logger.isDebugEnabled()) { if (requestContentType != null) { @@ -878,7 +881,9 @@ public void doWithRequest(ClientHttpRequest httpRequest) throws IOException { } else if (messageConverter.canWrite(requestBodyClass, requestContentType)) { if (!requestHeaders.isEmpty()) { - httpRequest.getHeaders().putAll(requestHeaders); + for (Map.Entry<String, List<String>> entry : requestHeaders.entrySet()) { + httpHeaders.put(entry.getKey(), new LinkedList<String>(entry.getValue())); + } } if (logger.isDebugEnabled()) { if (requestContentType != null) {
spring-web/src/test/java/org/springframework/web/client/RestTemplateTests.java+32 −1 modified@@ -836,7 +836,7 @@ public void exchangeParameterizedType() throws Exception { } @Test // SPR-15066 - public void requestInterceptorCanAddExistingHeaderValue() throws Exception { + public void requestInterceptorCanAddExistingHeaderValueWithoutBody() throws Exception { ClientHttpRequestInterceptor interceptor = (request, body, execution) -> { request.getHeaders().add("MyHeader", "MyInterceptorValue"); return execution.execute(request, body); @@ -861,4 +861,35 @@ public void requestInterceptorCanAddExistingHeaderValue() throws Exception { verify(response).close(); } + @Test // SPR-15066 + public void requestInterceptorCanAddExistingHeaderValueWithBody() throws Exception { + ClientHttpRequestInterceptor interceptor = (request, body, execution) -> { + request.getHeaders().add("MyHeader", "MyInterceptorValue"); + return execution.execute(request, body); + }; + template.setInterceptors(Collections.singletonList(interceptor)); + + MediaType contentType = MediaType.TEXT_PLAIN; + given(converter.canWrite(String.class, contentType)).willReturn(true); + given(requestFactory.createRequest(new URI("http://example.com"), HttpMethod.POST)).willReturn(request); + String helloWorld = "Hello World"; + HttpHeaders requestHeaders = new HttpHeaders(); + given(request.getHeaders()).willReturn(requestHeaders); + converter.write(helloWorld, contentType, request); + given(request.execute()).willReturn(response); + given(errorHandler.hasError(response)).willReturn(false); + HttpStatus status = HttpStatus.OK; + given(response.getStatusCode()).willReturn(status); + given(response.getStatusText()).willReturn(status.getReasonPhrase()); + + HttpHeaders entityHeaders = new HttpHeaders(); + entityHeaders.setContentType(contentType); + entityHeaders.add("MyHeader", "MyEntityValue"); + HttpEntity<String> entity = new HttpEntity<>(helloWorld, entityHeaders); + template.exchange("http://example.com", HttpMethod.POST, entity, Void.class); + assertThat(requestHeaders.get("MyHeader"), contains("MyEntityValue", "MyInterceptorValue")); + + verify(response).close(); + } + }
554662ebab87Allow interceptors to add existing header values
2 files changed · +33 −1
spring-web/src/main/java/org/springframework/web/client/RestTemplate.java+4 −1 modified@@ -21,6 +21,7 @@ import java.net.URI; import java.util.ArrayList; import java.util.Collections; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Set; @@ -909,7 +910,9 @@ public void doWithRequest(ClientHttpRequest httpRequest) throws IOException { HttpHeaders httpHeaders = httpRequest.getHeaders(); HttpHeaders requestHeaders = this.requestEntity.getHeaders(); if (!requestHeaders.isEmpty()) { - httpHeaders.putAll(requestHeaders); + for (Map.Entry<String, List<String>> entry : requestHeaders.entrySet()) { + httpHeaders.put(entry.getKey(), new LinkedList<>(entry.getValue())); + } } if (httpHeaders.getContentLength() < 0) { httpHeaders.setContentLength(0L);
spring-web/src/test/java/org/springframework/web/client/RestTemplateTests.java+29 −0 modified@@ -39,12 +39,15 @@ import org.springframework.http.ResponseEntity; import org.springframework.http.client.ClientHttpRequest; import org.springframework.http.client.ClientHttpRequestFactory; +import org.springframework.http.client.ClientHttpRequestInterceptor; import org.springframework.http.client.ClientHttpResponse; import org.springframework.http.converter.GenericHttpMessageConverter; import org.springframework.http.converter.HttpMessageConverter; import org.springframework.util.StreamUtils; import org.springframework.web.util.DefaultUriBuilderFactory; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.collection.IsIterableContainingInOrder.contains; import static org.junit.Assert.*; import static org.mockito.BDDMockito.*; import static org.springframework.http.HttpMethod.POST; @@ -822,4 +825,30 @@ public void exchangeParameterizedType() throws Exception { verify(response).close(); } + @Test // SPR-15066 + public void requestInterceptorCanAddExistingHeaderValue() throws Exception { + ClientHttpRequestInterceptor interceptor = (request, body, execution) -> { + request.getHeaders().add("MyHeader", "MyInterceptorValue"); + return execution.execute(request, body); + }; + template.setInterceptors(Collections.singletonList(interceptor)); + + given(requestFactory.createRequest(new URI("http://example.com"), HttpMethod.POST)).willReturn(request); + HttpHeaders requestHeaders = new HttpHeaders(); + given(request.getHeaders()).willReturn(requestHeaders); + given(request.execute()).willReturn(response); + given(errorHandler.hasError(response)).willReturn(false); + HttpStatus status = HttpStatus.OK; + given(response.getStatusCode()).willReturn(status); + given(response.getStatusText()).willReturn(status.getReasonPhrase()); + + HttpHeaders entityHeaders = new HttpHeaders(); + entityHeaders.add("MyHeader", "MyEntityValue"); + HttpEntity<Void> entity = new HttpEntity<>(null, entityHeaders); + template.exchange("http://example.com", HttpMethod.POST, entity, Void.class); + assertThat(requestHeaders.get("MyHeader"), contains("MyEntityValue", "MyInterceptorValue")); + + verify(response).close(); + } + }
65da28e4bf62Add StrictHttpFirewall
7 files changed · +610 −12
config/src/main/java/org/springframework/security/config/annotation/web/builders/WebSecurity.java+2 −2 modified@@ -47,7 +47,7 @@ import org.springframework.security.web.access.expression.DefaultWebSecurityExpressionHandler; import org.springframework.security.web.access.intercept.FilterSecurityInterceptor; import org.springframework.security.web.debug.DebugFilter; -import org.springframework.security.web.firewall.DefaultHttpFirewall; +import org.springframework.security.web.firewall.StrictHttpFirewall; import org.springframework.security.web.firewall.HttpFirewall; import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; @@ -159,7 +159,7 @@ public IgnoredRequestConfigurer ignoring() { /** * Allows customizing the {@link HttpFirewall}. The default is - * {@link DefaultHttpFirewall}. + * {@link StrictHttpFirewall}. * * @param httpFirewall the custom {@link HttpFirewall} * @return the {@link WebSecurity} for further customizations
config/src/test/java/org/springframework/security/config/FilterChainProxyConfigTests.java+4 −0 modified@@ -38,6 +38,7 @@ import org.springframework.security.web.SecurityFilterChain; import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter; import org.springframework.security.web.context.SecurityContextPersistenceFilter; +import org.springframework.security.web.firewall.DefaultHttpFirewall; import org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.AnyRequestMatcher; @@ -80,6 +81,7 @@ public void normalOperation() throws Exception { public void normalOperationWithNewConfig() throws Exception { FilterChainProxy filterChainProxy = appCtx.getBean("newFilterChainProxy", FilterChainProxy.class); + filterChainProxy.setFirewall(new DefaultHttpFirewall()); checkPathAndFilterOrder(filterChainProxy); doNormalOperation(filterChainProxy); } @@ -88,6 +90,7 @@ public void normalOperationWithNewConfig() throws Exception { public void normalOperationWithNewConfigRegex() throws Exception { FilterChainProxy filterChainProxy = appCtx.getBean("newFilterChainProxyRegex", FilterChainProxy.class); + filterChainProxy.setFirewall(new DefaultHttpFirewall()); checkPathAndFilterOrder(filterChainProxy); doNormalOperation(filterChainProxy); } @@ -96,6 +99,7 @@ public void normalOperationWithNewConfigRegex() throws Exception { public void normalOperationWithNewConfigNonNamespace() throws Exception { FilterChainProxy filterChainProxy = appCtx.getBean( "newFilterChainProxyNonNamespace", FilterChainProxy.class); + filterChainProxy.setFirewall(new DefaultHttpFirewall()); checkPathAndFilterOrder(filterChainProxy); doNormalOperation(filterChainProxy); }
itest/context/src/integration-test/java/org/springframework/security/integration/HttpPathParameterStrippingTests.java+9 −7 modified@@ -28,6 +28,7 @@ import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.web.FilterChainProxy; import org.springframework.security.web.context.HttpSessionSecurityContextRepository; +import org.springframework.security.web.firewall.RequestRejectedException; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; @@ -40,32 +41,33 @@ public class HttpPathParameterStrippingTests { @Autowired private FilterChainProxy fcp; - @Test + @Test(expected = RequestRejectedException.class) public void securedFilterChainCannotBeBypassedByAddingPathParameters() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); request.setPathInfo("/secured;x=y/admin.html"); request.setSession(createAuthenticatedSession("ROLE_USER")); MockHttpServletResponse response = new MockHttpServletResponse(); fcp.doFilter(request, response, new MockFilterChain()); - assertThat(response.getStatus()).isEqualTo(403); } - @Test + @Test(expected = RequestRejectedException.class) public void adminFilePatternCannotBeBypassedByAddingPathParameters() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); request.setServletPath("/secured/admin.html;x=user.html"); request.setSession(createAuthenticatedSession("ROLE_USER")); MockHttpServletResponse response = new MockHttpServletResponse(); fcp.doFilter(request, response, new MockFilterChain()); - assertThat(response.getStatus()).isEqualTo(403); + } - // Try with pathInfo - request = new MockHttpServletRequest(); + + @Test(expected = RequestRejectedException.class) + public void adminFilePatternCannotBeBypassedByAddingPathParametersWithPathInfo() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest(); request.setServletPath("/secured"); request.setPathInfo("/admin.html;x=user.html"); request.setSession(createAuthenticatedSession("ROLE_USER")); - response = new MockHttpServletResponse(); + MockHttpServletResponse response = new MockHttpServletResponse(); fcp.doFilter(request, response, new MockFilterChain()); assertThat(response.getStatus()).isEqualTo(403); }
web/src/main/java/org/springframework/security/web/FilterChainProxy.java+3 −3 modified@@ -19,9 +19,9 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.security.core.context.SecurityContextHolder; -import org.springframework.security.web.firewall.DefaultHttpFirewall; import org.springframework.security.web.firewall.FirewalledRequest; import org.springframework.security.web.firewall.HttpFirewall; +import org.springframework.security.web.firewall.StrictHttpFirewall; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.security.web.util.UrlUtils; import org.springframework.web.filter.DelegatingFilterProxy; @@ -96,7 +96,7 @@ * * An {@link HttpFirewall} instance is used to validate incoming requests and create a * wrapped request which provides consistent path values for matching against. See - * {@link DefaultHttpFirewall}, for more information on the type of attacks which the + * {@link StrictHttpFirewall}, for more information on the type of attacks which the * default implementation protects against. A custom implementation can be injected to * provide stricter control over the request contents or if an application needs to * support certain types of request which are rejected by default. @@ -147,7 +147,7 @@ public class FilterChainProxy extends GenericFilterBean { private FilterChainValidator filterChainValidator = new NullFilterChainValidator(); - private HttpFirewall firewall = new DefaultHttpFirewall(); + private HttpFirewall firewall = new StrictHttpFirewall(); // ~ Methods // ========================================================================================================
web/src/main/java/org/springframework/security/web/firewall/DefaultHttpFirewall.java+2 −0 modified@@ -37,8 +37,10 @@ * containers normalize the paths before performing the servlet-mapping, but * again this is not guaranteed by the servlet spec. * + * @deprecated Use {@link StrictHttpFirewall} instead * @author Luke Taylor */ +@Deprecated public class DefaultHttpFirewall implements HttpFirewall { private boolean allowUrlEncodedSlash;
web/src/main/java/org/springframework/security/web/firewall/StrictHttpFirewall.java+239 −0 added@@ -0,0 +1,239 @@ +/* + * Copyright 2012-2017 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.firewall; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +/** + * A strict implementation of {@link HttpFirewall} that rejects any suspicious requests + * with a {@link RequestRejectedException}. + * + * @author Rob Winch + * @since 5.0.1 + */ +public class StrictHttpFirewall implements HttpFirewall { + private static final String ENCODED_PERCENT = "%25"; + + private static final String PERCENT = "%"; + + private static final List<String> FORBIDDEN_ENCODED_PERIOD = Collections.unmodifiableList(Arrays.asList("%2e", "%2E")); + + private static final List<String> FORBIDDEN_SEMICOLON = Collections.unmodifiableList(Arrays.asList(";", "%3b", "%3B")); + + private static final List<String> FORBIDDEN_FORWARDSLASH = Collections.unmodifiableList(Arrays.asList("%2f", "%2F")); + + private static final List<String> FORBIDDEN_BACKSLASH = Collections.unmodifiableList(Arrays.asList("\\", "%5c", "%5C")); + + private Set<String> encodedUrlBlacklist = new HashSet<String>(); + + private Set<String> decodedUrlBlacklist = new HashSet<String>(); + + public StrictHttpFirewall() { + urlBlacklistsAddAll(FORBIDDEN_SEMICOLON); + urlBlacklistsAddAll(FORBIDDEN_FORWARDSLASH); + urlBlacklistsAddAll(FORBIDDEN_BACKSLASH); + + this.encodedUrlBlacklist.add(ENCODED_PERCENT); + this.encodedUrlBlacklist.addAll(FORBIDDEN_ENCODED_PERIOD); + this.decodedUrlBlacklist.add(PERCENT); + } + + /** + * + * @param allowSemicolon + */ + public void setAllowSemicolon(boolean allowSemicolon) { + if (allowSemicolon) { + urlBlacklistsRemoveAll(FORBIDDEN_SEMICOLON); + } else { + urlBlacklistsAddAll(FORBIDDEN_SEMICOLON); + } + } + + public void setAllowUrlEncodedSlash(boolean allowUrlEncodedSlash) { + if (allowUrlEncodedSlash) { + urlBlacklistsRemoveAll(FORBIDDEN_FORWARDSLASH); + } else { + urlBlacklistsAddAll(FORBIDDEN_FORWARDSLASH); + } + } + + public void setAllowUrlEncodedPeriod(boolean allowUrlEncodedPeriod) { + if (allowUrlEncodedPeriod) { + this.encodedUrlBlacklist.removeAll(FORBIDDEN_ENCODED_PERIOD); + } else { + this.encodedUrlBlacklist.addAll(FORBIDDEN_ENCODED_PERIOD); + } + } + + public void setAllowBackSlash(boolean allowBackSlash) { + if (allowBackSlash) { + urlBlacklistsRemoveAll(FORBIDDEN_BACKSLASH); + } else { + urlBlacklistsAddAll(FORBIDDEN_BACKSLASH); + } + } + + public void setAllowUrlEncodedPercent(boolean allowUrlEncodedPercent) { + if (allowUrlEncodedPercent) { + this.encodedUrlBlacklist.remove(ENCODED_PERCENT); + this.decodedUrlBlacklist.remove(PERCENT); + } else { + this.encodedUrlBlacklist.add(ENCODED_PERCENT); + this.decodedUrlBlacklist.add(PERCENT); + } + } + + private void urlBlacklistsAddAll(Collection<String> values) { + this.encodedUrlBlacklist.addAll(values); + this.decodedUrlBlacklist.addAll(values); + } + + private void urlBlacklistsRemoveAll(Collection<String> values) { + this.encodedUrlBlacklist.removeAll(values); + this.decodedUrlBlacklist.removeAll(values); + } + + @Override + public FirewalledRequest getFirewalledRequest(HttpServletRequest request) throws RequestRejectedException { + rejectedBlacklistedUrls(request); + + if (!isNormalized(request)) { + throw new RequestRejectedException("The request was rejected because the URL was not normalized."); + } + + String requestUri = request.getRequestURI(); + if (!containsOnlyPrintableAsciiCharacters(requestUri)) { + throw new RequestRejectedException("The requestURI was rejected because it can only contain printable ASCII characters."); + } + return new FirewalledRequest(request) { + @Override + public void reset() { + } + }; + } + + private void rejectedBlacklistedUrls(HttpServletRequest request) { + for (String forbidden : this.encodedUrlBlacklist) { + if (encodedUrlContains(request, forbidden)) { + throw new RequestRejectedException("The request was rejected because the URL contained a potentially malicious String \"" + forbidden + "\""); + } + } + for (String forbidden : this.decodedUrlBlacklist) { + if (decodedUrlContains(request, forbidden)) { + throw new RequestRejectedException("The request was rejected because the URL contained a potentially malicious String \"" + forbidden + "\""); + } + } + } + + @Override + public HttpServletResponse getFirewalledResponse(HttpServletResponse response) { + return new FirewalledResponse(response); + } + + private static boolean isNormalized(HttpServletRequest request) { + if (!isNormalized(request.getRequestURI())) { + return false; + } + if (!isNormalized(request.getContextPath())) { + return false; + } + if (!isNormalized(request.getServletPath())) { + return false; + } + if (!isNormalized(request.getPathInfo())) { + return false; + } + return true; + } + + private static boolean encodedUrlContains(HttpServletRequest request, String value) { + if (valueContains(request.getContextPath(), value)) { + return true; + } + return valueContains(request.getRequestURI(), value); + } + + private static boolean decodedUrlContains(HttpServletRequest request, String value) { + if (valueContains(request.getServletPath(), value)) { + return true; + } + if (valueContains(request.getPathInfo(), value)) { + return true; + } + return false; + } + + private static boolean containsOnlyPrintableAsciiCharacters(String uri) { + int length = uri.length(); + for (int i = 0; i < length; i++) { + char c = uri.charAt(i); + if (c < '\u0021' || '\u007e' < c) { + return false; + } + } + + return true; + } + + private static boolean valueContains(String value, String contains) { + return value != null && value.contains(contains); + } + + /** + * Checks whether a path is normalized (doesn't contain path traversal + * sequences like "./", "/../" or "/.") + * + * @param path + * the path to test + * @return true if the path doesn't contain any path-traversal character + * sequences. + */ + private static boolean isNormalized(String path) { + if (path == null) { + return true; + } + + if (path.indexOf("//") > 0) { + return false; + } + + for (int j = path.length(); j > 0;) { + int i = path.lastIndexOf('/', j - 1); + int gap = j - i; + + if (gap == 2 && path.charAt(i + 1) == '.') { + // ".", "/./" or "/." + return false; + } else if (gap == 3 && path.charAt(i + 1) == '.' && path.charAt(i + 2) == '.') { + return false; + } + + j = i; + } + + return true; + } + +}
web/src/test/java/org/springframework/security/web/firewall/StrictHttpFirewallTests.java+351 −0 added@@ -0,0 +1,351 @@ +/* + * Copyright 2012-2017 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.firewall; + +import org.junit.Test; +import org.springframework.mock.web.MockHttpServletRequest; + +import static org.assertj.core.api.Assertions.fail; + +/** + * @author Rob Winch + */ +public class StrictHttpFirewallTests { + public String[] unnormalizedPaths = { "/..", "/./path/", "/path/path/.", "/path/path//.", "./path/../path//.", + "./path", ".//path", ".", "/path//" }; + + private StrictHttpFirewall firewall = new StrictHttpFirewall(); + + private MockHttpServletRequest request = new MockHttpServletRequest(); + + @Test + public void getFirewalledRequestWhenRequestURINotNormalizedThenThrowsRequestRejectedException() throws Exception { + for (String path : this.unnormalizedPaths) { + this.request = new MockHttpServletRequest(); + this.request.setRequestURI(path); + try { + this.firewall.getFirewalledRequest(this.request); + fail(path + " is un-normalized"); + } catch (RequestRejectedException expected) { + } + } + } + + @Test + public void getFirewalledRequestWhenContextPathNotNormalizedThenThrowsRequestRejectedException() throws Exception { + for (String path : this.unnormalizedPaths) { + this.request = new MockHttpServletRequest(); + this.request.setContextPath(path); + try { + this.firewall.getFirewalledRequest(this.request); + fail(path + " is un-normalized"); + } catch (RequestRejectedException expected) { + } + } + } + + @Test + public void getFirewalledRequestWhenServletPathNotNormalizedThenThrowsRequestRejectedException() throws Exception { + for (String path : this.unnormalizedPaths) { + this.request = new MockHttpServletRequest(); + this.request.setServletPath(path); + try { + this.firewall.getFirewalledRequest(this.request); + fail(path + " is un-normalized"); + } catch (RequestRejectedException expected) { + } + } + } + + @Test + public void getFirewalledRequestWhenPathInfoNotNormalizedThenThrowsRequestRejectedException() throws Exception { + for (String path : this.unnormalizedPaths) { + this.request = new MockHttpServletRequest(); + this.request.setPathInfo(path); + try { + this.firewall.getFirewalledRequest(this.request); + fail(path + " is un-normalized"); + } catch (RequestRejectedException expected) { + } + } + } + + // --- ; --- + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenSemicolonInContextPathThenThrowsRequestRejectedException() { + this.request.setContextPath(";/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenSemicolonInServletPathThenThrowsRequestRejectedException() { + this.request.setServletPath("/spring;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenSemicolonInPathInfoThenThrowsRequestRejectedException() { + this.request.setPathInfo("/path;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenSemicolonInRequestUriThenThrowsRequestRejectedException() { + this.request.setRequestURI("/path;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedSemicolonInContextPathThenThrowsRequestRejectedException() { + this.request.setContextPath("%3B/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedSemicolonInServletPathThenThrowsRequestRejectedException() { + this.request.setServletPath("/spring%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedSemicolonInPathInfoThenThrowsRequestRejectedException() { + this.request.setPathInfo("/path%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedSemicolonInRequestUriThenThrowsRequestRejectedException() { + this.request.setRequestURI("/path%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInContextPathThenThrowsRequestRejectedException() { + this.request.setContextPath("%3b/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInServletPathThenThrowsRequestRejectedException() { + this.request.setServletPath("/spring%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInPathInfoThenThrowsRequestRejectedException() { + this.request.setPathInfo("/path%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInRequestUriThenThrowsRequestRejectedException() { + this.request.setRequestURI("/path%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenSemicolonInContextPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setContextPath(";/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenSemicolonInServletPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setServletPath("/spring;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenSemicolonInPathInfoAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setPathInfo("/path;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenSemicolonInRequestUriAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setRequestURI("/path;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenEncodedSemicolonInContextPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setContextPath("%3B/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenEncodedSemicolonInServletPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setServletPath("/spring%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenEncodedSemicolonInPathInfoAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setPathInfo("/path%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenEncodedSemicolonInRequestUriAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setRequestURI("/path%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInContextPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setContextPath("%3b/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInServletPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setServletPath("/spring%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInPathInfoAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setPathInfo("/path%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInRequestUriAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setRequestURI("/path%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + // --- encoded . --- + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedPeriodInThenThrowsRequestRejectedException() { + this.request.setRequestURI("/%2E/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedPeriodInThenThrowsRequestRejectedException() { + this.request.setRequestURI("/%2e/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenAllowEncodedPeriodAndEncodedPeriodInThenNoException() { + this.firewall.setAllowUrlEncodedPeriod(true); + this.request.setRequestURI("/%2E/"); + + this.firewall.getFirewalledRequest(this.request); + } + + // --- from DefaultHttpFirewallTests --- + + /** + * On WebSphere 8.5 a URL like /context-root/a/b;%2f1/c can bypass a rule on + * /a/b/c because the pathInfo is /a/b;/1/c which ends up being /a/b/1/c + * while Spring MVC will strip the ; content from requestURI before the path + * is URL decoded. + */ + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedPathThenException() { + this.request.setRequestURI("/context-root/a/b;%2f1/c"); + this.request.setContextPath("/context-root"); + this.request.setServletPath(""); + this.request.setPathInfo("/a/b;/1/c"); // URL decoded requestURI + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenUppercaseEncodedPathThenException() { + this.request.setRequestURI("/context-root/a/b;%2F1/c"); + this.request.setContextPath("/context-root"); + this.request.setServletPath(""); + this.request.setPathInfo("/a/b;/1/c"); // URL decoded requestURI + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenAllowUrlEncodedSlashAndLowercaseEncodedPathThenNoException() { + this.firewall.setAllowUrlEncodedSlash(true); + this.firewall.setAllowSemicolon(true); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setRequestURI("/context-root/a/b;%2f1/c"); + request.setContextPath("/context-root"); + request.setServletPath(""); + request.setPathInfo("/a/b;/1/c"); // URL decoded requestURI + + this.firewall.getFirewalledRequest(request); + } + + @Test + public void getFirewalledRequestWhenAllowUrlEncodedSlashAndUppercaseEncodedPathThenNoException() { + this.firewall.setAllowUrlEncodedSlash(true); + this.firewall.setAllowSemicolon(true); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setRequestURI("/context-root/a/b;%2F1/c"); + request.setContextPath("/context-root"); + request.setServletPath(""); + request.setPathInfo("/a/b;/1/c"); // URL decoded requestURI + + this.firewall.getFirewalledRequest(request); + } +}
cb8041ba6763Add StrictHttpFirewall
7 files changed · +610 −12
config/src/main/java/org/springframework/security/config/annotation/web/builders/WebSecurity.java+2 −2 modified@@ -47,7 +47,7 @@ import org.springframework.security.web.access.expression.DefaultWebSecurityExpressionHandler; import org.springframework.security.web.access.intercept.FilterSecurityInterceptor; import org.springframework.security.web.debug.DebugFilter; -import org.springframework.security.web.firewall.DefaultHttpFirewall; +import org.springframework.security.web.firewall.StrictHttpFirewall; import org.springframework.security.web.firewall.HttpFirewall; import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; @@ -159,7 +159,7 @@ public IgnoredRequestConfigurer ignoring() { /** * Allows customizing the {@link HttpFirewall}. The default is - * {@link DefaultHttpFirewall}. + * {@link StrictHttpFirewall}. * * @param httpFirewall the custom {@link HttpFirewall} * @return the {@link WebSecurity} for further customizations
config/src/test/java/org/springframework/security/config/FilterChainProxyConfigTests.java+4 −0 modified@@ -38,6 +38,7 @@ import org.springframework.security.web.SecurityFilterChain; import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter; import org.springframework.security.web.context.SecurityContextPersistenceFilter; +import org.springframework.security.web.firewall.DefaultHttpFirewall; import org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.AnyRequestMatcher; @@ -80,6 +81,7 @@ public void normalOperation() throws Exception { public void normalOperationWithNewConfig() throws Exception { FilterChainProxy filterChainProxy = appCtx.getBean("newFilterChainProxy", FilterChainProxy.class); + filterChainProxy.setFirewall(new DefaultHttpFirewall()); checkPathAndFilterOrder(filterChainProxy); doNormalOperation(filterChainProxy); } @@ -88,6 +90,7 @@ public void normalOperationWithNewConfig() throws Exception { public void normalOperationWithNewConfigRegex() throws Exception { FilterChainProxy filterChainProxy = appCtx.getBean("newFilterChainProxyRegex", FilterChainProxy.class); + filterChainProxy.setFirewall(new DefaultHttpFirewall()); checkPathAndFilterOrder(filterChainProxy); doNormalOperation(filterChainProxy); } @@ -96,6 +99,7 @@ public void normalOperationWithNewConfigRegex() throws Exception { public void normalOperationWithNewConfigNonNamespace() throws Exception { FilterChainProxy filterChainProxy = appCtx.getBean( "newFilterChainProxyNonNamespace", FilterChainProxy.class); + filterChainProxy.setFirewall(new DefaultHttpFirewall()); checkPathAndFilterOrder(filterChainProxy); doNormalOperation(filterChainProxy); }
itest/context/src/integration-test/java/org/springframework/security/integration/HttpPathParameterStrippingTests.java+9 −7 modified@@ -28,6 +28,7 @@ import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.web.FilterChainProxy; import org.springframework.security.web.context.HttpSessionSecurityContextRepository; +import org.springframework.security.web.firewall.RequestRejectedException; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; @@ -40,32 +41,33 @@ public class HttpPathParameterStrippingTests { @Autowired private FilterChainProxy fcp; - @Test + @Test(expected = RequestRejectedException.class) public void securedFilterChainCannotBeBypassedByAddingPathParameters() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); request.setPathInfo("/secured;x=y/admin.html"); request.setSession(createAuthenticatedSession("ROLE_USER")); MockHttpServletResponse response = new MockHttpServletResponse(); fcp.doFilter(request, response, new MockFilterChain()); - assertThat(response.getStatus()).isEqualTo(403); } - @Test + @Test(expected = RequestRejectedException.class) public void adminFilePatternCannotBeBypassedByAddingPathParameters() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); request.setServletPath("/secured/admin.html;x=user.html"); request.setSession(createAuthenticatedSession("ROLE_USER")); MockHttpServletResponse response = new MockHttpServletResponse(); fcp.doFilter(request, response, new MockFilterChain()); - assertThat(response.getStatus()).isEqualTo(403); + } - // Try with pathInfo - request = new MockHttpServletRequest(); + + @Test(expected = RequestRejectedException.class) + public void adminFilePatternCannotBeBypassedByAddingPathParametersWithPathInfo() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest(); request.setServletPath("/secured"); request.setPathInfo("/admin.html;x=user.html"); request.setSession(createAuthenticatedSession("ROLE_USER")); - response = new MockHttpServletResponse(); + MockHttpServletResponse response = new MockHttpServletResponse(); fcp.doFilter(request, response, new MockFilterChain()); assertThat(response.getStatus()).isEqualTo(403); }
web/src/main/java/org/springframework/security/web/FilterChainProxy.java+3 −3 modified@@ -19,9 +19,9 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.security.core.context.SecurityContextHolder; -import org.springframework.security.web.firewall.DefaultHttpFirewall; import org.springframework.security.web.firewall.FirewalledRequest; import org.springframework.security.web.firewall.HttpFirewall; +import org.springframework.security.web.firewall.StrictHttpFirewall; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.security.web.util.UrlUtils; import org.springframework.web.filter.DelegatingFilterProxy; @@ -96,7 +96,7 @@ * * An {@link HttpFirewall} instance is used to validate incoming requests and create a * wrapped request which provides consistent path values for matching against. See - * {@link DefaultHttpFirewall}, for more information on the type of attacks which the + * {@link StrictHttpFirewall}, for more information on the type of attacks which the * default implementation protects against. A custom implementation can be injected to * provide stricter control over the request contents or if an application needs to * support certain types of request which are rejected by default. @@ -147,7 +147,7 @@ public class FilterChainProxy extends GenericFilterBean { private FilterChainValidator filterChainValidator = new NullFilterChainValidator(); - private HttpFirewall firewall = new DefaultHttpFirewall(); + private HttpFirewall firewall = new StrictHttpFirewall(); // ~ Methods // ========================================================================================================
web/src/main/java/org/springframework/security/web/firewall/DefaultHttpFirewall.java+2 −0 modified@@ -37,8 +37,10 @@ * containers normalize the paths before performing the servlet-mapping, but * again this is not guaranteed by the servlet spec. * + * @deprecated Use {@link StrictHttpFirewall} instead * @author Luke Taylor */ +@Deprecated public class DefaultHttpFirewall implements HttpFirewall { private boolean allowUrlEncodedSlash;
web/src/main/java/org/springframework/security/web/firewall/StrictHttpFirewall.java+239 −0 added@@ -0,0 +1,239 @@ +/* + * Copyright 2012-2017 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.firewall; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +/** + * A strict implementation of {@link HttpFirewall} that rejects any suspicious requests + * with a {@link RequestRejectedException}. + * + * @author Rob Winch + * @since 5.0.1 + */ +public class StrictHttpFirewall implements HttpFirewall { + private static final String ENCODED_PERCENT = "%25"; + + private static final String PERCENT = "%"; + + private static final List<String> FORBIDDEN_ENCODED_PERIOD = Collections.unmodifiableList(Arrays.asList("%2e", "%2E")); + + private static final List<String> FORBIDDEN_SEMICOLON = Collections.unmodifiableList(Arrays.asList(";", "%3b", "%3B")); + + private static final List<String> FORBIDDEN_FORWARDSLASH = Collections.unmodifiableList(Arrays.asList("%2f", "%2F")); + + private static final List<String> FORBIDDEN_BACKSLASH = Collections.unmodifiableList(Arrays.asList("\\", "%5c", "%5C")); + + private Set<String> encodedUrlBlacklist = new HashSet<String>(); + + private Set<String> decodedUrlBlacklist = new HashSet<String>(); + + public StrictHttpFirewall() { + urlBlacklistsAddAll(FORBIDDEN_SEMICOLON); + urlBlacklistsAddAll(FORBIDDEN_FORWARDSLASH); + urlBlacklistsAddAll(FORBIDDEN_BACKSLASH); + + this.encodedUrlBlacklist.add(ENCODED_PERCENT); + this.encodedUrlBlacklist.addAll(FORBIDDEN_ENCODED_PERIOD); + this.decodedUrlBlacklist.add(PERCENT); + } + + /** + * + * @param allowSemicolon + */ + public void setAllowSemicolon(boolean allowSemicolon) { + if (allowSemicolon) { + urlBlacklistsRemoveAll(FORBIDDEN_SEMICOLON); + } else { + urlBlacklistsAddAll(FORBIDDEN_SEMICOLON); + } + } + + public void setAllowUrlEncodedSlash(boolean allowUrlEncodedSlash) { + if (allowUrlEncodedSlash) { + urlBlacklistsRemoveAll(FORBIDDEN_FORWARDSLASH); + } else { + urlBlacklistsAddAll(FORBIDDEN_FORWARDSLASH); + } + } + + public void setAllowUrlEncodedPeriod(boolean allowUrlEncodedPeriod) { + if (allowUrlEncodedPeriod) { + this.encodedUrlBlacklist.removeAll(FORBIDDEN_ENCODED_PERIOD); + } else { + this.encodedUrlBlacklist.addAll(FORBIDDEN_ENCODED_PERIOD); + } + } + + public void setAllowBackSlash(boolean allowBackSlash) { + if (allowBackSlash) { + urlBlacklistsRemoveAll(FORBIDDEN_BACKSLASH); + } else { + urlBlacklistsAddAll(FORBIDDEN_BACKSLASH); + } + } + + public void setAllowUrlEncodedPercent(boolean allowUrlEncodedPercent) { + if (allowUrlEncodedPercent) { + this.encodedUrlBlacklist.remove(ENCODED_PERCENT); + this.decodedUrlBlacklist.remove(PERCENT); + } else { + this.encodedUrlBlacklist.add(ENCODED_PERCENT); + this.decodedUrlBlacklist.add(PERCENT); + } + } + + private void urlBlacklistsAddAll(Collection<String> values) { + this.encodedUrlBlacklist.addAll(values); + this.decodedUrlBlacklist.addAll(values); + } + + private void urlBlacklistsRemoveAll(Collection<String> values) { + this.encodedUrlBlacklist.removeAll(values); + this.decodedUrlBlacklist.removeAll(values); + } + + @Override + public FirewalledRequest getFirewalledRequest(HttpServletRequest request) throws RequestRejectedException { + rejectedBlacklistedUrls(request); + + if (!isNormalized(request)) { + throw new RequestRejectedException("The request was rejected because the URL was not normalized."); + } + + String requestUri = request.getRequestURI(); + if (!containsOnlyPrintableAsciiCharacters(requestUri)) { + throw new RequestRejectedException("The requestURI was rejected because it can only contain printable ASCII characters."); + } + return new FirewalledRequest(request) { + @Override + public void reset() { + } + }; + } + + private void rejectedBlacklistedUrls(HttpServletRequest request) { + for (String forbidden : this.encodedUrlBlacklist) { + if (encodedUrlContains(request, forbidden)) { + throw new RequestRejectedException("The request was rejected because the URL contained a potentially malicious String \"" + forbidden + "\""); + } + } + for (String forbidden : this.decodedUrlBlacklist) { + if (decodedUrlContains(request, forbidden)) { + throw new RequestRejectedException("The request was rejected because the URL contained a potentially malicious String \"" + forbidden + "\""); + } + } + } + + @Override + public HttpServletResponse getFirewalledResponse(HttpServletResponse response) { + return new FirewalledResponse(response); + } + + private static boolean isNormalized(HttpServletRequest request) { + if (!isNormalized(request.getRequestURI())) { + return false; + } + if (!isNormalized(request.getContextPath())) { + return false; + } + if (!isNormalized(request.getServletPath())) { + return false; + } + if (!isNormalized(request.getPathInfo())) { + return false; + } + return true; + } + + private static boolean encodedUrlContains(HttpServletRequest request, String value) { + if (valueContains(request.getContextPath(), value)) { + return true; + } + return valueContains(request.getRequestURI(), value); + } + + private static boolean decodedUrlContains(HttpServletRequest request, String value) { + if (valueContains(request.getServletPath(), value)) { + return true; + } + if (valueContains(request.getPathInfo(), value)) { + return true; + } + return false; + } + + private static boolean containsOnlyPrintableAsciiCharacters(String uri) { + int length = uri.length(); + for (int i = 0; i < length; i++) { + char c = uri.charAt(i); + if (c < '\u0021' || '\u007e' < c) { + return false; + } + } + + return true; + } + + private static boolean valueContains(String value, String contains) { + return value != null && value.contains(contains); + } + + /** + * Checks whether a path is normalized (doesn't contain path traversal + * sequences like "./", "/../" or "/.") + * + * @param path + * the path to test + * @return true if the path doesn't contain any path-traversal character + * sequences. + */ + private static boolean isNormalized(String path) { + if (path == null) { + return true; + } + + if (path.indexOf("//") > 0) { + return false; + } + + for (int j = path.length(); j > 0;) { + int i = path.lastIndexOf('/', j - 1); + int gap = j - i; + + if (gap == 2 && path.charAt(i + 1) == '.') { + // ".", "/./" or "/." + return false; + } else if (gap == 3 && path.charAt(i + 1) == '.' && path.charAt(i + 2) == '.') { + return false; + } + + j = i; + } + + return true; + } + +}
web/src/test/java/org/springframework/security/web/firewall/StrictHttpFirewallTests.java+351 −0 added@@ -0,0 +1,351 @@ +/* + * Copyright 2012-2017 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.firewall; + +import org.junit.Test; +import org.springframework.mock.web.MockHttpServletRequest; + +import static org.assertj.core.api.Assertions.fail; + +/** + * @author Rob Winch + */ +public class StrictHttpFirewallTests { + public String[] unnormalizedPaths = { "/..", "/./path/", "/path/path/.", "/path/path//.", "./path/../path//.", + "./path", ".//path", ".", "/path//" }; + + private StrictHttpFirewall firewall = new StrictHttpFirewall(); + + private MockHttpServletRequest request = new MockHttpServletRequest(); + + @Test + public void getFirewalledRequestWhenRequestURINotNormalizedThenThrowsRequestRejectedException() throws Exception { + for (String path : this.unnormalizedPaths) { + this.request = new MockHttpServletRequest(); + this.request.setRequestURI(path); + try { + this.firewall.getFirewalledRequest(this.request); + fail(path + " is un-normalized"); + } catch (RequestRejectedException expected) { + } + } + } + + @Test + public void getFirewalledRequestWhenContextPathNotNormalizedThenThrowsRequestRejectedException() throws Exception { + for (String path : this.unnormalizedPaths) { + this.request = new MockHttpServletRequest(); + this.request.setContextPath(path); + try { + this.firewall.getFirewalledRequest(this.request); + fail(path + " is un-normalized"); + } catch (RequestRejectedException expected) { + } + } + } + + @Test + public void getFirewalledRequestWhenServletPathNotNormalizedThenThrowsRequestRejectedException() throws Exception { + for (String path : this.unnormalizedPaths) { + this.request = new MockHttpServletRequest(); + this.request.setServletPath(path); + try { + this.firewall.getFirewalledRequest(this.request); + fail(path + " is un-normalized"); + } catch (RequestRejectedException expected) { + } + } + } + + @Test + public void getFirewalledRequestWhenPathInfoNotNormalizedThenThrowsRequestRejectedException() throws Exception { + for (String path : this.unnormalizedPaths) { + this.request = new MockHttpServletRequest(); + this.request.setPathInfo(path); + try { + this.firewall.getFirewalledRequest(this.request); + fail(path + " is un-normalized"); + } catch (RequestRejectedException expected) { + } + } + } + + // --- ; --- + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenSemicolonInContextPathThenThrowsRequestRejectedException() { + this.request.setContextPath(";/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenSemicolonInServletPathThenThrowsRequestRejectedException() { + this.request.setServletPath("/spring;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenSemicolonInPathInfoThenThrowsRequestRejectedException() { + this.request.setPathInfo("/path;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenSemicolonInRequestUriThenThrowsRequestRejectedException() { + this.request.setRequestURI("/path;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedSemicolonInContextPathThenThrowsRequestRejectedException() { + this.request.setContextPath("%3B/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedSemicolonInServletPathThenThrowsRequestRejectedException() { + this.request.setServletPath("/spring%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedSemicolonInPathInfoThenThrowsRequestRejectedException() { + this.request.setPathInfo("/path%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedSemicolonInRequestUriThenThrowsRequestRejectedException() { + this.request.setRequestURI("/path%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInContextPathThenThrowsRequestRejectedException() { + this.request.setContextPath("%3b/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInServletPathThenThrowsRequestRejectedException() { + this.request.setServletPath("/spring%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInPathInfoThenThrowsRequestRejectedException() { + this.request.setPathInfo("/path%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInRequestUriThenThrowsRequestRejectedException() { + this.request.setRequestURI("/path%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenSemicolonInContextPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setContextPath(";/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenSemicolonInServletPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setServletPath("/spring;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenSemicolonInPathInfoAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setPathInfo("/path;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenSemicolonInRequestUriAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setRequestURI("/path;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenEncodedSemicolonInContextPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setContextPath("%3B/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenEncodedSemicolonInServletPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setServletPath("/spring%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenEncodedSemicolonInPathInfoAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setPathInfo("/path%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenEncodedSemicolonInRequestUriAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setRequestURI("/path%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInContextPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setContextPath("%3b/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInServletPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setServletPath("/spring%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInPathInfoAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setPathInfo("/path%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInRequestUriAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setRequestURI("/path%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + // --- encoded . --- + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedPeriodInThenThrowsRequestRejectedException() { + this.request.setRequestURI("/%2E/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedPeriodInThenThrowsRequestRejectedException() { + this.request.setRequestURI("/%2e/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenAllowEncodedPeriodAndEncodedPeriodInThenNoException() { + this.firewall.setAllowUrlEncodedPeriod(true); + this.request.setRequestURI("/%2E/"); + + this.firewall.getFirewalledRequest(this.request); + } + + // --- from DefaultHttpFirewallTests --- + + /** + * On WebSphere 8.5 a URL like /context-root/a/b;%2f1/c can bypass a rule on + * /a/b/c because the pathInfo is /a/b;/1/c which ends up being /a/b/1/c + * while Spring MVC will strip the ; content from requestURI before the path + * is URL decoded. + */ + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedPathThenException() { + this.request.setRequestURI("/context-root/a/b;%2f1/c"); + this.request.setContextPath("/context-root"); + this.request.setServletPath(""); + this.request.setPathInfo("/a/b;/1/c"); // URL decoded requestURI + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenUppercaseEncodedPathThenException() { + this.request.setRequestURI("/context-root/a/b;%2F1/c"); + this.request.setContextPath("/context-root"); + this.request.setServletPath(""); + this.request.setPathInfo("/a/b;/1/c"); // URL decoded requestURI + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenAllowUrlEncodedSlashAndLowercaseEncodedPathThenNoException() { + this.firewall.setAllowUrlEncodedSlash(true); + this.firewall.setAllowSemicolon(true); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setRequestURI("/context-root/a/b;%2f1/c"); + request.setContextPath("/context-root"); + request.setServletPath(""); + request.setPathInfo("/a/b;/1/c"); // URL decoded requestURI + + this.firewall.getFirewalledRequest(request); + } + + @Test + public void getFirewalledRequestWhenAllowUrlEncodedSlashAndUppercaseEncodedPathThenNoException() { + this.firewall.setAllowUrlEncodedSlash(true); + this.firewall.setAllowSemicolon(true); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setRequestURI("/context-root/a/b;%2F1/c"); + request.setContextPath("/context-root"); + request.setServletPath(""); + request.setPathInfo("/a/b;/1/c"); // URL decoded requestURI + + this.firewall.getFirewalledRequest(request); + } +}
0eef5b4b425aAdd StrictHttpFirewall
7 files changed · +610 −12
config/src/main/java/org/springframework/security/config/annotation/web/builders/WebSecurity.java+2 −2 modified@@ -47,7 +47,7 @@ import org.springframework.security.web.access.expression.DefaultWebSecurityExpressionHandler; import org.springframework.security.web.access.intercept.FilterSecurityInterceptor; import org.springframework.security.web.debug.DebugFilter; -import org.springframework.security.web.firewall.DefaultHttpFirewall; +import org.springframework.security.web.firewall.StrictHttpFirewall; import org.springframework.security.web.firewall.HttpFirewall; import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; @@ -159,7 +159,7 @@ public IgnoredRequestConfigurer ignoring() { /** * Allows customizing the {@link HttpFirewall}. The default is - * {@link DefaultHttpFirewall}. + * {@link StrictHttpFirewall}. * * @param httpFirewall the custom {@link HttpFirewall} * @return the {@link WebSecurity} for further customizations
config/src/test/java/org/springframework/security/config/FilterChainProxyConfigTests.java+4 −0 modified@@ -38,6 +38,7 @@ import org.springframework.security.web.SecurityFilterChain; import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter; import org.springframework.security.web.context.SecurityContextPersistenceFilter; +import org.springframework.security.web.firewall.DefaultHttpFirewall; import org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.AnyRequestMatcher; @@ -80,6 +81,7 @@ public void normalOperation() throws Exception { public void normalOperationWithNewConfig() throws Exception { FilterChainProxy filterChainProxy = appCtx.getBean("newFilterChainProxy", FilterChainProxy.class); + filterChainProxy.setFirewall(new DefaultHttpFirewall()); checkPathAndFilterOrder(filterChainProxy); doNormalOperation(filterChainProxy); } @@ -88,6 +90,7 @@ public void normalOperationWithNewConfig() throws Exception { public void normalOperationWithNewConfigRegex() throws Exception { FilterChainProxy filterChainProxy = appCtx.getBean("newFilterChainProxyRegex", FilterChainProxy.class); + filterChainProxy.setFirewall(new DefaultHttpFirewall()); checkPathAndFilterOrder(filterChainProxy); doNormalOperation(filterChainProxy); } @@ -96,6 +99,7 @@ public void normalOperationWithNewConfigRegex() throws Exception { public void normalOperationWithNewConfigNonNamespace() throws Exception { FilterChainProxy filterChainProxy = appCtx.getBean( "newFilterChainProxyNonNamespace", FilterChainProxy.class); + filterChainProxy.setFirewall(new DefaultHttpFirewall()); checkPathAndFilterOrder(filterChainProxy); doNormalOperation(filterChainProxy); }
itest/context/src/integration-test/java/org/springframework/security/integration/HttpPathParameterStrippingTests.java+9 −7 modified@@ -28,6 +28,7 @@ import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.web.FilterChainProxy; import org.springframework.security.web.context.HttpSessionSecurityContextRepository; +import org.springframework.security.web.firewall.RequestRejectedException; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; @@ -40,32 +41,33 @@ public class HttpPathParameterStrippingTests { @Autowired private FilterChainProxy fcp; - @Test + @Test(expected = RequestRejectedException.class) public void securedFilterChainCannotBeBypassedByAddingPathParameters() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); request.setPathInfo("/secured;x=y/admin.html"); request.setSession(createAuthenticatedSession("ROLE_USER")); MockHttpServletResponse response = new MockHttpServletResponse(); fcp.doFilter(request, response, new MockFilterChain()); - assertThat(response.getStatus()).isEqualTo(403); } - @Test + @Test(expected = RequestRejectedException.class) public void adminFilePatternCannotBeBypassedByAddingPathParameters() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); request.setServletPath("/secured/admin.html;x=user.html"); request.setSession(createAuthenticatedSession("ROLE_USER")); MockHttpServletResponse response = new MockHttpServletResponse(); fcp.doFilter(request, response, new MockFilterChain()); - assertThat(response.getStatus()).isEqualTo(403); + } - // Try with pathInfo - request = new MockHttpServletRequest(); + + @Test(expected = RequestRejectedException.class) + public void adminFilePatternCannotBeBypassedByAddingPathParametersWithPathInfo() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest(); request.setServletPath("/secured"); request.setPathInfo("/admin.html;x=user.html"); request.setSession(createAuthenticatedSession("ROLE_USER")); - response = new MockHttpServletResponse(); + MockHttpServletResponse response = new MockHttpServletResponse(); fcp.doFilter(request, response, new MockFilterChain()); assertThat(response.getStatus()).isEqualTo(403); }
web/src/main/java/org/springframework/security/web/FilterChainProxy.java+3 −3 modified@@ -19,9 +19,9 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.security.core.context.SecurityContextHolder; -import org.springframework.security.web.firewall.DefaultHttpFirewall; import org.springframework.security.web.firewall.FirewalledRequest; import org.springframework.security.web.firewall.HttpFirewall; +import org.springframework.security.web.firewall.StrictHttpFirewall; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.security.web.util.UrlUtils; import org.springframework.web.filter.DelegatingFilterProxy; @@ -96,7 +96,7 @@ * * An {@link HttpFirewall} instance is used to validate incoming requests and create a * wrapped request which provides consistent path values for matching against. See - * {@link DefaultHttpFirewall}, for more information on the type of attacks which the + * {@link StrictHttpFirewall}, for more information on the type of attacks which the * default implementation protects against. A custom implementation can be injected to * provide stricter control over the request contents or if an application needs to * support certain types of request which are rejected by default. @@ -147,7 +147,7 @@ public class FilterChainProxy extends GenericFilterBean { private FilterChainValidator filterChainValidator = new NullFilterChainValidator(); - private HttpFirewall firewall = new DefaultHttpFirewall(); + private HttpFirewall firewall = new StrictHttpFirewall(); // ~ Methods // ========================================================================================================
web/src/main/java/org/springframework/security/web/firewall/DefaultHttpFirewall.java+2 −0 modified@@ -37,8 +37,10 @@ * containers normalize the paths before performing the servlet-mapping, but * again this is not guaranteed by the servlet spec. * + * @deprecated Use {@link StrictHttpFirewall} instead * @author Luke Taylor */ +@Deprecated public class DefaultHttpFirewall implements HttpFirewall { private boolean allowUrlEncodedSlash;
web/src/main/java/org/springframework/security/web/firewall/StrictHttpFirewall.java+239 −0 added@@ -0,0 +1,239 @@ +/* + * Copyright 2012-2017 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.firewall; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +/** + * A strict implementation of {@link HttpFirewall} that rejects any suspicious requests + * with a {@link RequestRejectedException}. + * + * @author Rob Winch + * @since 5.0.1 + */ +public class StrictHttpFirewall implements HttpFirewall { + private static final String ENCODED_PERCENT = "%25"; + + private static final String PERCENT = "%"; + + private static final List<String> FORBIDDEN_ENCODED_PERIOD = Collections.unmodifiableList(Arrays.asList("%2e", "%2E")); + + private static final List<String> FORBIDDEN_SEMICOLON = Collections.unmodifiableList(Arrays.asList(";", "%3b", "%3B")); + + private static final List<String> FORBIDDEN_FORWARDSLASH = Collections.unmodifiableList(Arrays.asList("%2f", "%2F")); + + private static final List<String> FORBIDDEN_BACKSLASH = Collections.unmodifiableList(Arrays.asList("\\", "%5c", "%5C")); + + private Set<String> encodedUrlBlacklist = new HashSet<String>(); + + private Set<String> decodedUrlBlacklist = new HashSet<String>(); + + public StrictHttpFirewall() { + urlBlacklistsAddAll(FORBIDDEN_SEMICOLON); + urlBlacklistsAddAll(FORBIDDEN_FORWARDSLASH); + urlBlacklistsAddAll(FORBIDDEN_BACKSLASH); + + this.encodedUrlBlacklist.add(ENCODED_PERCENT); + this.encodedUrlBlacklist.addAll(FORBIDDEN_ENCODED_PERIOD); + this.decodedUrlBlacklist.add(PERCENT); + } + + /** + * + * @param allowSemicolon + */ + public void setAllowSemicolon(boolean allowSemicolon) { + if (allowSemicolon) { + urlBlacklistsRemoveAll(FORBIDDEN_SEMICOLON); + } else { + urlBlacklistsAddAll(FORBIDDEN_SEMICOLON); + } + } + + public void setAllowUrlEncodedSlash(boolean allowUrlEncodedSlash) { + if (allowUrlEncodedSlash) { + urlBlacklistsRemoveAll(FORBIDDEN_FORWARDSLASH); + } else { + urlBlacklistsAddAll(FORBIDDEN_FORWARDSLASH); + } + } + + public void setAllowUrlEncodedPeriod(boolean allowUrlEncodedPeriod) { + if (allowUrlEncodedPeriod) { + this.encodedUrlBlacklist.removeAll(FORBIDDEN_ENCODED_PERIOD); + } else { + this.encodedUrlBlacklist.addAll(FORBIDDEN_ENCODED_PERIOD); + } + } + + public void setAllowBackSlash(boolean allowBackSlash) { + if (allowBackSlash) { + urlBlacklistsRemoveAll(FORBIDDEN_BACKSLASH); + } else { + urlBlacklistsAddAll(FORBIDDEN_BACKSLASH); + } + } + + public void setAllowUrlEncodedPercent(boolean allowUrlEncodedPercent) { + if (allowUrlEncodedPercent) { + this.encodedUrlBlacklist.remove(ENCODED_PERCENT); + this.decodedUrlBlacklist.remove(PERCENT); + } else { + this.encodedUrlBlacklist.add(ENCODED_PERCENT); + this.decodedUrlBlacklist.add(PERCENT); + } + } + + private void urlBlacklistsAddAll(Collection<String> values) { + this.encodedUrlBlacklist.addAll(values); + this.decodedUrlBlacklist.addAll(values); + } + + private void urlBlacklistsRemoveAll(Collection<String> values) { + this.encodedUrlBlacklist.removeAll(values); + this.decodedUrlBlacklist.removeAll(values); + } + + @Override + public FirewalledRequest getFirewalledRequest(HttpServletRequest request) throws RequestRejectedException { + rejectedBlacklistedUrls(request); + + if (!isNormalized(request)) { + throw new RequestRejectedException("The request was rejected because the URL was not normalized."); + } + + String requestUri = request.getRequestURI(); + if (!containsOnlyPrintableAsciiCharacters(requestUri)) { + throw new RequestRejectedException("The requestURI was rejected because it can only contain printable ASCII characters."); + } + return new FirewalledRequest(request) { + @Override + public void reset() { + } + }; + } + + private void rejectedBlacklistedUrls(HttpServletRequest request) { + for (String forbidden : this.encodedUrlBlacklist) { + if (encodedUrlContains(request, forbidden)) { + throw new RequestRejectedException("The request was rejected because the URL contained a potentially malicious String \"" + forbidden + "\""); + } + } + for (String forbidden : this.decodedUrlBlacklist) { + if (decodedUrlContains(request, forbidden)) { + throw new RequestRejectedException("The request was rejected because the URL contained a potentially malicious String \"" + forbidden + "\""); + } + } + } + + @Override + public HttpServletResponse getFirewalledResponse(HttpServletResponse response) { + return new FirewalledResponse(response); + } + + private static boolean isNormalized(HttpServletRequest request) { + if (!isNormalized(request.getRequestURI())) { + return false; + } + if (!isNormalized(request.getContextPath())) { + return false; + } + if (!isNormalized(request.getServletPath())) { + return false; + } + if (!isNormalized(request.getPathInfo())) { + return false; + } + return true; + } + + private static boolean encodedUrlContains(HttpServletRequest request, String value) { + if (valueContains(request.getContextPath(), value)) { + return true; + } + return valueContains(request.getRequestURI(), value); + } + + private static boolean decodedUrlContains(HttpServletRequest request, String value) { + if (valueContains(request.getServletPath(), value)) { + return true; + } + if (valueContains(request.getPathInfo(), value)) { + return true; + } + return false; + } + + private static boolean containsOnlyPrintableAsciiCharacters(String uri) { + int length = uri.length(); + for (int i = 0; i < length; i++) { + char c = uri.charAt(i); + if (c < '\u0021' || '\u007e' < c) { + return false; + } + } + + return true; + } + + private static boolean valueContains(String value, String contains) { + return value != null && value.contains(contains); + } + + /** + * Checks whether a path is normalized (doesn't contain path traversal + * sequences like "./", "/../" or "/.") + * + * @param path + * the path to test + * @return true if the path doesn't contain any path-traversal character + * sequences. + */ + private static boolean isNormalized(String path) { + if (path == null) { + return true; + } + + if (path.indexOf("//") > 0) { + return false; + } + + for (int j = path.length(); j > 0;) { + int i = path.lastIndexOf('/', j - 1); + int gap = j - i; + + if (gap == 2 && path.charAt(i + 1) == '.') { + // ".", "/./" or "/." + return false; + } else if (gap == 3 && path.charAt(i + 1) == '.' && path.charAt(i + 2) == '.') { + return false; + } + + j = i; + } + + return true; + } + +}
web/src/test/java/org/springframework/security/web/firewall/StrictHttpFirewallTests.java+351 −0 added@@ -0,0 +1,351 @@ +/* + * Copyright 2012-2017 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.firewall; + +import org.junit.Test; +import org.springframework.mock.web.MockHttpServletRequest; + +import static org.assertj.core.api.Assertions.fail; + +/** + * @author Rob Winch + */ +public class StrictHttpFirewallTests { + public String[] unnormalizedPaths = { "/..", "/./path/", "/path/path/.", "/path/path//.", "./path/../path//.", + "./path", ".//path", ".", "/path//" }; + + private StrictHttpFirewall firewall = new StrictHttpFirewall(); + + private MockHttpServletRequest request = new MockHttpServletRequest(); + + @Test + public void getFirewalledRequestWhenRequestURINotNormalizedThenThrowsRequestRejectedException() throws Exception { + for (String path : this.unnormalizedPaths) { + this.request = new MockHttpServletRequest(); + this.request.setRequestURI(path); + try { + this.firewall.getFirewalledRequest(this.request); + fail(path + " is un-normalized"); + } catch (RequestRejectedException expected) { + } + } + } + + @Test + public void getFirewalledRequestWhenContextPathNotNormalizedThenThrowsRequestRejectedException() throws Exception { + for (String path : this.unnormalizedPaths) { + this.request = new MockHttpServletRequest(); + this.request.setContextPath(path); + try { + this.firewall.getFirewalledRequest(this.request); + fail(path + " is un-normalized"); + } catch (RequestRejectedException expected) { + } + } + } + + @Test + public void getFirewalledRequestWhenServletPathNotNormalizedThenThrowsRequestRejectedException() throws Exception { + for (String path : this.unnormalizedPaths) { + this.request = new MockHttpServletRequest(); + this.request.setServletPath(path); + try { + this.firewall.getFirewalledRequest(this.request); + fail(path + " is un-normalized"); + } catch (RequestRejectedException expected) { + } + } + } + + @Test + public void getFirewalledRequestWhenPathInfoNotNormalizedThenThrowsRequestRejectedException() throws Exception { + for (String path : this.unnormalizedPaths) { + this.request = new MockHttpServletRequest(); + this.request.setPathInfo(path); + try { + this.firewall.getFirewalledRequest(this.request); + fail(path + " is un-normalized"); + } catch (RequestRejectedException expected) { + } + } + } + + // --- ; --- + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenSemicolonInContextPathThenThrowsRequestRejectedException() { + this.request.setContextPath(";/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenSemicolonInServletPathThenThrowsRequestRejectedException() { + this.request.setServletPath("/spring;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenSemicolonInPathInfoThenThrowsRequestRejectedException() { + this.request.setPathInfo("/path;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenSemicolonInRequestUriThenThrowsRequestRejectedException() { + this.request.setRequestURI("/path;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedSemicolonInContextPathThenThrowsRequestRejectedException() { + this.request.setContextPath("%3B/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedSemicolonInServletPathThenThrowsRequestRejectedException() { + this.request.setServletPath("/spring%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedSemicolonInPathInfoThenThrowsRequestRejectedException() { + this.request.setPathInfo("/path%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedSemicolonInRequestUriThenThrowsRequestRejectedException() { + this.request.setRequestURI("/path%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInContextPathThenThrowsRequestRejectedException() { + this.request.setContextPath("%3b/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInServletPathThenThrowsRequestRejectedException() { + this.request.setServletPath("/spring%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInPathInfoThenThrowsRequestRejectedException() { + this.request.setPathInfo("/path%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInRequestUriThenThrowsRequestRejectedException() { + this.request.setRequestURI("/path%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenSemicolonInContextPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setContextPath(";/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenSemicolonInServletPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setServletPath("/spring;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenSemicolonInPathInfoAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setPathInfo("/path;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenSemicolonInRequestUriAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setRequestURI("/path;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenEncodedSemicolonInContextPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setContextPath("%3B/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenEncodedSemicolonInServletPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setServletPath("/spring%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenEncodedSemicolonInPathInfoAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setPathInfo("/path%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenEncodedSemicolonInRequestUriAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setRequestURI("/path%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInContextPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setContextPath("%3b/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInServletPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setServletPath("/spring%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInPathInfoAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setPathInfo("/path%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInRequestUriAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setRequestURI("/path%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + // --- encoded . --- + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedPeriodInThenThrowsRequestRejectedException() { + this.request.setRequestURI("/%2E/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedPeriodInThenThrowsRequestRejectedException() { + this.request.setRequestURI("/%2e/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenAllowEncodedPeriodAndEncodedPeriodInThenNoException() { + this.firewall.setAllowUrlEncodedPeriod(true); + this.request.setRequestURI("/%2E/"); + + this.firewall.getFirewalledRequest(this.request); + } + + // --- from DefaultHttpFirewallTests --- + + /** + * On WebSphere 8.5 a URL like /context-root/a/b;%2f1/c can bypass a rule on + * /a/b/c because the pathInfo is /a/b;/1/c which ends up being /a/b/1/c + * while Spring MVC will strip the ; content from requestURI before the path + * is URL decoded. + */ + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedPathThenException() { + this.request.setRequestURI("/context-root/a/b;%2f1/c"); + this.request.setContextPath("/context-root"); + this.request.setServletPath(""); + this.request.setPathInfo("/a/b;/1/c"); // URL decoded requestURI + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenUppercaseEncodedPathThenException() { + this.request.setRequestURI("/context-root/a/b;%2F1/c"); + this.request.setContextPath("/context-root"); + this.request.setServletPath(""); + this.request.setPathInfo("/a/b;/1/c"); // URL decoded requestURI + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenAllowUrlEncodedSlashAndLowercaseEncodedPathThenNoException() { + this.firewall.setAllowUrlEncodedSlash(true); + this.firewall.setAllowSemicolon(true); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setRequestURI("/context-root/a/b;%2f1/c"); + request.setContextPath("/context-root"); + request.setServletPath(""); + request.setPathInfo("/a/b;/1/c"); // URL decoded requestURI + + this.firewall.getFirewalledRequest(request); + } + + @Test + public void getFirewalledRequestWhenAllowUrlEncodedSlashAndUppercaseEncodedPathThenNoException() { + this.firewall.setAllowUrlEncodedSlash(true); + this.firewall.setAllowSemicolon(true); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setRequestURI("/context-root/a/b;%2F1/c"); + request.setContextPath("/context-root"); + request.setServletPath(""); + request.setPathInfo("/a/b;/1/c"); // URL decoded requestURI + + this.firewall.getFirewalledRequest(request); + } +}
0eef5b4b425aAdd StrictHttpFirewall
7 files changed · +610 −12
config/src/main/java/org/springframework/security/config/annotation/web/builders/WebSecurity.java+2 −2 modified@@ -47,7 +47,7 @@ import org.springframework.security.web.access.expression.DefaultWebSecurityExpressionHandler; import org.springframework.security.web.access.intercept.FilterSecurityInterceptor; import org.springframework.security.web.debug.DebugFilter; -import org.springframework.security.web.firewall.DefaultHttpFirewall; +import org.springframework.security.web.firewall.StrictHttpFirewall; import org.springframework.security.web.firewall.HttpFirewall; import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; @@ -159,7 +159,7 @@ public IgnoredRequestConfigurer ignoring() { /** * Allows customizing the {@link HttpFirewall}. The default is - * {@link DefaultHttpFirewall}. + * {@link StrictHttpFirewall}. * * @param httpFirewall the custom {@link HttpFirewall} * @return the {@link WebSecurity} for further customizations
config/src/test/java/org/springframework/security/config/FilterChainProxyConfigTests.java+4 −0 modified@@ -38,6 +38,7 @@ import org.springframework.security.web.SecurityFilterChain; import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter; import org.springframework.security.web.context.SecurityContextPersistenceFilter; +import org.springframework.security.web.firewall.DefaultHttpFirewall; import org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.AnyRequestMatcher; @@ -80,6 +81,7 @@ public void normalOperation() throws Exception { public void normalOperationWithNewConfig() throws Exception { FilterChainProxy filterChainProxy = appCtx.getBean("newFilterChainProxy", FilterChainProxy.class); + filterChainProxy.setFirewall(new DefaultHttpFirewall()); checkPathAndFilterOrder(filterChainProxy); doNormalOperation(filterChainProxy); } @@ -88,6 +90,7 @@ public void normalOperationWithNewConfig() throws Exception { public void normalOperationWithNewConfigRegex() throws Exception { FilterChainProxy filterChainProxy = appCtx.getBean("newFilterChainProxyRegex", FilterChainProxy.class); + filterChainProxy.setFirewall(new DefaultHttpFirewall()); checkPathAndFilterOrder(filterChainProxy); doNormalOperation(filterChainProxy); } @@ -96,6 +99,7 @@ public void normalOperationWithNewConfigRegex() throws Exception { public void normalOperationWithNewConfigNonNamespace() throws Exception { FilterChainProxy filterChainProxy = appCtx.getBean( "newFilterChainProxyNonNamespace", FilterChainProxy.class); + filterChainProxy.setFirewall(new DefaultHttpFirewall()); checkPathAndFilterOrder(filterChainProxy); doNormalOperation(filterChainProxy); }
itest/context/src/integration-test/java/org/springframework/security/integration/HttpPathParameterStrippingTests.java+9 −7 modified@@ -28,6 +28,7 @@ import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.web.FilterChainProxy; import org.springframework.security.web.context.HttpSessionSecurityContextRepository; +import org.springframework.security.web.firewall.RequestRejectedException; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; @@ -40,32 +41,33 @@ public class HttpPathParameterStrippingTests { @Autowired private FilterChainProxy fcp; - @Test + @Test(expected = RequestRejectedException.class) public void securedFilterChainCannotBeBypassedByAddingPathParameters() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); request.setPathInfo("/secured;x=y/admin.html"); request.setSession(createAuthenticatedSession("ROLE_USER")); MockHttpServletResponse response = new MockHttpServletResponse(); fcp.doFilter(request, response, new MockFilterChain()); - assertThat(response.getStatus()).isEqualTo(403); } - @Test + @Test(expected = RequestRejectedException.class) public void adminFilePatternCannotBeBypassedByAddingPathParameters() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); request.setServletPath("/secured/admin.html;x=user.html"); request.setSession(createAuthenticatedSession("ROLE_USER")); MockHttpServletResponse response = new MockHttpServletResponse(); fcp.doFilter(request, response, new MockFilterChain()); - assertThat(response.getStatus()).isEqualTo(403); + } - // Try with pathInfo - request = new MockHttpServletRequest(); + + @Test(expected = RequestRejectedException.class) + public void adminFilePatternCannotBeBypassedByAddingPathParametersWithPathInfo() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest(); request.setServletPath("/secured"); request.setPathInfo("/admin.html;x=user.html"); request.setSession(createAuthenticatedSession("ROLE_USER")); - response = new MockHttpServletResponse(); + MockHttpServletResponse response = new MockHttpServletResponse(); fcp.doFilter(request, response, new MockFilterChain()); assertThat(response.getStatus()).isEqualTo(403); }
web/src/main/java/org/springframework/security/web/FilterChainProxy.java+3 −3 modified@@ -19,9 +19,9 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.security.core.context.SecurityContextHolder; -import org.springframework.security.web.firewall.DefaultHttpFirewall; import org.springframework.security.web.firewall.FirewalledRequest; import org.springframework.security.web.firewall.HttpFirewall; +import org.springframework.security.web.firewall.StrictHttpFirewall; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.security.web.util.UrlUtils; import org.springframework.web.filter.DelegatingFilterProxy; @@ -96,7 +96,7 @@ * * An {@link HttpFirewall} instance is used to validate incoming requests and create a * wrapped request which provides consistent path values for matching against. See - * {@link DefaultHttpFirewall}, for more information on the type of attacks which the + * {@link StrictHttpFirewall}, for more information on the type of attacks which the * default implementation protects against. A custom implementation can be injected to * provide stricter control over the request contents or if an application needs to * support certain types of request which are rejected by default. @@ -147,7 +147,7 @@ public class FilterChainProxy extends GenericFilterBean { private FilterChainValidator filterChainValidator = new NullFilterChainValidator(); - private HttpFirewall firewall = new DefaultHttpFirewall(); + private HttpFirewall firewall = new StrictHttpFirewall(); // ~ Methods // ========================================================================================================
web/src/main/java/org/springframework/security/web/firewall/DefaultHttpFirewall.java+2 −0 modified@@ -37,8 +37,10 @@ * containers normalize the paths before performing the servlet-mapping, but * again this is not guaranteed by the servlet spec. * + * @deprecated Use {@link StrictHttpFirewall} instead * @author Luke Taylor */ +@Deprecated public class DefaultHttpFirewall implements HttpFirewall { private boolean allowUrlEncodedSlash;
web/src/main/java/org/springframework/security/web/firewall/StrictHttpFirewall.java+239 −0 added@@ -0,0 +1,239 @@ +/* + * Copyright 2012-2017 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.firewall; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +/** + * A strict implementation of {@link HttpFirewall} that rejects any suspicious requests + * with a {@link RequestRejectedException}. + * + * @author Rob Winch + * @since 5.0.1 + */ +public class StrictHttpFirewall implements HttpFirewall { + private static final String ENCODED_PERCENT = "%25"; + + private static final String PERCENT = "%"; + + private static final List<String> FORBIDDEN_ENCODED_PERIOD = Collections.unmodifiableList(Arrays.asList("%2e", "%2E")); + + private static final List<String> FORBIDDEN_SEMICOLON = Collections.unmodifiableList(Arrays.asList(";", "%3b", "%3B")); + + private static final List<String> FORBIDDEN_FORWARDSLASH = Collections.unmodifiableList(Arrays.asList("%2f", "%2F")); + + private static final List<String> FORBIDDEN_BACKSLASH = Collections.unmodifiableList(Arrays.asList("\\", "%5c", "%5C")); + + private Set<String> encodedUrlBlacklist = new HashSet<String>(); + + private Set<String> decodedUrlBlacklist = new HashSet<String>(); + + public StrictHttpFirewall() { + urlBlacklistsAddAll(FORBIDDEN_SEMICOLON); + urlBlacklistsAddAll(FORBIDDEN_FORWARDSLASH); + urlBlacklistsAddAll(FORBIDDEN_BACKSLASH); + + this.encodedUrlBlacklist.add(ENCODED_PERCENT); + this.encodedUrlBlacklist.addAll(FORBIDDEN_ENCODED_PERIOD); + this.decodedUrlBlacklist.add(PERCENT); + } + + /** + * + * @param allowSemicolon + */ + public void setAllowSemicolon(boolean allowSemicolon) { + if (allowSemicolon) { + urlBlacklistsRemoveAll(FORBIDDEN_SEMICOLON); + } else { + urlBlacklistsAddAll(FORBIDDEN_SEMICOLON); + } + } + + public void setAllowUrlEncodedSlash(boolean allowUrlEncodedSlash) { + if (allowUrlEncodedSlash) { + urlBlacklistsRemoveAll(FORBIDDEN_FORWARDSLASH); + } else { + urlBlacklistsAddAll(FORBIDDEN_FORWARDSLASH); + } + } + + public void setAllowUrlEncodedPeriod(boolean allowUrlEncodedPeriod) { + if (allowUrlEncodedPeriod) { + this.encodedUrlBlacklist.removeAll(FORBIDDEN_ENCODED_PERIOD); + } else { + this.encodedUrlBlacklist.addAll(FORBIDDEN_ENCODED_PERIOD); + } + } + + public void setAllowBackSlash(boolean allowBackSlash) { + if (allowBackSlash) { + urlBlacklistsRemoveAll(FORBIDDEN_BACKSLASH); + } else { + urlBlacklistsAddAll(FORBIDDEN_BACKSLASH); + } + } + + public void setAllowUrlEncodedPercent(boolean allowUrlEncodedPercent) { + if (allowUrlEncodedPercent) { + this.encodedUrlBlacklist.remove(ENCODED_PERCENT); + this.decodedUrlBlacklist.remove(PERCENT); + } else { + this.encodedUrlBlacklist.add(ENCODED_PERCENT); + this.decodedUrlBlacklist.add(PERCENT); + } + } + + private void urlBlacklistsAddAll(Collection<String> values) { + this.encodedUrlBlacklist.addAll(values); + this.decodedUrlBlacklist.addAll(values); + } + + private void urlBlacklistsRemoveAll(Collection<String> values) { + this.encodedUrlBlacklist.removeAll(values); + this.decodedUrlBlacklist.removeAll(values); + } + + @Override + public FirewalledRequest getFirewalledRequest(HttpServletRequest request) throws RequestRejectedException { + rejectedBlacklistedUrls(request); + + if (!isNormalized(request)) { + throw new RequestRejectedException("The request was rejected because the URL was not normalized."); + } + + String requestUri = request.getRequestURI(); + if (!containsOnlyPrintableAsciiCharacters(requestUri)) { + throw new RequestRejectedException("The requestURI was rejected because it can only contain printable ASCII characters."); + } + return new FirewalledRequest(request) { + @Override + public void reset() { + } + }; + } + + private void rejectedBlacklistedUrls(HttpServletRequest request) { + for (String forbidden : this.encodedUrlBlacklist) { + if (encodedUrlContains(request, forbidden)) { + throw new RequestRejectedException("The request was rejected because the URL contained a potentially malicious String \"" + forbidden + "\""); + } + } + for (String forbidden : this.decodedUrlBlacklist) { + if (decodedUrlContains(request, forbidden)) { + throw new RequestRejectedException("The request was rejected because the URL contained a potentially malicious String \"" + forbidden + "\""); + } + } + } + + @Override + public HttpServletResponse getFirewalledResponse(HttpServletResponse response) { + return new FirewalledResponse(response); + } + + private static boolean isNormalized(HttpServletRequest request) { + if (!isNormalized(request.getRequestURI())) { + return false; + } + if (!isNormalized(request.getContextPath())) { + return false; + } + if (!isNormalized(request.getServletPath())) { + return false; + } + if (!isNormalized(request.getPathInfo())) { + return false; + } + return true; + } + + private static boolean encodedUrlContains(HttpServletRequest request, String value) { + if (valueContains(request.getContextPath(), value)) { + return true; + } + return valueContains(request.getRequestURI(), value); + } + + private static boolean decodedUrlContains(HttpServletRequest request, String value) { + if (valueContains(request.getServletPath(), value)) { + return true; + } + if (valueContains(request.getPathInfo(), value)) { + return true; + } + return false; + } + + private static boolean containsOnlyPrintableAsciiCharacters(String uri) { + int length = uri.length(); + for (int i = 0; i < length; i++) { + char c = uri.charAt(i); + if (c < '\u0021' || '\u007e' < c) { + return false; + } + } + + return true; + } + + private static boolean valueContains(String value, String contains) { + return value != null && value.contains(contains); + } + + /** + * Checks whether a path is normalized (doesn't contain path traversal + * sequences like "./", "/../" or "/.") + * + * @param path + * the path to test + * @return true if the path doesn't contain any path-traversal character + * sequences. + */ + private static boolean isNormalized(String path) { + if (path == null) { + return true; + } + + if (path.indexOf("//") > 0) { + return false; + } + + for (int j = path.length(); j > 0;) { + int i = path.lastIndexOf('/', j - 1); + int gap = j - i; + + if (gap == 2 && path.charAt(i + 1) == '.') { + // ".", "/./" or "/." + return false; + } else if (gap == 3 && path.charAt(i + 1) == '.' && path.charAt(i + 2) == '.') { + return false; + } + + j = i; + } + + return true; + } + +}
web/src/test/java/org/springframework/security/web/firewall/StrictHttpFirewallTests.java+351 −0 added@@ -0,0 +1,351 @@ +/* + * Copyright 2012-2017 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.firewall; + +import org.junit.Test; +import org.springframework.mock.web.MockHttpServletRequest; + +import static org.assertj.core.api.Assertions.fail; + +/** + * @author Rob Winch + */ +public class StrictHttpFirewallTests { + public String[] unnormalizedPaths = { "/..", "/./path/", "/path/path/.", "/path/path//.", "./path/../path//.", + "./path", ".//path", ".", "/path//" }; + + private StrictHttpFirewall firewall = new StrictHttpFirewall(); + + private MockHttpServletRequest request = new MockHttpServletRequest(); + + @Test + public void getFirewalledRequestWhenRequestURINotNormalizedThenThrowsRequestRejectedException() throws Exception { + for (String path : this.unnormalizedPaths) { + this.request = new MockHttpServletRequest(); + this.request.setRequestURI(path); + try { + this.firewall.getFirewalledRequest(this.request); + fail(path + " is un-normalized"); + } catch (RequestRejectedException expected) { + } + } + } + + @Test + public void getFirewalledRequestWhenContextPathNotNormalizedThenThrowsRequestRejectedException() throws Exception { + for (String path : this.unnormalizedPaths) { + this.request = new MockHttpServletRequest(); + this.request.setContextPath(path); + try { + this.firewall.getFirewalledRequest(this.request); + fail(path + " is un-normalized"); + } catch (RequestRejectedException expected) { + } + } + } + + @Test + public void getFirewalledRequestWhenServletPathNotNormalizedThenThrowsRequestRejectedException() throws Exception { + for (String path : this.unnormalizedPaths) { + this.request = new MockHttpServletRequest(); + this.request.setServletPath(path); + try { + this.firewall.getFirewalledRequest(this.request); + fail(path + " is un-normalized"); + } catch (RequestRejectedException expected) { + } + } + } + + @Test + public void getFirewalledRequestWhenPathInfoNotNormalizedThenThrowsRequestRejectedException() throws Exception { + for (String path : this.unnormalizedPaths) { + this.request = new MockHttpServletRequest(); + this.request.setPathInfo(path); + try { + this.firewall.getFirewalledRequest(this.request); + fail(path + " is un-normalized"); + } catch (RequestRejectedException expected) { + } + } + } + + // --- ; --- + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenSemicolonInContextPathThenThrowsRequestRejectedException() { + this.request.setContextPath(";/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenSemicolonInServletPathThenThrowsRequestRejectedException() { + this.request.setServletPath("/spring;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenSemicolonInPathInfoThenThrowsRequestRejectedException() { + this.request.setPathInfo("/path;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenSemicolonInRequestUriThenThrowsRequestRejectedException() { + this.request.setRequestURI("/path;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedSemicolonInContextPathThenThrowsRequestRejectedException() { + this.request.setContextPath("%3B/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedSemicolonInServletPathThenThrowsRequestRejectedException() { + this.request.setServletPath("/spring%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedSemicolonInPathInfoThenThrowsRequestRejectedException() { + this.request.setPathInfo("/path%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedSemicolonInRequestUriThenThrowsRequestRejectedException() { + this.request.setRequestURI("/path%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInContextPathThenThrowsRequestRejectedException() { + this.request.setContextPath("%3b/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInServletPathThenThrowsRequestRejectedException() { + this.request.setServletPath("/spring%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInPathInfoThenThrowsRequestRejectedException() { + this.request.setPathInfo("/path%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInRequestUriThenThrowsRequestRejectedException() { + this.request.setRequestURI("/path%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenSemicolonInContextPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setContextPath(";/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenSemicolonInServletPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setServletPath("/spring;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenSemicolonInPathInfoAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setPathInfo("/path;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenSemicolonInRequestUriAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setRequestURI("/path;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenEncodedSemicolonInContextPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setContextPath("%3B/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenEncodedSemicolonInServletPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setServletPath("/spring%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenEncodedSemicolonInPathInfoAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setPathInfo("/path%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenEncodedSemicolonInRequestUriAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setRequestURI("/path%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInContextPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setContextPath("%3b/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInServletPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setServletPath("/spring%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInPathInfoAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setPathInfo("/path%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInRequestUriAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setRequestURI("/path%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + // --- encoded . --- + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedPeriodInThenThrowsRequestRejectedException() { + this.request.setRequestURI("/%2E/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedPeriodInThenThrowsRequestRejectedException() { + this.request.setRequestURI("/%2e/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenAllowEncodedPeriodAndEncodedPeriodInThenNoException() { + this.firewall.setAllowUrlEncodedPeriod(true); + this.request.setRequestURI("/%2E/"); + + this.firewall.getFirewalledRequest(this.request); + } + + // --- from DefaultHttpFirewallTests --- + + /** + * On WebSphere 8.5 a URL like /context-root/a/b;%2f1/c can bypass a rule on + * /a/b/c because the pathInfo is /a/b;/1/c which ends up being /a/b/1/c + * while Spring MVC will strip the ; content from requestURI before the path + * is URL decoded. + */ + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedPathThenException() { + this.request.setRequestURI("/context-root/a/b;%2f1/c"); + this.request.setContextPath("/context-root"); + this.request.setServletPath(""); + this.request.setPathInfo("/a/b;/1/c"); // URL decoded requestURI + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenUppercaseEncodedPathThenException() { + this.request.setRequestURI("/context-root/a/b;%2F1/c"); + this.request.setContextPath("/context-root"); + this.request.setServletPath(""); + this.request.setPathInfo("/a/b;/1/c"); // URL decoded requestURI + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenAllowUrlEncodedSlashAndLowercaseEncodedPathThenNoException() { + this.firewall.setAllowUrlEncodedSlash(true); + this.firewall.setAllowSemicolon(true); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setRequestURI("/context-root/a/b;%2f1/c"); + request.setContextPath("/context-root"); + request.setServletPath(""); + request.setPathInfo("/a/b;/1/c"); // URL decoded requestURI + + this.firewall.getFirewalledRequest(request); + } + + @Test + public void getFirewalledRequestWhenAllowUrlEncodedSlashAndUppercaseEncodedPathThenNoException() { + this.firewall.setAllowUrlEncodedSlash(true); + this.firewall.setAllowSemicolon(true); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setRequestURI("/context-root/a/b;%2F1/c"); + request.setContextPath("/context-root"); + request.setServletPath(""); + request.setPathInfo("/a/b;/1/c"); // URL decoded requestURI + + this.firewall.getFirewalledRequest(request); + } +}
cb8041ba6763Add StrictHttpFirewall
7 files changed · +610 −12
config/src/main/java/org/springframework/security/config/annotation/web/builders/WebSecurity.java+2 −2 modified@@ -47,7 +47,7 @@ import org.springframework.security.web.access.expression.DefaultWebSecurityExpressionHandler; import org.springframework.security.web.access.intercept.FilterSecurityInterceptor; import org.springframework.security.web.debug.DebugFilter; -import org.springframework.security.web.firewall.DefaultHttpFirewall; +import org.springframework.security.web.firewall.StrictHttpFirewall; import org.springframework.security.web.firewall.HttpFirewall; import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; @@ -159,7 +159,7 @@ public IgnoredRequestConfigurer ignoring() { /** * Allows customizing the {@link HttpFirewall}. The default is - * {@link DefaultHttpFirewall}. + * {@link StrictHttpFirewall}. * * @param httpFirewall the custom {@link HttpFirewall} * @return the {@link WebSecurity} for further customizations
config/src/test/java/org/springframework/security/config/FilterChainProxyConfigTests.java+4 −0 modified@@ -38,6 +38,7 @@ import org.springframework.security.web.SecurityFilterChain; import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter; import org.springframework.security.web.context.SecurityContextPersistenceFilter; +import org.springframework.security.web.firewall.DefaultHttpFirewall; import org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.AnyRequestMatcher; @@ -80,6 +81,7 @@ public void normalOperation() throws Exception { public void normalOperationWithNewConfig() throws Exception { FilterChainProxy filterChainProxy = appCtx.getBean("newFilterChainProxy", FilterChainProxy.class); + filterChainProxy.setFirewall(new DefaultHttpFirewall()); checkPathAndFilterOrder(filterChainProxy); doNormalOperation(filterChainProxy); } @@ -88,6 +90,7 @@ public void normalOperationWithNewConfig() throws Exception { public void normalOperationWithNewConfigRegex() throws Exception { FilterChainProxy filterChainProxy = appCtx.getBean("newFilterChainProxyRegex", FilterChainProxy.class); + filterChainProxy.setFirewall(new DefaultHttpFirewall()); checkPathAndFilterOrder(filterChainProxy); doNormalOperation(filterChainProxy); } @@ -96,6 +99,7 @@ public void normalOperationWithNewConfigRegex() throws Exception { public void normalOperationWithNewConfigNonNamespace() throws Exception { FilterChainProxy filterChainProxy = appCtx.getBean( "newFilterChainProxyNonNamespace", FilterChainProxy.class); + filterChainProxy.setFirewall(new DefaultHttpFirewall()); checkPathAndFilterOrder(filterChainProxy); doNormalOperation(filterChainProxy); }
itest/context/src/integration-test/java/org/springframework/security/integration/HttpPathParameterStrippingTests.java+9 −7 modified@@ -28,6 +28,7 @@ import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.web.FilterChainProxy; import org.springframework.security.web.context.HttpSessionSecurityContextRepository; +import org.springframework.security.web.firewall.RequestRejectedException; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; @@ -40,32 +41,33 @@ public class HttpPathParameterStrippingTests { @Autowired private FilterChainProxy fcp; - @Test + @Test(expected = RequestRejectedException.class) public void securedFilterChainCannotBeBypassedByAddingPathParameters() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); request.setPathInfo("/secured;x=y/admin.html"); request.setSession(createAuthenticatedSession("ROLE_USER")); MockHttpServletResponse response = new MockHttpServletResponse(); fcp.doFilter(request, response, new MockFilterChain()); - assertThat(response.getStatus()).isEqualTo(403); } - @Test + @Test(expected = RequestRejectedException.class) public void adminFilePatternCannotBeBypassedByAddingPathParameters() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); request.setServletPath("/secured/admin.html;x=user.html"); request.setSession(createAuthenticatedSession("ROLE_USER")); MockHttpServletResponse response = new MockHttpServletResponse(); fcp.doFilter(request, response, new MockFilterChain()); - assertThat(response.getStatus()).isEqualTo(403); + } - // Try with pathInfo - request = new MockHttpServletRequest(); + + @Test(expected = RequestRejectedException.class) + public void adminFilePatternCannotBeBypassedByAddingPathParametersWithPathInfo() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest(); request.setServletPath("/secured"); request.setPathInfo("/admin.html;x=user.html"); request.setSession(createAuthenticatedSession("ROLE_USER")); - response = new MockHttpServletResponse(); + MockHttpServletResponse response = new MockHttpServletResponse(); fcp.doFilter(request, response, new MockFilterChain()); assertThat(response.getStatus()).isEqualTo(403); }
web/src/main/java/org/springframework/security/web/FilterChainProxy.java+3 −3 modified@@ -19,9 +19,9 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.security.core.context.SecurityContextHolder; -import org.springframework.security.web.firewall.DefaultHttpFirewall; import org.springframework.security.web.firewall.FirewalledRequest; import org.springframework.security.web.firewall.HttpFirewall; +import org.springframework.security.web.firewall.StrictHttpFirewall; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.security.web.util.UrlUtils; import org.springframework.web.filter.DelegatingFilterProxy; @@ -96,7 +96,7 @@ * * An {@link HttpFirewall} instance is used to validate incoming requests and create a * wrapped request which provides consistent path values for matching against. See - * {@link DefaultHttpFirewall}, for more information on the type of attacks which the + * {@link StrictHttpFirewall}, for more information on the type of attacks which the * default implementation protects against. A custom implementation can be injected to * provide stricter control over the request contents or if an application needs to * support certain types of request which are rejected by default. @@ -147,7 +147,7 @@ public class FilterChainProxy extends GenericFilterBean { private FilterChainValidator filterChainValidator = new NullFilterChainValidator(); - private HttpFirewall firewall = new DefaultHttpFirewall(); + private HttpFirewall firewall = new StrictHttpFirewall(); // ~ Methods // ========================================================================================================
web/src/main/java/org/springframework/security/web/firewall/DefaultHttpFirewall.java+2 −0 modified@@ -37,8 +37,10 @@ * containers normalize the paths before performing the servlet-mapping, but * again this is not guaranteed by the servlet spec. * + * @deprecated Use {@link StrictHttpFirewall} instead * @author Luke Taylor */ +@Deprecated public class DefaultHttpFirewall implements HttpFirewall { private boolean allowUrlEncodedSlash;
web/src/main/java/org/springframework/security/web/firewall/StrictHttpFirewall.java+239 −0 added@@ -0,0 +1,239 @@ +/* + * Copyright 2012-2017 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.firewall; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +/** + * A strict implementation of {@link HttpFirewall} that rejects any suspicious requests + * with a {@link RequestRejectedException}. + * + * @author Rob Winch + * @since 5.0.1 + */ +public class StrictHttpFirewall implements HttpFirewall { + private static final String ENCODED_PERCENT = "%25"; + + private static final String PERCENT = "%"; + + private static final List<String> FORBIDDEN_ENCODED_PERIOD = Collections.unmodifiableList(Arrays.asList("%2e", "%2E")); + + private static final List<String> FORBIDDEN_SEMICOLON = Collections.unmodifiableList(Arrays.asList(";", "%3b", "%3B")); + + private static final List<String> FORBIDDEN_FORWARDSLASH = Collections.unmodifiableList(Arrays.asList("%2f", "%2F")); + + private static final List<String> FORBIDDEN_BACKSLASH = Collections.unmodifiableList(Arrays.asList("\\", "%5c", "%5C")); + + private Set<String> encodedUrlBlacklist = new HashSet<String>(); + + private Set<String> decodedUrlBlacklist = new HashSet<String>(); + + public StrictHttpFirewall() { + urlBlacklistsAddAll(FORBIDDEN_SEMICOLON); + urlBlacklistsAddAll(FORBIDDEN_FORWARDSLASH); + urlBlacklistsAddAll(FORBIDDEN_BACKSLASH); + + this.encodedUrlBlacklist.add(ENCODED_PERCENT); + this.encodedUrlBlacklist.addAll(FORBIDDEN_ENCODED_PERIOD); + this.decodedUrlBlacklist.add(PERCENT); + } + + /** + * + * @param allowSemicolon + */ + public void setAllowSemicolon(boolean allowSemicolon) { + if (allowSemicolon) { + urlBlacklistsRemoveAll(FORBIDDEN_SEMICOLON); + } else { + urlBlacklistsAddAll(FORBIDDEN_SEMICOLON); + } + } + + public void setAllowUrlEncodedSlash(boolean allowUrlEncodedSlash) { + if (allowUrlEncodedSlash) { + urlBlacklistsRemoveAll(FORBIDDEN_FORWARDSLASH); + } else { + urlBlacklistsAddAll(FORBIDDEN_FORWARDSLASH); + } + } + + public void setAllowUrlEncodedPeriod(boolean allowUrlEncodedPeriod) { + if (allowUrlEncodedPeriod) { + this.encodedUrlBlacklist.removeAll(FORBIDDEN_ENCODED_PERIOD); + } else { + this.encodedUrlBlacklist.addAll(FORBIDDEN_ENCODED_PERIOD); + } + } + + public void setAllowBackSlash(boolean allowBackSlash) { + if (allowBackSlash) { + urlBlacklistsRemoveAll(FORBIDDEN_BACKSLASH); + } else { + urlBlacklistsAddAll(FORBIDDEN_BACKSLASH); + } + } + + public void setAllowUrlEncodedPercent(boolean allowUrlEncodedPercent) { + if (allowUrlEncodedPercent) { + this.encodedUrlBlacklist.remove(ENCODED_PERCENT); + this.decodedUrlBlacklist.remove(PERCENT); + } else { + this.encodedUrlBlacklist.add(ENCODED_PERCENT); + this.decodedUrlBlacklist.add(PERCENT); + } + } + + private void urlBlacklistsAddAll(Collection<String> values) { + this.encodedUrlBlacklist.addAll(values); + this.decodedUrlBlacklist.addAll(values); + } + + private void urlBlacklistsRemoveAll(Collection<String> values) { + this.encodedUrlBlacklist.removeAll(values); + this.decodedUrlBlacklist.removeAll(values); + } + + @Override + public FirewalledRequest getFirewalledRequest(HttpServletRequest request) throws RequestRejectedException { + rejectedBlacklistedUrls(request); + + if (!isNormalized(request)) { + throw new RequestRejectedException("The request was rejected because the URL was not normalized."); + } + + String requestUri = request.getRequestURI(); + if (!containsOnlyPrintableAsciiCharacters(requestUri)) { + throw new RequestRejectedException("The requestURI was rejected because it can only contain printable ASCII characters."); + } + return new FirewalledRequest(request) { + @Override + public void reset() { + } + }; + } + + private void rejectedBlacklistedUrls(HttpServletRequest request) { + for (String forbidden : this.encodedUrlBlacklist) { + if (encodedUrlContains(request, forbidden)) { + throw new RequestRejectedException("The request was rejected because the URL contained a potentially malicious String \"" + forbidden + "\""); + } + } + for (String forbidden : this.decodedUrlBlacklist) { + if (decodedUrlContains(request, forbidden)) { + throw new RequestRejectedException("The request was rejected because the URL contained a potentially malicious String \"" + forbidden + "\""); + } + } + } + + @Override + public HttpServletResponse getFirewalledResponse(HttpServletResponse response) { + return new FirewalledResponse(response); + } + + private static boolean isNormalized(HttpServletRequest request) { + if (!isNormalized(request.getRequestURI())) { + return false; + } + if (!isNormalized(request.getContextPath())) { + return false; + } + if (!isNormalized(request.getServletPath())) { + return false; + } + if (!isNormalized(request.getPathInfo())) { + return false; + } + return true; + } + + private static boolean encodedUrlContains(HttpServletRequest request, String value) { + if (valueContains(request.getContextPath(), value)) { + return true; + } + return valueContains(request.getRequestURI(), value); + } + + private static boolean decodedUrlContains(HttpServletRequest request, String value) { + if (valueContains(request.getServletPath(), value)) { + return true; + } + if (valueContains(request.getPathInfo(), value)) { + return true; + } + return false; + } + + private static boolean containsOnlyPrintableAsciiCharacters(String uri) { + int length = uri.length(); + for (int i = 0; i < length; i++) { + char c = uri.charAt(i); + if (c < '\u0021' || '\u007e' < c) { + return false; + } + } + + return true; + } + + private static boolean valueContains(String value, String contains) { + return value != null && value.contains(contains); + } + + /** + * Checks whether a path is normalized (doesn't contain path traversal + * sequences like "./", "/../" or "/.") + * + * @param path + * the path to test + * @return true if the path doesn't contain any path-traversal character + * sequences. + */ + private static boolean isNormalized(String path) { + if (path == null) { + return true; + } + + if (path.indexOf("//") > 0) { + return false; + } + + for (int j = path.length(); j > 0;) { + int i = path.lastIndexOf('/', j - 1); + int gap = j - i; + + if (gap == 2 && path.charAt(i + 1) == '.') { + // ".", "/./" or "/." + return false; + } else if (gap == 3 && path.charAt(i + 1) == '.' && path.charAt(i + 2) == '.') { + return false; + } + + j = i; + } + + return true; + } + +}
web/src/test/java/org/springframework/security/web/firewall/StrictHttpFirewallTests.java+351 −0 added@@ -0,0 +1,351 @@ +/* + * Copyright 2012-2017 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.firewall; + +import org.junit.Test; +import org.springframework.mock.web.MockHttpServletRequest; + +import static org.assertj.core.api.Assertions.fail; + +/** + * @author Rob Winch + */ +public class StrictHttpFirewallTests { + public String[] unnormalizedPaths = { "/..", "/./path/", "/path/path/.", "/path/path//.", "./path/../path//.", + "./path", ".//path", ".", "/path//" }; + + private StrictHttpFirewall firewall = new StrictHttpFirewall(); + + private MockHttpServletRequest request = new MockHttpServletRequest(); + + @Test + public void getFirewalledRequestWhenRequestURINotNormalizedThenThrowsRequestRejectedException() throws Exception { + for (String path : this.unnormalizedPaths) { + this.request = new MockHttpServletRequest(); + this.request.setRequestURI(path); + try { + this.firewall.getFirewalledRequest(this.request); + fail(path + " is un-normalized"); + } catch (RequestRejectedException expected) { + } + } + } + + @Test + public void getFirewalledRequestWhenContextPathNotNormalizedThenThrowsRequestRejectedException() throws Exception { + for (String path : this.unnormalizedPaths) { + this.request = new MockHttpServletRequest(); + this.request.setContextPath(path); + try { + this.firewall.getFirewalledRequest(this.request); + fail(path + " is un-normalized"); + } catch (RequestRejectedException expected) { + } + } + } + + @Test + public void getFirewalledRequestWhenServletPathNotNormalizedThenThrowsRequestRejectedException() throws Exception { + for (String path : this.unnormalizedPaths) { + this.request = new MockHttpServletRequest(); + this.request.setServletPath(path); + try { + this.firewall.getFirewalledRequest(this.request); + fail(path + " is un-normalized"); + } catch (RequestRejectedException expected) { + } + } + } + + @Test + public void getFirewalledRequestWhenPathInfoNotNormalizedThenThrowsRequestRejectedException() throws Exception { + for (String path : this.unnormalizedPaths) { + this.request = new MockHttpServletRequest(); + this.request.setPathInfo(path); + try { + this.firewall.getFirewalledRequest(this.request); + fail(path + " is un-normalized"); + } catch (RequestRejectedException expected) { + } + } + } + + // --- ; --- + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenSemicolonInContextPathThenThrowsRequestRejectedException() { + this.request.setContextPath(";/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenSemicolonInServletPathThenThrowsRequestRejectedException() { + this.request.setServletPath("/spring;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenSemicolonInPathInfoThenThrowsRequestRejectedException() { + this.request.setPathInfo("/path;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenSemicolonInRequestUriThenThrowsRequestRejectedException() { + this.request.setRequestURI("/path;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedSemicolonInContextPathThenThrowsRequestRejectedException() { + this.request.setContextPath("%3B/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedSemicolonInServletPathThenThrowsRequestRejectedException() { + this.request.setServletPath("/spring%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedSemicolonInPathInfoThenThrowsRequestRejectedException() { + this.request.setPathInfo("/path%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedSemicolonInRequestUriThenThrowsRequestRejectedException() { + this.request.setRequestURI("/path%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInContextPathThenThrowsRequestRejectedException() { + this.request.setContextPath("%3b/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInServletPathThenThrowsRequestRejectedException() { + this.request.setServletPath("/spring%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInPathInfoThenThrowsRequestRejectedException() { + this.request.setPathInfo("/path%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInRequestUriThenThrowsRequestRejectedException() { + this.request.setRequestURI("/path%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenSemicolonInContextPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setContextPath(";/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenSemicolonInServletPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setServletPath("/spring;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenSemicolonInPathInfoAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setPathInfo("/path;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenSemicolonInRequestUriAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setRequestURI("/path;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenEncodedSemicolonInContextPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setContextPath("%3B/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenEncodedSemicolonInServletPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setServletPath("/spring%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenEncodedSemicolonInPathInfoAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setPathInfo("/path%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenEncodedSemicolonInRequestUriAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setRequestURI("/path%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInContextPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setContextPath("%3b/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInServletPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setServletPath("/spring%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInPathInfoAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setPathInfo("/path%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInRequestUriAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setRequestURI("/path%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + // --- encoded . --- + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedPeriodInThenThrowsRequestRejectedException() { + this.request.setRequestURI("/%2E/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedPeriodInThenThrowsRequestRejectedException() { + this.request.setRequestURI("/%2e/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenAllowEncodedPeriodAndEncodedPeriodInThenNoException() { + this.firewall.setAllowUrlEncodedPeriod(true); + this.request.setRequestURI("/%2E/"); + + this.firewall.getFirewalledRequest(this.request); + } + + // --- from DefaultHttpFirewallTests --- + + /** + * On WebSphere 8.5 a URL like /context-root/a/b;%2f1/c can bypass a rule on + * /a/b/c because the pathInfo is /a/b;/1/c which ends up being /a/b/1/c + * while Spring MVC will strip the ; content from requestURI before the path + * is URL decoded. + */ + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedPathThenException() { + this.request.setRequestURI("/context-root/a/b;%2f1/c"); + this.request.setContextPath("/context-root"); + this.request.setServletPath(""); + this.request.setPathInfo("/a/b;/1/c"); // URL decoded requestURI + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenUppercaseEncodedPathThenException() { + this.request.setRequestURI("/context-root/a/b;%2F1/c"); + this.request.setContextPath("/context-root"); + this.request.setServletPath(""); + this.request.setPathInfo("/a/b;/1/c"); // URL decoded requestURI + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenAllowUrlEncodedSlashAndLowercaseEncodedPathThenNoException() { + this.firewall.setAllowUrlEncodedSlash(true); + this.firewall.setAllowSemicolon(true); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setRequestURI("/context-root/a/b;%2f1/c"); + request.setContextPath("/context-root"); + request.setServletPath(""); + request.setPathInfo("/a/b;/1/c"); // URL decoded requestURI + + this.firewall.getFirewalledRequest(request); + } + + @Test + public void getFirewalledRequestWhenAllowUrlEncodedSlashAndUppercaseEncodedPathThenNoException() { + this.firewall.setAllowUrlEncodedSlash(true); + this.firewall.setAllowSemicolon(true); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setRequestURI("/context-root/a/b;%2F1/c"); + request.setContextPath("/context-root"); + request.setServletPath(""); + request.setPathInfo("/a/b;/1/c"); // URL decoded requestURI + + this.firewall.getFirewalledRequest(request); + } +}
65da28e4bf62Add StrictHttpFirewall
7 files changed · +610 −12
config/src/main/java/org/springframework/security/config/annotation/web/builders/WebSecurity.java+2 −2 modified@@ -47,7 +47,7 @@ import org.springframework.security.web.access.expression.DefaultWebSecurityExpressionHandler; import org.springframework.security.web.access.intercept.FilterSecurityInterceptor; import org.springframework.security.web.debug.DebugFilter; -import org.springframework.security.web.firewall.DefaultHttpFirewall; +import org.springframework.security.web.firewall.StrictHttpFirewall; import org.springframework.security.web.firewall.HttpFirewall; import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; @@ -159,7 +159,7 @@ public IgnoredRequestConfigurer ignoring() { /** * Allows customizing the {@link HttpFirewall}. The default is - * {@link DefaultHttpFirewall}. + * {@link StrictHttpFirewall}. * * @param httpFirewall the custom {@link HttpFirewall} * @return the {@link WebSecurity} for further customizations
config/src/test/java/org/springframework/security/config/FilterChainProxyConfigTests.java+4 −0 modified@@ -38,6 +38,7 @@ import org.springframework.security.web.SecurityFilterChain; import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter; import org.springframework.security.web.context.SecurityContextPersistenceFilter; +import org.springframework.security.web.firewall.DefaultHttpFirewall; import org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.AnyRequestMatcher; @@ -80,6 +81,7 @@ public void normalOperation() throws Exception { public void normalOperationWithNewConfig() throws Exception { FilterChainProxy filterChainProxy = appCtx.getBean("newFilterChainProxy", FilterChainProxy.class); + filterChainProxy.setFirewall(new DefaultHttpFirewall()); checkPathAndFilterOrder(filterChainProxy); doNormalOperation(filterChainProxy); } @@ -88,6 +90,7 @@ public void normalOperationWithNewConfig() throws Exception { public void normalOperationWithNewConfigRegex() throws Exception { FilterChainProxy filterChainProxy = appCtx.getBean("newFilterChainProxyRegex", FilterChainProxy.class); + filterChainProxy.setFirewall(new DefaultHttpFirewall()); checkPathAndFilterOrder(filterChainProxy); doNormalOperation(filterChainProxy); } @@ -96,6 +99,7 @@ public void normalOperationWithNewConfigRegex() throws Exception { public void normalOperationWithNewConfigNonNamespace() throws Exception { FilterChainProxy filterChainProxy = appCtx.getBean( "newFilterChainProxyNonNamespace", FilterChainProxy.class); + filterChainProxy.setFirewall(new DefaultHttpFirewall()); checkPathAndFilterOrder(filterChainProxy); doNormalOperation(filterChainProxy); }
itest/context/src/integration-test/java/org/springframework/security/integration/HttpPathParameterStrippingTests.java+9 −7 modified@@ -28,6 +28,7 @@ import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.web.FilterChainProxy; import org.springframework.security.web.context.HttpSessionSecurityContextRepository; +import org.springframework.security.web.firewall.RequestRejectedException; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; @@ -40,32 +41,33 @@ public class HttpPathParameterStrippingTests { @Autowired private FilterChainProxy fcp; - @Test + @Test(expected = RequestRejectedException.class) public void securedFilterChainCannotBeBypassedByAddingPathParameters() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); request.setPathInfo("/secured;x=y/admin.html"); request.setSession(createAuthenticatedSession("ROLE_USER")); MockHttpServletResponse response = new MockHttpServletResponse(); fcp.doFilter(request, response, new MockFilterChain()); - assertThat(response.getStatus()).isEqualTo(403); } - @Test + @Test(expected = RequestRejectedException.class) public void adminFilePatternCannotBeBypassedByAddingPathParameters() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); request.setServletPath("/secured/admin.html;x=user.html"); request.setSession(createAuthenticatedSession("ROLE_USER")); MockHttpServletResponse response = new MockHttpServletResponse(); fcp.doFilter(request, response, new MockFilterChain()); - assertThat(response.getStatus()).isEqualTo(403); + } - // Try with pathInfo - request = new MockHttpServletRequest(); + + @Test(expected = RequestRejectedException.class) + public void adminFilePatternCannotBeBypassedByAddingPathParametersWithPathInfo() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest(); request.setServletPath("/secured"); request.setPathInfo("/admin.html;x=user.html"); request.setSession(createAuthenticatedSession("ROLE_USER")); - response = new MockHttpServletResponse(); + MockHttpServletResponse response = new MockHttpServletResponse(); fcp.doFilter(request, response, new MockFilterChain()); assertThat(response.getStatus()).isEqualTo(403); }
web/src/main/java/org/springframework/security/web/FilterChainProxy.java+3 −3 modified@@ -19,9 +19,9 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.security.core.context.SecurityContextHolder; -import org.springframework.security.web.firewall.DefaultHttpFirewall; import org.springframework.security.web.firewall.FirewalledRequest; import org.springframework.security.web.firewall.HttpFirewall; +import org.springframework.security.web.firewall.StrictHttpFirewall; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.security.web.util.UrlUtils; import org.springframework.web.filter.DelegatingFilterProxy; @@ -96,7 +96,7 @@ * * An {@link HttpFirewall} instance is used to validate incoming requests and create a * wrapped request which provides consistent path values for matching against. See - * {@link DefaultHttpFirewall}, for more information on the type of attacks which the + * {@link StrictHttpFirewall}, for more information on the type of attacks which the * default implementation protects against. A custom implementation can be injected to * provide stricter control over the request contents or if an application needs to * support certain types of request which are rejected by default. @@ -147,7 +147,7 @@ public class FilterChainProxy extends GenericFilterBean { private FilterChainValidator filterChainValidator = new NullFilterChainValidator(); - private HttpFirewall firewall = new DefaultHttpFirewall(); + private HttpFirewall firewall = new StrictHttpFirewall(); // ~ Methods // ========================================================================================================
web/src/main/java/org/springframework/security/web/firewall/DefaultHttpFirewall.java+2 −0 modified@@ -37,8 +37,10 @@ * containers normalize the paths before performing the servlet-mapping, but * again this is not guaranteed by the servlet spec. * + * @deprecated Use {@link StrictHttpFirewall} instead * @author Luke Taylor */ +@Deprecated public class DefaultHttpFirewall implements HttpFirewall { private boolean allowUrlEncodedSlash;
web/src/main/java/org/springframework/security/web/firewall/StrictHttpFirewall.java+239 −0 added@@ -0,0 +1,239 @@ +/* + * Copyright 2012-2017 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.firewall; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +/** + * A strict implementation of {@link HttpFirewall} that rejects any suspicious requests + * with a {@link RequestRejectedException}. + * + * @author Rob Winch + * @since 5.0.1 + */ +public class StrictHttpFirewall implements HttpFirewall { + private static final String ENCODED_PERCENT = "%25"; + + private static final String PERCENT = "%"; + + private static final List<String> FORBIDDEN_ENCODED_PERIOD = Collections.unmodifiableList(Arrays.asList("%2e", "%2E")); + + private static final List<String> FORBIDDEN_SEMICOLON = Collections.unmodifiableList(Arrays.asList(";", "%3b", "%3B")); + + private static final List<String> FORBIDDEN_FORWARDSLASH = Collections.unmodifiableList(Arrays.asList("%2f", "%2F")); + + private static final List<String> FORBIDDEN_BACKSLASH = Collections.unmodifiableList(Arrays.asList("\\", "%5c", "%5C")); + + private Set<String> encodedUrlBlacklist = new HashSet<String>(); + + private Set<String> decodedUrlBlacklist = new HashSet<String>(); + + public StrictHttpFirewall() { + urlBlacklistsAddAll(FORBIDDEN_SEMICOLON); + urlBlacklistsAddAll(FORBIDDEN_FORWARDSLASH); + urlBlacklistsAddAll(FORBIDDEN_BACKSLASH); + + this.encodedUrlBlacklist.add(ENCODED_PERCENT); + this.encodedUrlBlacklist.addAll(FORBIDDEN_ENCODED_PERIOD); + this.decodedUrlBlacklist.add(PERCENT); + } + + /** + * + * @param allowSemicolon + */ + public void setAllowSemicolon(boolean allowSemicolon) { + if (allowSemicolon) { + urlBlacklistsRemoveAll(FORBIDDEN_SEMICOLON); + } else { + urlBlacklistsAddAll(FORBIDDEN_SEMICOLON); + } + } + + public void setAllowUrlEncodedSlash(boolean allowUrlEncodedSlash) { + if (allowUrlEncodedSlash) { + urlBlacklistsRemoveAll(FORBIDDEN_FORWARDSLASH); + } else { + urlBlacklistsAddAll(FORBIDDEN_FORWARDSLASH); + } + } + + public void setAllowUrlEncodedPeriod(boolean allowUrlEncodedPeriod) { + if (allowUrlEncodedPeriod) { + this.encodedUrlBlacklist.removeAll(FORBIDDEN_ENCODED_PERIOD); + } else { + this.encodedUrlBlacklist.addAll(FORBIDDEN_ENCODED_PERIOD); + } + } + + public void setAllowBackSlash(boolean allowBackSlash) { + if (allowBackSlash) { + urlBlacklistsRemoveAll(FORBIDDEN_BACKSLASH); + } else { + urlBlacklistsAddAll(FORBIDDEN_BACKSLASH); + } + } + + public void setAllowUrlEncodedPercent(boolean allowUrlEncodedPercent) { + if (allowUrlEncodedPercent) { + this.encodedUrlBlacklist.remove(ENCODED_PERCENT); + this.decodedUrlBlacklist.remove(PERCENT); + } else { + this.encodedUrlBlacklist.add(ENCODED_PERCENT); + this.decodedUrlBlacklist.add(PERCENT); + } + } + + private void urlBlacklistsAddAll(Collection<String> values) { + this.encodedUrlBlacklist.addAll(values); + this.decodedUrlBlacklist.addAll(values); + } + + private void urlBlacklistsRemoveAll(Collection<String> values) { + this.encodedUrlBlacklist.removeAll(values); + this.decodedUrlBlacklist.removeAll(values); + } + + @Override + public FirewalledRequest getFirewalledRequest(HttpServletRequest request) throws RequestRejectedException { + rejectedBlacklistedUrls(request); + + if (!isNormalized(request)) { + throw new RequestRejectedException("The request was rejected because the URL was not normalized."); + } + + String requestUri = request.getRequestURI(); + if (!containsOnlyPrintableAsciiCharacters(requestUri)) { + throw new RequestRejectedException("The requestURI was rejected because it can only contain printable ASCII characters."); + } + return new FirewalledRequest(request) { + @Override + public void reset() { + } + }; + } + + private void rejectedBlacklistedUrls(HttpServletRequest request) { + for (String forbidden : this.encodedUrlBlacklist) { + if (encodedUrlContains(request, forbidden)) { + throw new RequestRejectedException("The request was rejected because the URL contained a potentially malicious String \"" + forbidden + "\""); + } + } + for (String forbidden : this.decodedUrlBlacklist) { + if (decodedUrlContains(request, forbidden)) { + throw new RequestRejectedException("The request was rejected because the URL contained a potentially malicious String \"" + forbidden + "\""); + } + } + } + + @Override + public HttpServletResponse getFirewalledResponse(HttpServletResponse response) { + return new FirewalledResponse(response); + } + + private static boolean isNormalized(HttpServletRequest request) { + if (!isNormalized(request.getRequestURI())) { + return false; + } + if (!isNormalized(request.getContextPath())) { + return false; + } + if (!isNormalized(request.getServletPath())) { + return false; + } + if (!isNormalized(request.getPathInfo())) { + return false; + } + return true; + } + + private static boolean encodedUrlContains(HttpServletRequest request, String value) { + if (valueContains(request.getContextPath(), value)) { + return true; + } + return valueContains(request.getRequestURI(), value); + } + + private static boolean decodedUrlContains(HttpServletRequest request, String value) { + if (valueContains(request.getServletPath(), value)) { + return true; + } + if (valueContains(request.getPathInfo(), value)) { + return true; + } + return false; + } + + private static boolean containsOnlyPrintableAsciiCharacters(String uri) { + int length = uri.length(); + for (int i = 0; i < length; i++) { + char c = uri.charAt(i); + if (c < '\u0021' || '\u007e' < c) { + return false; + } + } + + return true; + } + + private static boolean valueContains(String value, String contains) { + return value != null && value.contains(contains); + } + + /** + * Checks whether a path is normalized (doesn't contain path traversal + * sequences like "./", "/../" or "/.") + * + * @param path + * the path to test + * @return true if the path doesn't contain any path-traversal character + * sequences. + */ + private static boolean isNormalized(String path) { + if (path == null) { + return true; + } + + if (path.indexOf("//") > 0) { + return false; + } + + for (int j = path.length(); j > 0;) { + int i = path.lastIndexOf('/', j - 1); + int gap = j - i; + + if (gap == 2 && path.charAt(i + 1) == '.') { + // ".", "/./" or "/." + return false; + } else if (gap == 3 && path.charAt(i + 1) == '.' && path.charAt(i + 2) == '.') { + return false; + } + + j = i; + } + + return true; + } + +}
web/src/test/java/org/springframework/security/web/firewall/StrictHttpFirewallTests.java+351 −0 added@@ -0,0 +1,351 @@ +/* + * Copyright 2012-2017 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.firewall; + +import org.junit.Test; +import org.springframework.mock.web.MockHttpServletRequest; + +import static org.assertj.core.api.Assertions.fail; + +/** + * @author Rob Winch + */ +public class StrictHttpFirewallTests { + public String[] unnormalizedPaths = { "/..", "/./path/", "/path/path/.", "/path/path//.", "./path/../path//.", + "./path", ".//path", ".", "/path//" }; + + private StrictHttpFirewall firewall = new StrictHttpFirewall(); + + private MockHttpServletRequest request = new MockHttpServletRequest(); + + @Test + public void getFirewalledRequestWhenRequestURINotNormalizedThenThrowsRequestRejectedException() throws Exception { + for (String path : this.unnormalizedPaths) { + this.request = new MockHttpServletRequest(); + this.request.setRequestURI(path); + try { + this.firewall.getFirewalledRequest(this.request); + fail(path + " is un-normalized"); + } catch (RequestRejectedException expected) { + } + } + } + + @Test + public void getFirewalledRequestWhenContextPathNotNormalizedThenThrowsRequestRejectedException() throws Exception { + for (String path : this.unnormalizedPaths) { + this.request = new MockHttpServletRequest(); + this.request.setContextPath(path); + try { + this.firewall.getFirewalledRequest(this.request); + fail(path + " is un-normalized"); + } catch (RequestRejectedException expected) { + } + } + } + + @Test + public void getFirewalledRequestWhenServletPathNotNormalizedThenThrowsRequestRejectedException() throws Exception { + for (String path : this.unnormalizedPaths) { + this.request = new MockHttpServletRequest(); + this.request.setServletPath(path); + try { + this.firewall.getFirewalledRequest(this.request); + fail(path + " is un-normalized"); + } catch (RequestRejectedException expected) { + } + } + } + + @Test + public void getFirewalledRequestWhenPathInfoNotNormalizedThenThrowsRequestRejectedException() throws Exception { + for (String path : this.unnormalizedPaths) { + this.request = new MockHttpServletRequest(); + this.request.setPathInfo(path); + try { + this.firewall.getFirewalledRequest(this.request); + fail(path + " is un-normalized"); + } catch (RequestRejectedException expected) { + } + } + } + + // --- ; --- + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenSemicolonInContextPathThenThrowsRequestRejectedException() { + this.request.setContextPath(";/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenSemicolonInServletPathThenThrowsRequestRejectedException() { + this.request.setServletPath("/spring;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenSemicolonInPathInfoThenThrowsRequestRejectedException() { + this.request.setPathInfo("/path;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenSemicolonInRequestUriThenThrowsRequestRejectedException() { + this.request.setRequestURI("/path;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedSemicolonInContextPathThenThrowsRequestRejectedException() { + this.request.setContextPath("%3B/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedSemicolonInServletPathThenThrowsRequestRejectedException() { + this.request.setServletPath("/spring%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedSemicolonInPathInfoThenThrowsRequestRejectedException() { + this.request.setPathInfo("/path%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedSemicolonInRequestUriThenThrowsRequestRejectedException() { + this.request.setRequestURI("/path%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInContextPathThenThrowsRequestRejectedException() { + this.request.setContextPath("%3b/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInServletPathThenThrowsRequestRejectedException() { + this.request.setServletPath("/spring%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInPathInfoThenThrowsRequestRejectedException() { + this.request.setPathInfo("/path%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInRequestUriThenThrowsRequestRejectedException() { + this.request.setRequestURI("/path%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenSemicolonInContextPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setContextPath(";/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenSemicolonInServletPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setServletPath("/spring;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenSemicolonInPathInfoAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setPathInfo("/path;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenSemicolonInRequestUriAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setRequestURI("/path;/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenEncodedSemicolonInContextPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setContextPath("%3B/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenEncodedSemicolonInServletPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setServletPath("/spring%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenEncodedSemicolonInPathInfoAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setPathInfo("/path%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenEncodedSemicolonInRequestUriAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setRequestURI("/path%3B/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInContextPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setContextPath("%3b/context"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInServletPathAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setServletPath("/spring%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInPathInfoAndAllowSemicolonThenNoException() { + this.firewall.setAllowUrlEncodedPercent(true); + this.firewall.setAllowSemicolon(true); + this.request.setPathInfo("/path%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenLowercaseEncodedSemicolonInRequestUriAndAllowSemicolonThenNoException() { + this.firewall.setAllowSemicolon(true); + this.request.setRequestURI("/path%3b/"); + + this.firewall.getFirewalledRequest(this.request); + } + + // --- encoded . --- + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenEncodedPeriodInThenThrowsRequestRejectedException() { + this.request.setRequestURI("/%2E/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedPeriodInThenThrowsRequestRejectedException() { + this.request.setRequestURI("/%2e/"); + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenAllowEncodedPeriodAndEncodedPeriodInThenNoException() { + this.firewall.setAllowUrlEncodedPeriod(true); + this.request.setRequestURI("/%2E/"); + + this.firewall.getFirewalledRequest(this.request); + } + + // --- from DefaultHttpFirewallTests --- + + /** + * On WebSphere 8.5 a URL like /context-root/a/b;%2f1/c can bypass a rule on + * /a/b/c because the pathInfo is /a/b;/1/c which ends up being /a/b/1/c + * while Spring MVC will strip the ; content from requestURI before the path + * is URL decoded. + */ + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenLowercaseEncodedPathThenException() { + this.request.setRequestURI("/context-root/a/b;%2f1/c"); + this.request.setContextPath("/context-root"); + this.request.setServletPath(""); + this.request.setPathInfo("/a/b;/1/c"); // URL decoded requestURI + this.firewall.getFirewalledRequest(this.request); + } + + @Test(expected = RequestRejectedException.class) + public void getFirewalledRequestWhenUppercaseEncodedPathThenException() { + this.request.setRequestURI("/context-root/a/b;%2F1/c"); + this.request.setContextPath("/context-root"); + this.request.setServletPath(""); + this.request.setPathInfo("/a/b;/1/c"); // URL decoded requestURI + + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenAllowUrlEncodedSlashAndLowercaseEncodedPathThenNoException() { + this.firewall.setAllowUrlEncodedSlash(true); + this.firewall.setAllowSemicolon(true); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setRequestURI("/context-root/a/b;%2f1/c"); + request.setContextPath("/context-root"); + request.setServletPath(""); + request.setPathInfo("/a/b;/1/c"); // URL decoded requestURI + + this.firewall.getFirewalledRequest(request); + } + + @Test + public void getFirewalledRequestWhenAllowUrlEncodedSlashAndUppercaseEncodedPathThenNoException() { + this.firewall.setAllowUrlEncodedSlash(true); + this.firewall.setAllowSemicolon(true); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setRequestURI("/context-root/a/b;%2F1/c"); + request.setContextPath("/context-root"); + request.setServletPath(""); + request.setPathInfo("/a/b;/1/c"); // URL decoded requestURI + + this.firewall.getFirewalledRequest(request); + } +}
Vulnerability mechanics
Generated on May 9, 2026. Inputs: CWE entries + fix-commit diffs from this CVE's patches. Citations validated against bundle.
References
17- access.redhat.com/errata/RHSA-2018:2405ghsavendor-advisoryx_refsource_REDHATWEB
- github.com/advisories/GHSA-v596-fwhq-8x48ghsaADVISORY
- nvd.nist.gov/vuln/detail/CVE-2018-1199ghsaADVISORY
- github.com/spring-projects/spring-framework/commit/554662ebab87af97ba25d0c9f5449c7acda8df9cghsaWEB
- github.com/spring-projects/spring-framework/commit/73a81f98d40eb6f5faa91aceb868db53fae2a94bghsaWEB
- github.com/spring-projects/spring-framework/commit/e6e6b8f4adcad99d133de97fcfac5ae5dd14153cghsaWEB
- github.com/spring-projects/spring-security/commit/0eef5b4b425ab42b9fa0fde1a3f36a37b92558fghsaWEB
- github.com/spring-projects/spring-security/commit/65da28e4bf62f58fb130ba727cbbd621b44a36dghsaWEB
- github.com/spring-projects/spring-security/commit/cb8041ba67635edafcc934498ef82707157fd22ghsaWEB
- lists.apache.org/thread.html/4ed49b103f64a0cecb38064f26cbf1389afc12124653da2d35166dbe%40%3Cissues.activemq.apache.org%3Emitremailing-listx_refsource_MLIST
- lists.apache.org/thread.html/4ed49b103f64a0cecb38064f26cbf1389afc12124653da2d35166dbe@%3Cissues.activemq.apache.org%3EghsaWEB
- lists.apache.org/thread.html/ab825fcade0b49becfa30235b3d54f4a51bb74ea96b6c9adb5d1378c%40%3Cissues.activemq.apache.org%3Emitremailing-listx_refsource_MLIST
- lists.apache.org/thread.html/ab825fcade0b49becfa30235b3d54f4a51bb74ea96b6c9adb5d1378c@%3Cissues.activemq.apache.org%3EghsaWEB
- lists.apache.org/thread.html/dcf8599b80e43a6b60482607adb76c64672772dc2d9209ae2170f369%40%3Cissues.activemq.apache.org%3Emitremailing-listx_refsource_MLIST
- lists.apache.org/thread.html/dcf8599b80e43a6b60482607adb76c64672772dc2d9209ae2170f369@%3Cissues.activemq.apache.org%3EghsaWEB
- pivotal.io/security/cve-2018-1199ghsax_refsource_CONFIRMWEB
- www.oracle.com/security-alerts/cpujul2020.htmlghsax_refsource_MISCWEB
News mentions
0No linked articles in our index yet.