You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shiro.apache.org by bd...@apache.org on 2023/07/10 14:00:20 UTC
[shiro] branch 1.12.x updated: Adds improved path filter
This is an automated email from the ASF dual-hosted git repository.
bdemers pushed a commit to branch 1.12.x
in repository https://gitbox.apache.org/repos/asf/shiro.git
The following commit(s) were added to refs/heads/1.12.x by this push:
new c3ede3f94 Adds improved path filter
c3ede3f94 is described below
commit c3ede3f94efb442acb0795714a022c2c121d1da0
Author: Brian Demers <bd...@apache.org>
AuthorDate: Wed May 31 13:54:37 2023 -0400
Adds improved path filter
---
.../shiro/web/filter/InvalidRequestFilter.java | 51 +++++++++++++++++++++-
.../web/filter/InvalidRequestFilterTest.groovy | 51 ++++++++++++++++++++++
2 files changed, 101 insertions(+), 1 deletion(-)
diff --git a/web/src/main/java/org/apache/shiro/web/filter/InvalidRequestFilter.java b/web/src/main/java/org/apache/shiro/web/filter/InvalidRequestFilter.java
index e352d5c14..97133500f 100644
--- a/web/src/main/java/org/apache/shiro/web/filter/InvalidRequestFilter.java
+++ b/web/src/main/java/org/apache/shiro/web/filter/InvalidRequestFilter.java
@@ -37,6 +37,7 @@ import java.util.List;
* <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 @@ public class InvalidRequestFilter extends AccessControlFilter {
return !StringUtils.hasText(uri)
|| ( !containsSemicolon(uri)
&& !containsBackslash(uri)
- && !containsNonAsciiCharacters(uri));
+ && !containsNonAsciiCharacters(uri))
+ && !containsTraversal(uri);
}
@Override
@@ -108,6 +116,39 @@ public class InvalidRequestFilter extends AccessControlFilter {
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 class InvalidRequestFilter extends AccessControlFilter {
public void setBlockNonAscii(boolean blockNonAscii) {
this.blockNonAscii = blockNonAscii;
}
+
+ public boolean isBlockTraversal() {
+ return blockTraversal;
+ }
+
+ public void setBlockTraversal(boolean blockTraversal) {
+ this.blockTraversal = blockTraversal;
+ }
}
diff --git a/web/src/test/groovy/org/apache/shiro/web/filter/InvalidRequestFilterTest.groovy b/web/src/test/groovy/org/apache/shiro/web/filter/InvalidRequestFilterTest.groovy
index c7a052504..9e37b3fef 100644
--- a/web/src/test/groovy/org/apache/shiro/web/filter/InvalidRequestFilterTest.groovy
+++ b/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)