Block URL Encoded "/" in DefaultHttpFirewall

Fixes gh-4170

web/src/main/java/org/springframework/security/web/firewall/DefaultHttpFirewall.java

@@ -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
+ * Default implementation which wraps requests in order to provide consistent
- * the {@code servletPath} and {@code pathInfo}, which do not contain path parameters (as
+ * values of the {@code servletPath} and {@code pathInfo}, which do not contain
- * defined in <a href="http://www.ietf.org/rfc/rfc2396.txt">RFC 2396</a>). Different
+ * path parameters (as defined in
- * servlet containers interpret the servlet spec differently as to how path parameters are
+ * <a href="http://www.ietf.org/rfc/rfc2396.txt">RFC 2396</a>). Different
- * treated and it is possible they might be added in order to bypass particular security
+ * servlet containers interpret the servlet spec differently as to how path
- * constraints. When using this implementation, they will be removed for all requests as
+ * parameters are treated and it is possible they might be added in order to
- * the request passes through the security filter chain. Note that this means that any
+ * bypass particular security constraints. When using this implementation, they
- * segments in the decoded path which contain a semi-colon, will have the part following
+ * will be removed for all requests as the request passes through the security
- * the semi-colon removed for request matching. Your application should not contain any
+ * filter chain. Note that this means that any segments in the decoded path
- * valid paths which contain semi-colons.
+ * 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
+ * If any un-normalized paths are found (containing directory-traversal
- * sequences), the request will be rejected immediately. Most containers normalize the
+ * character sequences), the request will be rejected immediately. Most
- * paths before performing the servlet-mapping, but again this is not guaranteed by the
+ * containers normalize the paths before performing the servlet-mapping, but
- * servlet spec.
+ * 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)
+	@Override
-			throws RequestRejectedException {
+	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: "
+			throw new RequestRejectedException("Un-normalized paths are not supported: " + fwr.getServletPath()
-					+ 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
+	 * <p>
-	 * "./", "/../" or "/.")
+	 * 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
+	 * @param allowUrlEncodedSlash
-	 * @return true if the path doesn't contain any path-traversal character sequences.
+	 *            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;
 			}
 

web/src/test/java/org/springframework/security/web/firewall/DefaultHttpFirewallTests.java

@@ -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/.",
+	public String[] unnormalizedPaths = { "/..", "/./path/", "/path/path/.", "/path/path//.", "./path/../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);
+	}
 }