Skip to content

Commit

Permalink
Adds improved path filter
Browse files Browse the repository at this point in the history
  • Loading branch information
@bdemers
bdemers committed Jul 10, 2023
1 parent 2eac0dc commit c3ede3f
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 1 deletion.
51 changes: 50 additions & 1 deletion web/src/main/java/org/apache/shiro/web/filter/InvalidRequestFilter.java
View file
Expand Up @@ -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>
Expand All @@ -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);
Expand All @@ -67,7 +74,8 @@ private boolean isValid(String uri) {
return !StringUtils.hasText(uri)
|| ( !containsSemicolon(uri)
&& !containsBackslash(uri)
&& !containsNonAsciiCharacters(uri));
&& !containsNonAsciiCharacters(uri))
&& !containsTraversal(uri);
}

@Override
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
}
51 changes: 51 additions & 0 deletions web/src/test/groovy/org/apache/shiro/web/filter/InvalidRequestFilterTest.groovy
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit c3ede3f

Please sign in to comment.