Skip to content

Commit ce02984

Browse files
hasterhaster
committed
Adds a 3-valued enum for path-traversal-blockmode
Default is NORMAL, which only blocks actual paths. STRICT also blocks encoded slashes ('/') and periods ('.'). NO_BLOCK disables.
1 parent fbd31e1 commit ce02984

File tree

2 files changed

+78
-17
lines changed

2 files changed

+78
-17
lines changed

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

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,12 @@
4545
*/
4646
public class InvalidRequestFilter extends AccessControlFilter {
4747

48+
static enum PathTraversalBlockMode {
49+
STRICT,
50+
NORMAL,
51+
NO_BLOCK;
52+
}
53+
4854
private static final List<String> SEMICOLON = Collections.unmodifiableList(Arrays.asList(";", "%3b", "%3B"));
4955

5056
private static final List<String> BACKSLASH = Collections.unmodifiableList(Arrays.asList("\\", "%5c", "%5C"));
@@ -59,7 +65,7 @@ public class InvalidRequestFilter extends AccessControlFilter {
5965

6066
private boolean blockNonAscii = true;
6167

62-
private boolean blockTraversal = true;
68+
private PathTraversalBlockMode pathTraversalBlockMode = PathTraversalBlockMode.NORMAL;
6369

6470
@Override
6571
protected boolean isAccessAllowed(ServletRequest req, ServletResponse response, Object mappedValue) throws Exception {
@@ -117,7 +123,10 @@ private static boolean containsOnlyPrintableAsciiCharacters(String uri) {
117123
}
118124

119125
private boolean containsTraversal(String uri) {
120-
if (isBlockTraversal()) {
126+
if (isBlockTraversalNormal()) {
127+
return !(isNormalized(uri));
128+
}
129+
if (isBlockTraversalStrict()) {
121130
return !(isNormalized(uri)
122131
&& PERIOD.stream().noneMatch(uri::contains)
123132
&& FORWARDSLASH.stream().noneMatch(uri::contains));
@@ -173,11 +182,24 @@ public void setBlockNonAscii(boolean blockNonAscii) {
173182
this.blockNonAscii = blockNonAscii;
174183
}
175184

176-
public boolean isBlockTraversal() {
177-
return blockTraversal;
185+
public boolean isBlockTraversalNormal() {
186+
return pathTraversalBlockMode == PathTraversalBlockMode.NORMAL;
187+
}
188+
189+
public boolean isBlockTraversalStrict() {
190+
return pathTraversalBlockMode == PathTraversalBlockMode.STRICT;
178191
}
179192

193+
public void setPathTraversalBlockMode(PathTraversalBlockMode mode) {
194+
this.pathTraversalBlockMode = mode;
195+
}
196+
197+
/**
198+
*
199+
* @deprecated Use {@link #setPathTraversalBlockMode(PathTraversalBlockMode)}
200+
*/
201+
@Deprecated
180202
public void setBlockTraversal(boolean blockTraversal) {
181-
this.blockTraversal = blockTraversal;
203+
this.pathTraversalBlockMode = blockTraversal ? PathTraversalBlockMode.NORMAL : PathTraversalBlockMode.NO_BLOCK;
182204
}
183205
}

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

Lines changed: 51 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ class InvalidRequestFilterTest {
3737
assertThat "filter.blockBackslash expected to be true", filter.isBlockBackslash()
3838
assertThat "filter.blockNonAscii expected to be true", filter.isBlockNonAscii()
3939
assertThat "filter.blockSemicolon expected to be true", filter.isBlockSemicolon()
40-
assertThat "filter.blockTraversal expected to be true", filter.isBlockTraversal()
40+
assertThat "filter.blockTraversal expected to be NORMAL", filter.isBlockTraversalNormal()
4141
}
4242

4343
@Test
@@ -75,29 +75,63 @@ class InvalidRequestFilterTest {
7575
}
7676

7777
@Test
78-
void testBlocksTraversal() {
78+
void testBlocksTraversalNormal() {
7979
InvalidRequestFilter filter = new InvalidRequestFilter()
8080
assertPathBlocked(filter, "/something/../")
8181
assertPathBlocked(filter, "/something/../bar")
8282
assertPathBlocked(filter, "/something/../bar/")
83-
assertPathBlocked(filter, "/something/%2e%2E/bar/")
8483
assertPathBlocked(filter, "/something/..")
8584
assertPathBlocked(filter, "/..")
8685
assertPathBlocked(filter, "..")
8786
assertPathBlocked(filter, "../")
88-
assertPathBlocked(filter, "%2E./")
89-
assertPathBlocked(filter, "%2F./")
9087
assertPathBlocked(filter, "/something/./")
9188
assertPathBlocked(filter, "/something/./bar")
9289
assertPathBlocked(filter, "/something/\u002e/bar")
9390
assertPathBlocked(filter, "/something/./bar/")
94-
assertPathBlocked(filter, "/something/%2e/bar/")
95-
assertPathBlocked(filter, "/something/%2f/bar/")
9691
assertPathBlocked(filter, "/something/.")
9792
assertPathBlocked(filter, "/.")
9893
assertPathBlocked(filter, "/something/../something/.")
9994
assertPathBlocked(filter, "/something/../something/.")
95+
96+
assertPathAllowed(filter, "%2E./")
97+
assertPathAllowed(filter, "%2F./")
98+
assertPathAllowed(filter, "/something/%2e/bar/")
99+
assertPathAllowed(filter, "/something/%2f/bar/")
100+
assertPathAllowed(filter, "/something/http:%2f%2fmydomain.example.com%2foidc/bar/")
101+
assertPathAllowed(filter, "/something/http:%2f%2fmydomain.example.com%2foidc/bar/")
102+
assertPathAllowed(filter, "/something/%2e%2E/bar/")
103+
assertPathAllowed(filter, "/something/http:%2f%2fmydomain%2eexample%2ecom%2foidc/bar/")
100104
}
105+
106+
@Test
107+
void testBlocksTraversalStrict() {
108+
InvalidRequestFilter filter = new InvalidRequestFilter()
109+
filter.setPathTraversalBlockMode(InvalidRequestFilter.PathTraversalBlockMode.STRICT)
110+
assertPathBlocked(filter, "/something/../")
111+
assertPathBlocked(filter, "/something/../bar")
112+
assertPathBlocked(filter, "/something/../bar/")
113+
assertPathBlocked(filter, "/something/..")
114+
assertPathBlocked(filter, "/..")
115+
assertPathBlocked(filter, "..")
116+
assertPathBlocked(filter, "../")
117+
assertPathBlocked(filter, "/something/./")
118+
assertPathBlocked(filter, "/something/./bar")
119+
assertPathBlocked(filter, "/something/\u002e/bar")
120+
assertPathBlocked(filter, "/something/./bar/")
121+
assertPathBlocked(filter, "/something/.")
122+
assertPathBlocked(filter, "/.")
123+
assertPathBlocked(filter, "/something/../something/.")
124+
assertPathBlocked(filter, "/something/../something/.")
125+
126+
assertPathBlocked(filter, "%2E./")
127+
assertPathBlocked(filter, "%2F./")
128+
assertPathBlocked(filter, "/something/%2e/bar/")
129+
assertPathBlocked(filter, "/something/%2f/bar/")
130+
assertPathBlocked(filter, "/something/http:%2f%2fmydomain.example.com%2foidc/bar/")
131+
assertPathBlocked(filter, "/something/http:%2f%2fmydomain.example.com%2foidc/bar/")
132+
assertPathBlocked(filter, "/something/%2e%2E/bar/")
133+
assertPathBlocked(filter, "/something/http:%2f%2fmydomain%2eexample%2ecom%2foidc/bar/")
134+
}
101135

102136
@Test
103137
void testFilterAllowsBackslash() {
@@ -149,7 +183,7 @@ class InvalidRequestFilterTest {
149183
@Test
150184
void testAllowTraversal() {
151185
InvalidRequestFilter filter = new InvalidRequestFilter()
152-
filter.setBlockTraversal(false)
186+
filter.setPathTraversalBlockMode(InvalidRequestFilter.PathTraversalBlockMode.NO_BLOCK);
153187

154188
assertPathAllowed(filter, "/something/../")
155189
assertPathAllowed(filter, "/something/../bar")
@@ -158,18 +192,23 @@ class InvalidRequestFilterTest {
158192
assertPathAllowed(filter, "/..")
159193
assertPathAllowed(filter, "..")
160194
assertPathAllowed(filter, "../")
161-
assertPathAllowed(filter, "%2E./")
162-
assertPathAllowed(filter, "%2F./")
163195
assertPathAllowed(filter, "/something/./")
164196
assertPathAllowed(filter, "/something/./bar")
165197
assertPathAllowed(filter, "/something/\u002e/bar")
166198
assertPathAllowed(filter, "/something\u002fbar")
167199
assertPathAllowed(filter, "/something/./bar/")
168-
assertPathAllowed(filter, "/something/%2e/bar/")
169-
assertPathAllowed(filter, "/something/%2f/bar/")
170200
assertPathAllowed(filter, "/something/.")
171201
assertPathAllowed(filter, "/.")
172202
assertPathAllowed(filter, "/something/../something/.")
203+
204+
assertPathAllowed(filter, "%2E./")
205+
assertPathAllowed(filter, "%2F./")
206+
assertPathAllowed(filter, "/something/%2e/bar/")
207+
assertPathAllowed(filter, "/something/%2f/bar/")
208+
assertPathAllowed(filter, "/something/http:%2f%2fmydomain.example.com%2foidc/bar/")
209+
assertPathAllowed(filter, "/something/http:%2f%2fmydomain.example.com%2foidc/bar/")
210+
assertPathAllowed(filter, "/something/%2e%2E/bar/")
211+
assertPathAllowed(filter, "/something/http:%2f%2fmydomain%2eexample%2ecom%2foidc/bar/")
173212
}
174213

175214
static void assertPathBlocked(InvalidRequestFilter filter, String requestUri, String servletPath = requestUri, String pathInfo = null) {

0 commit comments

Comments
 (0)