Block URL Encoded "/" in DefaultHttpFirewall
Fixes gh-4169
This commit is contained in:
parent
fc516b55a6
commit
666e356ebc
|
@ -19,49 +19,89 @@ import javax.servlet.http.HttpServletRequest;
|
|||
import javax.servlet.http.HttpServletResponse;
|
||||
|
||||
/**
|
||||
* Default implementation which wraps requests in order to provide consistent values of
|
||||
* the {@code servletPath} and {@code pathInfo}, which do not contain path parameters (as
|
||||
* defined in <a href="http://www.ietf.org/rfc/rfc2396.txt">RFC 2396</a>). Different
|
||||
* servlet containers interpret the servlet spec differently as to how path parameters are
|
||||
* treated and it is possible they might be added in order to bypass particular security
|
||||
* constraints. When using this implementation, they will be removed for all requests as
|
||||
* the request passes through the security filter chain. Note that this means that any
|
||||
* segments in the decoded path which contain a semi-colon, will have the part following
|
||||
* the semi-colon removed for request matching. Your application should not contain any
|
||||
* valid paths which contain semi-colons.
|
||||
* Default implementation which wraps requests in order to provide consistent
|
||||
* values of the {@code servletPath} and {@code pathInfo}, which do not contain
|
||||
* path parameters (as defined in
|
||||
* <a href="http://www.ietf.org/rfc/rfc2396.txt">RFC 2396</a>). Different
|
||||
* servlet containers interpret the servlet spec differently as to how path
|
||||
* parameters are treated and it is possible they might be added in order to
|
||||
* bypass particular security constraints. When using this implementation, they
|
||||
* will be removed for all requests as the request passes through the security
|
||||
* filter chain. Note that this means that any segments in the decoded path
|
||||
* which contain a semi-colon, will have the part following the semi-colon
|
||||
* removed for request matching. Your application should not contain any valid
|
||||
* paths which contain semi-colons.
|
||||
* <p>
|
||||
* If any un-normalized paths are found (containing directory-traversal character
|
||||
* sequences), the request will be rejected immediately. Most containers normalize the
|
||||
* paths before performing the servlet-mapping, but again this is not guaranteed by the
|
||||
* servlet spec.
|
||||
* If any un-normalized paths are found (containing directory-traversal
|
||||
* character sequences), the request will be rejected immediately. Most
|
||||
* containers normalize the paths before performing the servlet-mapping, but
|
||||
* again this is not guaranteed by the servlet spec.
|
||||
*
|
||||
* @author Luke Taylor
|
||||
*/
|
||||
public class DefaultHttpFirewall implements HttpFirewall {
|
||||
private boolean allowUrlEncodedSlash;
|
||||
|
||||
public FirewalledRequest getFirewalledRequest(HttpServletRequest request)
|
||||
throws RequestRejectedException {
|
||||
@Override
|
||||
public FirewalledRequest getFirewalledRequest(HttpServletRequest request) throws RequestRejectedException {
|
||||
FirewalledRequest fwr = new RequestWrapper(request);
|
||||
|
||||
if (!isNormalized(fwr.getServletPath()) || !isNormalized(fwr.getPathInfo())) {
|
||||
throw new RequestRejectedException("Un-normalized paths are not supported: "
|
||||
+ fwr.getServletPath()
|
||||
throw new RequestRejectedException("Un-normalized paths are not supported: " + fwr.getServletPath()
|
||||
+ (fwr.getPathInfo() != null ? fwr.getPathInfo() : ""));
|
||||
}
|
||||
|
||||
String requestURI = fwr.getRequestURI();
|
||||
if (containsInvalidUrlEncodedSlash(requestURI)) {
|
||||
throw new RequestRejectedException("The requestURI cannot contain encoded slash. Got " + requestURI);
|
||||
}
|
||||
|
||||
return fwr;
|
||||
}
|
||||
|
||||
@Override
|
||||
public HttpServletResponse getFirewalledResponse(HttpServletResponse response) {
|
||||
return new FirewalledResponse(response);
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks whether a path is normalized (doesn't contain path traversal sequences like
|
||||
* "./", "/../" or "/.")
|
||||
* <p>
|
||||
* Sets if the application should allow a URL encoded slash character.
|
||||
* </p>
|
||||
* <p>
|
||||
* If true (default is false), a URL encoded slash will be allowed in the
|
||||
* URL. Allowing encoded slashes can cause security vulnerabilities in some
|
||||
* situations depending on how the container constructs the
|
||||
* HttpServletRequest.
|
||||
* </p>
|
||||
*
|
||||
* @param path the path to test
|
||||
* @return true if the path doesn't contain any path-traversal character sequences.
|
||||
* @param allowUrlEncodedSlash
|
||||
* the new value (default false)
|
||||
*/
|
||||
public void setAllowUrlEncodedSlash(boolean allowUrlEncodedSlash) {
|
||||
this.allowUrlEncodedSlash = allowUrlEncodedSlash;
|
||||
}
|
||||
|
||||
private boolean containsInvalidUrlEncodedSlash(String uri) {
|
||||
if (this.allowUrlEncodedSlash || uri == null) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (uri.contains("%2f") || uri.contains("%2F")) {
|
||||
return true;
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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 boolean isNormalized(String path) {
|
||||
if (path == null) {
|
||||
|
@ -75,8 +115,7 @@ public class DefaultHttpFirewall implements HttpFirewall {
|
|||
if (gap == 2 && path.charAt(i + 1) == '.') {
|
||||
// ".", "/./" or "/."
|
||||
return false;
|
||||
}
|
||||
else if (gap == 3 && path.charAt(i + 1) == '.' && path.charAt(i + 2) == '.') {
|
||||
} else if (gap == 3 && path.charAt(i + 1) == '.' && path.charAt(i + 2) == '.') {
|
||||
return false;
|
||||
}
|
||||
|
||||
|
|
|
@ -15,39 +15,93 @@
|
|||
*/
|
||||
package org.springframework.security.web.firewall;
|
||||
|
||||
import static org.assertj.core.api.Assertions.fail;
|
||||
|
||||
import org.junit.Test;
|
||||
|
||||
import org.springframework.mock.web.MockHttpServletRequest;
|
||||
|
||||
import static org.assertj.core.api.Assertions.fail;
|
||||
|
||||
/**
|
||||
* @author Luke Taylor
|
||||
*/
|
||||
public class DefaultHttpFirewallTests {
|
||||
public String[] unnormalizedPaths = { "/..", "/./path/", "/path/path/.",
|
||||
"/path/path//.", "./path/../path//.", "./path", ".//path", "." };
|
||||
public String[] unnormalizedPaths = { "/..", "/./path/", "/path/path/.", "/path/path//.", "./path/../path//.",
|
||||
"./path", ".//path", "." };
|
||||
|
||||
@Test
|
||||
public void unnormalizedPathsAreRejected() throws Exception {
|
||||
DefaultHttpFirewall fw = new DefaultHttpFirewall();
|
||||
|
||||
MockHttpServletRequest request;
|
||||
for (String path : unnormalizedPaths) {
|
||||
for (String path : this.unnormalizedPaths) {
|
||||
request = new MockHttpServletRequest();
|
||||
request.setServletPath(path);
|
||||
try {
|
||||
fw.getFirewalledRequest(request);
|
||||
fail(path + " is un-normalized");
|
||||
}
|
||||
catch (RequestRejectedException expected) {
|
||||
} catch (RequestRejectedException expected) {
|
||||
}
|
||||
request.setPathInfo(path);
|
||||
try {
|
||||
fw.getFirewalledRequest(request);
|
||||
fail(path + " is un-normalized");
|
||||
}
|
||||
catch (RequestRejectedException expected) {
|
||||
} catch (RequestRejectedException expected) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 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() {
|
||||
DefaultHttpFirewall fw = new DefaultHttpFirewall();
|
||||
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
|
||||
fw.getFirewalledRequest(request);
|
||||
}
|
||||
|
||||
@Test(expected = RequestRejectedException.class)
|
||||
public void getFirewalledRequestWhenUppercaseEncodedPathThenException() {
|
||||
DefaultHttpFirewall fw = new DefaultHttpFirewall();
|
||||
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
|
||||
|
||||
fw.getFirewalledRequest(request);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void getFirewalledRequestWhenAllowUrlEncodedSlashAndLowercaseEncodedPathThenNoException() {
|
||||
DefaultHttpFirewall fw = new DefaultHttpFirewall();
|
||||
fw.setAllowUrlEncodedSlash(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
|
||||
|
||||
fw.getFirewalledRequest(request);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void getFirewalledRequestWhenAllowUrlEncodedSlashAndUppercaseEncodedPathThenNoException() {
|
||||
DefaultHttpFirewall fw = new DefaultHttpFirewall();
|
||||
fw.setAllowUrlEncodedSlash(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
|
||||
|
||||
fw.getFirewalledRequest(request);
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue