This is an automated email from the ASF dual-hosted git repository.
steinarb pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/shiro.git
The following commit(s) were added to refs/heads/main by this push:
new a453acaeb [#2837] Fail closed when getPathWithinApplication() cannot
normalize the request path (#2836)
a453acaeb is described below
commit a453acaeb4ab74d3d9ba7ea0e30df798cdcae281
Author: xaccefy <[email protected]>
AuthorDate: Sat Aug 15 23:33:39 2026 +0800
[#2837] Fail closed when getPathWithinApplication() cannot normalize the
request path (#2836)
* fix: prevent filter chain bypass when path normalizes to null
When WebUtils.getPathWithinApplication() receives a path that
normalize() resolves to null (path traversal above root, e.g.
getServletPath()=/ followed by getPathInfo()=../), the method
returned null. This caused PathMatchingFilterChainResolver.getChain()
to fail matching any pattern including the required /** catch-all,
causing AbstractShiroFilter.getExecutionChain() to pass the request
to the original container FilterChain without any Shiro filtering.
This bypass meant global filters such as InvalidRequestFilter never
ran, enabling auth bypass via path traversal.
Fix:
- WebUtils.getPathWithinApplication(): return '/' instead of null
when normalize() fails, so the path always matches /**.
- PathMatchingFilterChainResolver.getChain(): safety net that
falls back to the /** chain when requestURI is null or empty
(defense-in-depth for subclasses that override resolution).
Tests added for both the WebUtils fix and the resolver fallback.
* Fixed wording in the comment
Co-authored-by: Copilot Autofix powered by AI
<[email protected]>
* simplify isEmpty /null check
Co-authored-by: Copilot Autofix powered by AI
<[email protected]>
* fail closed when path normalization fails
WebUtils.getPathWithinApplication() returned null when normalize()
failed (e.g. path traversal above root: servletPath="/" with
pathInfo="../"). The null propagated through
PathMatchingFilterChainResolver.getChain(), which matched no
patterns and returned null, causing AbstractShiroFilter to execute
the original, unfiltered container FilterChain - bypassing all
Shiro filters, including global ones like InvalidRequestFilter.
The null guard has been missing since commit b90f91875 (Shiro
1.5.3, CVE-2020-11989 fix) added normalize() to
getPathWithinApplication().
Fix: throw IllegalStateException when normalize() returns null.
The exception propagates through the resolver to
AbstractShiroFilter, which fails the request closed instead of
serving it unfiltered. The catch-all fallback for null/empty
request URIs stays as defense in depth for subclass overrides.
Tests:
- WebUtilsTest: traversal-above-root paths now assert the exception
- PathMatchingFilterChainResolverTest: traversal request throws;
a null request URI from a subclass override falls back to the
catch-all chain
- NullNormalizationBypassPocTest: reproduces the bypass scenario
(WebUtils throws, resolver fails closed, normal requests still
resolve)
Fixes #2837
* fail closed when getPathWithinApplication() cannot normalize the request
path
WebUtils.getPathWithinApplication() returned null when normalize()
failed (e.g. path traversal above root: servletPath="/" with
pathInfo="../"). The null propagated through
PathMatchingFilterChainResolver.getChain(), which matched no
patterns and returned null, causing AbstractShiroFilter to execute
the original, unfiltered container FilterChain - bypassing all
Shiro filters, including global ones like InvalidRequestFilter.
The null guard has been missing since commit b90f91875 (Shiro
1.5.3, CVE-2020-11989 fix) added normalize() to
getPathWithinApplication().
Fix: throw IllegalStateException when normalize() returns null or
an empty path. The exception propagates through the resolver (and
the Guice SimpleFilterChainResolver, which calls WebUtils
directly) to AbstractShiroFilter, which fails the request closed
instead of serving it unfiltered.
The catch-all fallback from the previous revision is removed per
review: it can never be reached once getPathWithinApplication()
throws, and the null handling now lives in one place.
Tests:
- WebUtilsTest: traversal-above-root paths now assert the exception
- PathMatchingFilterChainResolverTest: traversal request throws
instead of silently matching nothing
- NullNormalizationBypassPocTest: reproduces the bypass scenario
(WebUtils throws, resolver fails closed, normal requests still
resolve)
Fixes #2837
---------
Co-authored-by: Lenny Primak <[email protected]>
Co-authored-by: Copilot Autofix powered by AI
<[email protected]>
Co-authored-by: xaccefy <[email protected]>
---
.../java/org/apache/shiro/web/util/WebUtils.java | 12 ++-
.../org/apache/shiro/web/util/WebUtilsTest.groovy | 17 ++++
.../filter/mgt/NullNormalizationBypassPocTest.java | 100 +++++++++++++++++++++
.../mgt/PathMatchingFilterChainResolverTest.java | 23 +++++
4 files changed, 151 insertions(+), 1 deletion(-)
diff --git a/web/src/main/java/org/apache/shiro/web/util/WebUtils.java
b/web/src/main/java/org/apache/shiro/web/util/WebUtils.java
index dec232bdf..364565cc6 100644
--- a/web/src/main/java/org/apache/shiro/web/util/WebUtils.java
+++ b/web/src/main/java/org/apache/shiro/web/util/WebUtils.java
@@ -116,9 +116,19 @@ public final class WebUtils {
*
* @param request current HTTP request
* @return the path within the web application
+ * @throws IllegalStateException if the path cannot be normalized (e.g. it
+ * traverses above the root, {@code "/.."}) or is empty. Callers must fail
closed
+ * instead of operating on a {@code null} path, which would otherwise
bypass the
+ * filter chain.
*/
public static String getPathWithinApplication(HttpServletRequest request) {
- return normalize(removeSemicolon(getServletPath(request) +
getPathInfo(request)));
+ String path = normalize(removeSemicolon(getServletPath(request) +
getPathInfo(request)));
+ if (path == null || path.isEmpty()) {
+ String servletPath = getServletPath(request);
+ String pathInfo = getPathInfo(request);
+ throw new IllegalStateException("Unable to normalize path: " +
servletPath + pathInfo);
+ }
+ return path;
}
/**
diff --git a/web/src/test/groovy/org/apache/shiro/web/util/WebUtilsTest.groovy
b/web/src/test/groovy/org/apache/shiro/web/util/WebUtilsTest.groovy
index 53b8e3c0f..27561532d 100644
--- a/web/src/test/groovy/org/apache/shiro/web/util/WebUtilsTest.groovy
+++ b/web/src/test/groovy/org/apache/shiro/web/util/WebUtilsTest.groovy
@@ -177,6 +177,9 @@ class WebUtilsTest {
doTestGetPathWithinApplication("/foobar", "//extra", "/foobar/extra");
doTestGetPathWithinApplication("/foobar", "//extra///",
"/foobar/extra/");
doTestGetPathWithinApplication("/foo bar", "/path info", "/foo
bar/path info");
+ // path traversal above root returns null from normalize(); must fail
closed
+ doTestGetPathWithinApplicationExpectException("", "/../");
+ doTestGetPathWithinApplicationExpectException("/", "../");
}
@Test
@@ -260,6 +263,20 @@ class WebUtilsTest {
verify request
}
+ void doTestGetPathWithinApplicationExpectException(String servletPath,
String pathInfo) {
+ def request = createMock(HttpServletRequest)
+ // first read: to build and normalize the path; second read: to build
the exception message
+
expect(request.getAttribute(WebUtils.INCLUDE_SERVLET_PATH_ATTRIBUTE)).andReturn(servletPath).times(2)
+
expect(request.getAttribute(WebUtils.INCLUDE_PATH_INFO_ATTRIBUTE)).andReturn(pathInfo).times(2)
+ if (pathInfo == null) {
+ expect(request.getPathInfo()).andReturn(null).times(2)
+ }
+ replay request
+ assertThrows(IllegalStateException.class,
+ () -> WebUtils.getPathWithinApplication(request))
+ verify request
+ }
+
void doTestGetRequestURI(String rawRequestUri, String expectedValue) {
def request = createMock(HttpServletRequest)
diff --git
a/web/src/test/java/org/apache/shiro/web/filter/mgt/NullNormalizationBypassPocTest.java
b/web/src/test/java/org/apache/shiro/web/filter/mgt/NullNormalizationBypassPocTest.java
new file mode 100644
index 000000000..73716fda5
--- /dev/null
+++
b/web/src/test/java/org/apache/shiro/web/filter/mgt/NullNormalizationBypassPocTest.java
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.apache.shiro.web.filter.mgt;
+
+import org.apache.shiro.web.util.WebUtils;
+import org.junit.jupiter.api.Test;
+
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * Reproduces the filter chain bypass caused by null path normalization and
asserts
+ * the fix fails closed.
+ *
+ * <p>Before the fix, {@link
WebUtils#getPathWithinApplication(jakarta.servlet.http.HttpServletRequest)}
+ * returned {@code null} when {@code normalize()} failed (e.g. path traversal
above root, like
+ * {@code servletPath="/"} + {@code pathInfo="../"}). The {@code null}
propagated through
+ * {@link PathMatchingFilterChainResolver#getChain}, which matched no patterns
and returned
+ * {@code null}, causing {@code AbstractShiroFilter} to execute the original,
unfiltered
+ * container chain.</p>
+ *
+ * <p>After the fix, normalization failure throws {@link
IllegalStateException}, so the
+ * request fails closed instead of being served without any Shiro
filtering.</p>
+ */
+public class NullNormalizationBypassPocTest {
+
+ @Test
+ void getPathWithinApplicationThrowsForTraversalFromRoot() {
+ HttpServletRequest request = request("/", "../");
+
+ assertThrows(IllegalStateException.class,
+ () -> WebUtils.getPathWithinApplication(request));
+ }
+
+ @Test
+ void getPathWithinApplicationThrowsForTraversalAboveRoot() {
+ HttpServletRequest request = request("/app", "/../../");
+
+ assertThrows(IllegalStateException.class,
+ () -> WebUtils.getPathWithinApplication(request));
+ }
+
+ @Test
+ void resolverFailsClosedOnTraversalRequest() {
+ PathMatchingFilterChainResolver resolver = resolverWithCatchAllChain();
+ HttpServletRequest request = request("/", "../");
+ HttpServletResponse response = mock(HttpServletResponse.class);
+ FilterChain originalChain = mock(FilterChain.class);
+
+ // Before the fix this returned null (no pattern matched), which made
+ // AbstractShiroFilter fall back to the unfiltered original chain.
+ assertThrows(IllegalStateException.class,
+ () -> resolver.getChain(request, response, originalChain));
+ }
+
+ @Test
+ void resolverStillResolvesNormalRequests() {
+ PathMatchingFilterChainResolver resolver = resolverWithCatchAllChain();
+ HttpServletRequest request = request("/", "resource/menus");
+ HttpServletResponse response = mock(HttpServletResponse.class);
+ FilterChain originalChain = mock(FilterChain.class);
+
+ assertNotNull(resolver.getChain(request, response, originalChain));
+ }
+
+ private static PathMatchingFilterChainResolver resolverWithCatchAllChain()
{
+ PathMatchingFilterChainResolver resolver = new
PathMatchingFilterChainResolver();
+ resolver.getFilterChainManager().createChain("/**", "anon");
+ return resolver;
+ }
+
+ private static HttpServletRequest request(String servletPath, String
pathInfo) {
+ HttpServletRequest request = mock(HttpServletRequest.class);
+ when(request.getServletPath()).thenReturn(servletPath);
+ when(request.getPathInfo()).thenReturn(pathInfo);
+ return request;
+ }
+}
diff --git
a/web/src/test/java/org/apache/shiro/web/filter/mgt/PathMatchingFilterChainResolverTest.java
b/web/src/test/java/org/apache/shiro/web/filter/mgt/PathMatchingFilterChainResolverTest.java
index f05f3d949..afee0690f 100644
---
a/web/src/test/java/org/apache/shiro/web/filter/mgt/PathMatchingFilterChainResolverTest.java
+++
b/web/src/test/java/org/apache/shiro/web/filter/mgt/PathMatchingFilterChainResolverTest.java
@@ -34,6 +34,7 @@ import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
+import static org.junit.jupiter.api.Assertions.assertThrows;
/**
* Tests for {@link
org.apache.shiro.web.filter.mgt.PathMatchingFilterChainResolver}.
@@ -290,4 +291,26 @@ public class PathMatchingFilterChainResolverTest extends
WebTest {
assertThat(resolved).isNotNull();
verify(request).getServletPath();
}
+
+ /**
+ * Verifies that path traversal above root (where normalize returns null)
+ * throws IllegalStateException from getPathWithinApplication.
+ */
+ @Test
+ void testPathTraversalAboveRootThrowsException() {
+ HttpServletRequest request = mock(HttpServletRequest.class);
+ HttpServletResponse response = mock(HttpServletResponse.class);
+ FilterChain chain = mock(FilterChain.class);
+
+ // Create at least one chain so the resolver doesn't short-circuit.
+ resolver.getFilterChainManager().createChain("/public", "anon");
+
+ // Path that normalizes to null (above root)
+ when(request.getServletPath()).thenReturn("/");
+ when(request.getPathInfo()).thenReturn("../");
+
+ // getPathWithinApplication throws IllegalStateException when
normalize() returns null.
+ assertThrows(IllegalStateException.class,
+ () -> resolver.getChain(request, response, chain));
+ }
}
