Adds improved path filter

Author: bdemers

web/src/main/java/org/apache/shiro/web/filter/InvalidRequestFilter.java

@@ -37,6 +37,7 @@
  *     <li>Semicolon - can be disabled by setting {@code blockSemicolon = false}</li>
  *     <li>Backslash - can be disabled by setting {@code blockBackslash = false}</li>
  *     <li>Non-ASCII characters - can be disabled by setting {@code blockNonAscii = false}, the ability to disable this check will be removed in future version.</li>
+ *     <li>Path traversals - can be disabled by setting {@code blockTraversal = false}</li>
  * </ul>
  *
  * @see <a href="https://docs.spring.io/spring-security/site/docs/current/api/org/springframework/security/web/firewall/StrictHttpFirewall.html">This class was inspired by Spring Security StrictHttpFirewall</a>
@@ -48,12 +49,18 @@ public class InvalidRequestFilter extends AccessControlFilter {
 
     private static final List<String> BACKSLASH = Collections.unmodifiableList(Arrays.asList("\\", "%5c", "%5C"));
 
+    private static final List<String> FORWARDSLASH = Collections.unmodifiableList(Arrays.asList("%2f", "%2F"));
+
+    private static final List<String> PERIOD = Collections.unmodifiableList(Arrays.asList("%2e", "%2E"));
+
     private boolean blockSemicolon = true;
 
     private boolean blockBackslash = !Boolean.getBoolean(WebUtils.ALLOW_BACKSLASH);
 
     private boolean blockNonAscii = true;
 
+    private boolean blockTraversal = true;
+
     @Override
     protected boolean isAccessAllowed(ServletRequest req, ServletResponse response, Object mappedValue) throws Exception {
         HttpServletRequest request = WebUtils.toHttp(req);
@@ -67,7 +74,8 @@ private boolean isValid(String uri) {
         return !StringUtils.hasText(uri)
                || ( !containsSemicolon(uri)
                  && !containsBackslash(uri)
-                 && !containsNonAsciiCharacters(uri));
+                 && !containsNonAsciiCharacters(uri))
+                 && !containsTraversal(uri);
     }
 
     @Override
@@ -108,6 +116,39 @@ private static boolean containsOnlyPrintableAsciiCharacters(String uri) {
         return true;
     }
 
+    private boolean containsTraversal(String uri) {
+        if (isBlockTraversal()) {
+            return !(isNormalized(uri)
+                    && PERIOD.stream().noneMatch(uri::contains)
+                    && FORWARDSLASH.stream().noneMatch(uri::contains));
+        }
+        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) {
+            return true;
+        }
+        for (int i = path.length(); i > 0;) {
+            int slashIndex = path.lastIndexOf('/', i - 1);
+            int gap = i - slashIndex;
+            if (gap == 2 && path.charAt(slashIndex + 1) == '.') {
+                return false; // ".", "/./" or "/."
+            }
+            if (gap == 3 && path.charAt(slashIndex + 1) == '.' && path.charAt(slashIndex + 2) == '.') {
+                return false;
+            }
+            i = slashIndex;
+        }
+        return true;
+    }
+
     public boolean isBlockSemicolon() {
         return blockSemicolon;
     }
@@ -131,4 +172,12 @@ public boolean isBlockNonAscii() {
     public void setBlockNonAscii(boolean blockNonAscii) {
         this.blockNonAscii = blockNonAscii;
     }
+
+    public boolean isBlockTraversal() {
+        return blockTraversal;
+    }
+
+    public void setBlockTraversal(boolean blockTraversal) {
+        this.blockTraversal = blockTraversal;
+    }
 }

web/src/test/groovy/org/apache/shiro/web/filter/InvalidRequestFilterTest.groovy

@@ -37,6 +37,7 @@ class InvalidRequestFilterTest {
         assertThat "filter.blockBackslash expected to be true", filter.isBlockBackslash()
         assertThat "filter.blockNonAscii expected to be true", filter.isBlockNonAscii()
         assertThat "filter.blockSemicolon expected to be true", filter.isBlockSemicolon()
+        assertThat "filter.blockTraversal expected to be true", filter.isBlockTraversal()
     }
 
     @Test
@@ -73,6 +74,31 @@ class InvalidRequestFilterTest {
         assertPathBlocked(filter, "/something", "/something", "/;")
     }
 
+    @Test
+    void testBlocksTraversal() {
+        InvalidRequestFilter filter = new InvalidRequestFilter()
+        assertPathBlocked(filter, "/something/../")
+        assertPathBlocked(filter, "/something/../bar")
+        assertPathBlocked(filter, "/something/../bar/")
+        assertPathBlocked(filter, "/something/%2e%2E/bar/")
+        assertPathBlocked(filter, "/something/..")
+        assertPathBlocked(filter, "/..")
+        assertPathBlocked(filter, "..")
+        assertPathBlocked(filter, "../")
+        assertPathBlocked(filter, "%2E./")
+        assertPathBlocked(filter, "%2F./")
+        assertPathBlocked(filter, "/something/./")
+        assertPathBlocked(filter, "/something/./bar")
+        assertPathBlocked(filter, "/something/\u002e/bar")
+        assertPathBlocked(filter, "/something/./bar/")
+        assertPathBlocked(filter, "/something/%2e/bar/")
+        assertPathBlocked(filter, "/something/%2f/bar/")
+        assertPathBlocked(filter, "/something/.")
+        assertPathBlocked(filter, "/.")
+        assertPathBlocked(filter, "/something/../something/.")
+        assertPathBlocked(filter, "/something/../something/.")
+    }
+
     @Test
     void testFilterAllowsBackslash() {
         InvalidRequestFilter filter = new InvalidRequestFilter()
@@ -120,6 +146,31 @@ class InvalidRequestFilterTest {
         assertPathAllowed(filter, "/something", "/something", "/;")
     }
 
+    @Test
+    void testAllowTraversal() {
+        InvalidRequestFilter filter = new InvalidRequestFilter()
+        filter.setBlockTraversal(false)
+
+        assertPathAllowed(filter, "/something/../")
+        assertPathAllowed(filter, "/something/../bar")
+        assertPathAllowed(filter, "/something/../bar/")
+        assertPathAllowed(filter, "/something/..")
+        assertPathAllowed(filter, "/..")
+        assertPathAllowed(filter, "..")
+        assertPathAllowed(filter, "../")
+        assertPathAllowed(filter, "%2E./")
+        assertPathAllowed(filter, "%2F./")
+        assertPathAllowed(filter, "/something/./")
+        assertPathAllowed(filter, "/something/./bar")
+        assertPathAllowed(filter, "/something/\u002e/bar")
+        assertPathAllowed(filter, "/something\u002fbar")
+        assertPathAllowed(filter, "/something/./bar/")
+        assertPathAllowed(filter, "/something/%2e/bar/")
+        assertPathAllowed(filter, "/something/%2f/bar/")
+        assertPathAllowed(filter, "/something/.")
+        assertPathAllowed(filter, "/.")
+        assertPathAllowed(filter, "/something/../something/.")
+    }
 
     static void assertPathBlocked(InvalidRequestFilter filter, String requestUri, String servletPath = requestUri, String pathInfo = null) {
         assertThat "Expected path '${requestUri}', to be blocked", !filter.isAccessAllowed(mockRequest(requestUri, servletPath, pathInfo), null, null)