Browse Source

Block URL Encoded "/" in DefaultHttpFirewall

Fixes gh-4171
Rob Winch 8 years ago
parent
commit
6d30da2e1f

+ 42 - 3
web/src/main/java/org/springframework/security/web/firewall/DefaultHttpFirewall.java

@@ -20,6 +20,7 @@ import javax.servlet.http.HttpServletResponse;
  * @author Luke Taylor
  */
 public class DefaultHttpFirewall implements HttpFirewall {
+    private boolean allowUrlEncodedSlash;
 
     public FirewalledRequest getFirewalledRequest(HttpServletRequest request) throws RequestRejectedException {
         FirewalledRequest fwr = new RequestWrapper(request);
@@ -29,6 +30,11 @@ public class DefaultHttpFirewall implements HttpFirewall {
                 (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;
     }
 
@@ -37,10 +43,43 @@ public class DefaultHttpFirewall implements HttpFirewall {
     }
 
     /**
-     * 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 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.
+     * @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) {

+ 70 - 0
web/src/test/java/org/springframework/security/web/firewall/DefaultHttpFirewallTests.java

@@ -1,3 +1,18 @@
+/*
+ * Copyright 2002-2016 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 static org.junit.Assert.fail;
@@ -41,4 +56,59 @@ public class 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() {
+        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);
+    }
 }