[#2837] - Null path normalization in getPathWithinApplication causes filter chain bypass#2836
Open
xaccefy wants to merge 3 commits into
Open
[#2837] - Null path normalization in getPathWithinApplication causes filter chain bypass#2836xaccefy wants to merge 3 commits into
xaccefy wants to merge 3 commits into
Conversation
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.
xaccefy
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a security-relevant edge case where WebUtils.getPathWithinApplication() could return null (when normalize() fails on traversal-above-root paths), which could propagate into PathMatchingFilterChainResolver.getChain() and result in Shiro returning null and falling back to the container chain (bypassing Shiro filters).
Changes:
- Guard
WebUtils.getPathWithinApplication()so normalization failures don’t returnnull. - Add a “catch-all” fallback in
PathMatchingFilterChainResolver.getChain()for null/empty request paths. - Add regression tests covering traversal-above-root normalization behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| web/src/main/java/org/apache/shiro/web/util/WebUtils.java | Prevents null path propagation from normalization failures. |
| web/src/main/java/org/apache/shiro/web/filter/mgt/PathMatchingFilterChainResolver.java | Adds a safety-net to avoid returning null when a catch-all chain is available and the request path is missing. |
| web/src/test/groovy/org/apache/shiro/web/util/WebUtilsTest.groovy | Adds tests for traversal-above-root inputs. |
| web/src/test/java/org/apache/shiro/web/filter/mgt/PathMatchingFilterChainResolverTest.java | Adds regression coverage to ensure traversal-above-root no longer results in a null resolved chain when /** exists. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+294
to
+306
| /** | ||
| * Verifies that path traversal above root (where normalize returns null) | ||
| * no longer bypasses Shiro. The path is normalized to "/" and can match | ||
| * the /** catch-all chain if one exists. | ||
| */ | ||
| @Test | ||
| void testPathTraversalAboveRootFallsBackToCatchAll() { | ||
| HttpServletRequest request = mock(HttpServletRequest.class); | ||
| HttpServletResponse response = mock(HttpServletResponse.class); | ||
| FilterChain chain = mock(FilterChain.class); | ||
|
|
||
| // Create the /** catch-all chain (as ShiroFactoryBean would) | ||
| resolver.getFilterChainManager().createChain("/**", "anon"); |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Comment on lines
+153
to
+157
| // If no chain matched and the requestURI is null/empty, fall back to the catch-all chain if one exists. | ||
| // This ensures global filters (e.g. InvalidRequestFilter) always run when the request path cannot be resolved. | ||
| if (requestURI == null || "".equals(requestURI)) { | ||
| NamedFilterList catchAllChain = filterChainManager.getChain("/**"); | ||
| if (catchAllChain != null) { |
lprimak
requested changes
Jul 22, 2026
| */ | ||
| public static String getPathWithinApplication(HttpServletRequest request) { | ||
| return normalize(removeSemicolon(getServletPath(request) + getPathInfo(request))); | ||
| String path = normalize(removeSemicolon(getServletPath(request) + getPathInfo(request))); |
Contributor
There was a problem hiding this comment.
This change is very risky, and possibly introduces bypass vectors.
I think this needs to be reverted
| } | ||
| } | ||
|
|
||
| // If no chain matched and the requestURI is null/empty, fall back to the catch-all chain if one exists. |
Contributor
There was a problem hiding this comment.
This change also needs to be introduced in support/guice/src/main/java/org/apache/shiro/guice/web/SimpleFilterChainResolver.java
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
WebUtils.getPathWithinApplication()can returnnullwhennormalize()fails (path traverses above root, e.g./../). This null propagates throughPathMatchingFilterChainResolver.getChain()which matches zero patterns, causingAbstractShiroFilterto fall back to the original containerFilterChain— bypassing all Shiro filters.This was introduced in commit b90f918 (Shiro 1.5.3, CVE-2020-11989 fix) when
normalize()was added togetPathWithinApplication()without a null guard, and has remained unhandled through all subsequent versions (1.5.3 → 3.0.0).Fix
Two layers:
WebUtils.getPathWithinApplication(): Return"/"instead ofnullwhennormalize()fails.PathMatchingFilterChainResolver.getChain(): Safety net — fall back to/**chain whenrequestURIis null/empty.Tests
NullNormalizationBypassPocTest— 4 tests for the null-normalization scenarioWebUtilsTest— traversal-above-root pathsPathMatchingFilterChainResolverTest— catch-all fallback integration testImpact
Prevents the class of auth bypass vulnerabilities caused by null path propagation through the filter chain.
fixes #2837