Skip to content

Commit c0e66a9

Browse files
ief2009rwinch
authored andcommitted
1. add customization support for double forwardslash in StrickHttpFirewall
2. add getEncodedUrlBlacklist() and getDecodedUrlBlacklist() method in StrickHttpFirewall Fixes gh-6292
1 parent d099a62 commit c0e66a9

File tree

2 files changed

+133
-4
lines changed

2 files changed

+133
-4
lines changed

web/src/main/java/org/springframework/security/web/firewall/StrictHttpFirewall.java

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ public class StrictHttpFirewall implements HttpFirewall {
8888

8989
private static final List<String> FORBIDDEN_FORWARDSLASH = Collections.unmodifiableList(Arrays.asList("%2f", "%2F"));
9090

91+
private static final List<String> FORBIDDEN_DOUBLE_FORWARDSLASH = Collections.unmodifiableList(Arrays.asList("//", "%2f%2f", "%2f%2F", "%2F%2f", "%2F%2F"));
92+
9193
private static final List<String> FORBIDDEN_BACKSLASH = Collections.unmodifiableList(Arrays.asList("\\", "%5c", "%5C"));
9294

9395
private Set<String> encodedUrlBlacklist = new HashSet<String>();
@@ -99,6 +101,7 @@ public class StrictHttpFirewall implements HttpFirewall {
99101
public StrictHttpFirewall() {
100102
urlBlacklistsAddAll(FORBIDDEN_SEMICOLON);
101103
urlBlacklistsAddAll(FORBIDDEN_FORWARDSLASH);
104+
urlBlacklistsAddAll(FORBIDDEN_DOUBLE_FORWARDSLASH);
102105
urlBlacklistsAddAll(FORBIDDEN_BACKSLASH);
103106

104107
this.encodedUrlBlacklist.add(ENCODED_PERCENT);
@@ -203,6 +206,23 @@ public void setAllowUrlEncodedSlash(boolean allowUrlEncodedSlash) {
203206
}
204207
}
205208

209+
/**
210+
* <p>
211+
* Determines if double slash "//" that is URL encoded "%2F%2F" should be allowed in the path or
212+
* not. The default is to not allow.
213+
* </p>
214+
*
215+
* @param allowUrlEncodedDoubleSlash should a slash "//" that is URL encoded "%2F%2F" be allowed
216+
* in the path or not. Default is false.
217+
*/
218+
public void setAllowUrlEncodedDoubleSlash(boolean allowUrlEncodedDoubleSlash) {
219+
if (allowUrlEncodedDoubleSlash) {
220+
urlBlacklistsRemoveAll(FORBIDDEN_DOUBLE_FORWARDSLASH);
221+
} else {
222+
urlBlacklistsAddAll(FORBIDDEN_DOUBLE_FORWARDSLASH);
223+
}
224+
}
225+
206226
/**
207227
* <p>
208228
* Determines if a period "." that is URL encoded "%2E" should be allowed in the path
@@ -412,10 +432,6 @@ private static boolean isNormalized(String path) {
412432
return true;
413433
}
414434

415-
if (path.indexOf("//") > -1) {
416-
return false;
417-
}
418-
419435
for (int j = path.length(); j > 0;) {
420436
int i = path.lastIndexOf('/', j - 1);
421437
int gap = j - i;
@@ -433,4 +449,21 @@ private static boolean isNormalized(String path) {
433449
return true;
434450
}
435451

452+
/**
453+
* Provides the existing encoded url blacklist which can add/remove entries from
454+
*
455+
* @return the existing encoded url blacklist, never null
456+
*/
457+
public Set<String> getEncodedUrlBlacklist() {
458+
return encodedUrlBlacklist;
459+
}
460+
461+
/**
462+
* Provides the existing decoded url blacklist which can add/remove entries from
463+
*
464+
* @return the existing decoded url blacklist, never null
465+
*/
466+
public Set<String> getDecodedUrlBlacklist() {
467+
return decodedUrlBlacklist;
468+
}
436469
}

web/src/test/java/org/springframework/security/web/firewall/StrictHttpFirewallTests.java

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,4 +428,100 @@ public void getFirewalledRequestWhenAllowUrlEncodedSlashAndUppercaseEncodedPathT
428428

429429
this.firewall.getFirewalledRequest(request);
430430
}
431+
432+
@Test
433+
public void getFirewalledRequestWhenAllowUrlLowerCaseEncodedDoubleSlashThenNoException() throws Exception {
434+
this.firewall.setAllowUrlEncodedSlash(true);
435+
this.firewall.setAllowUrlEncodedDoubleSlash(true);
436+
MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
437+
request.setRequestURI("/context-root/a/b%2f%2fc");
438+
request.setContextPath("/context-root");
439+
request.setServletPath("");
440+
request.setPathInfo("/a/b//c");
441+
assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException();
442+
}
443+
444+
@Test
445+
public void getFirewalledRequestWhenAllowUrlUpperCaseEncodedDoubleSlashThenNoException() throws Exception {
446+
this.firewall.setAllowUrlEncodedSlash(true);
447+
this.firewall.setAllowUrlEncodedDoubleSlash(true);
448+
MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
449+
request.setRequestURI("/context-root/a/b%2F%2Fc");
450+
request.setContextPath("/context-root");
451+
request.setServletPath("");
452+
request.setPathInfo("/a/b//c");
453+
assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException();
454+
}
455+
456+
@Test
457+
public void getFirewalledRequestWhenAllowUrlLowerCaseAndUpperCaseEncodedDoubleSlashThenNoException()
458+
throws Exception {
459+
this.firewall.setAllowUrlEncodedSlash(true);
460+
this.firewall.setAllowUrlEncodedDoubleSlash(true);
461+
MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
462+
request.setRequestURI("/context-root/a/b%2f%2Fc");
463+
request.setContextPath("/context-root");
464+
request.setServletPath("");
465+
request.setPathInfo("/a/b//c");
466+
assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException();
467+
}
468+
469+
@Test
470+
public void getFirewalledRequestWhenAllowUrlUpperCaseAndLowerCaseEncodedDoubleSlashThenNoException()
471+
throws Exception {
472+
this.firewall.setAllowUrlEncodedSlash(true);
473+
this.firewall.setAllowUrlEncodedDoubleSlash(true);
474+
MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
475+
request.setRequestURI("/context-root/a/b%2F%2fc");
476+
request.setContextPath("/context-root");
477+
request.setServletPath("");
478+
request.setPathInfo("/a/b//c");
479+
assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException();
480+
}
481+
482+
@Test
483+
public void getFirewalledRequestWhenRemoveFromUpperCaseEncodedUrlBlacklistThenNoException() throws Exception {
484+
this.firewall.setAllowUrlEncodedSlash(true);
485+
MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
486+
request.setRequestURI("/context-root/a/b%2F%2Fc");
487+
this.firewall.getEncodedUrlBlacklist().removeAll(Arrays.asList("%2F%2F"));
488+
assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException();
489+
}
490+
491+
@Test
492+
public void getFirewalledRequestWhenRemoveFromLowerCaseEncodedUrlBlacklistThenNoException() throws Exception {
493+
this.firewall.setAllowUrlEncodedSlash(true);
494+
MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
495+
request.setRequestURI("/context-root/a/b%2f%2fc");
496+
this.firewall.getEncodedUrlBlacklist().removeAll(Arrays.asList("%2f%2f"));
497+
assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException();
498+
}
499+
500+
@Test
501+
public void getFirewalledRequestWhenRemoveFromLowerCaseAndUpperCaseEncodedUrlBlacklistThenNoException()
502+
throws Exception {
503+
this.firewall.setAllowUrlEncodedSlash(true);
504+
MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
505+
request.setRequestURI("/context-root/a/b%2f%2Fc");
506+
this.firewall.getEncodedUrlBlacklist().removeAll(Arrays.asList("%2f%2F"));
507+
assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException();
508+
}
509+
510+
@Test
511+
public void getFirewalledRequestWhenRemoveFromUpperCaseAndLowerCaseEncodedUrlBlacklistThenNoException()
512+
throws Exception {
513+
this.firewall.setAllowUrlEncodedSlash(true);
514+
MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
515+
request.setRequestURI("/context-root/a/b%2F%2fc");
516+
this.firewall.getEncodedUrlBlacklist().removeAll(Arrays.asList("%2F%2f"));
517+
assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException();
518+
}
519+
520+
@Test
521+
public void getFirewalledRequestWhenRemoveFromDecodedUrlBlacklistThenNoException() throws Exception {
522+
MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
523+
request.setPathInfo("/a/b//c");
524+
this.firewall.getDecodedUrlBlacklist().removeAll(Arrays.asList("//"));
525+
assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException();
526+
}
431527
}

0 commit comments

Comments
 (0)