Skip to content

Commit

Permalink
1. add customization support for double forwardslash in StrickHttpFir…
Browse files Browse the repository at this point in the history 
…ewall

2. add getEncodedUrlBlacklist() and getDecodedUrlBlacklist() method in StrickHttpFirewall

Fixes gh-6292
  • Loading branch information
@ief2009
ief2009 committed Jan 15, 2019
1 parent 4ff5149 commit 1469a2c
Show file tree
Hide file tree
Showing 2 changed files with 133 additions and 4 deletions.
41 changes: 37 additions & 4 deletions web/src/main/java/org/springframework/security/web/firewall/StrictHttpFirewall.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ public class StrictHttpFirewall implements HttpFirewall {

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

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

private static final List<String> FORBIDDEN_BACKSLASH = Collections.unmodifiableList(Arrays.asList("\\", "%5c", "%5C"));

private Set<String> encodedUrlBlacklist = new HashSet<String>();
Expand All @@ -99,6 +101,7 @@ public class StrictHttpFirewall implements HttpFirewall {
public StrictHttpFirewall() {
urlBlacklistsAddAll(FORBIDDEN_SEMICOLON);
urlBlacklistsAddAll(FORBIDDEN_FORWARDSLASH);
urlBlacklistsAddAll(FORBIDDEN_DOUBLE_FORWARDSLASH);
urlBlacklistsAddAll(FORBIDDEN_BACKSLASH);

this.encodedUrlBlacklist.add(ENCODED_PERCENT);
Expand Down Expand Up @@ -203,6 +206,23 @@ public void setAllowUrlEncodedSlash(boolean allowUrlEncodedSlash) {
}
}

/**
* <p>
* Determines if double slash "//" that is URL encoded "%2F%2F" should be allowed in the path or
* not. The default is to not allow.
* </p>
*
* @param allowUrlEncodedDoubleSlash should a slash "//" that is URL encoded "%2F%2F" be allowed
* in the path or not. Default is false.
*/
public void setAllowUrlEncodedDoubleSlash(boolean allowUrlEncodedDoubleSlash) {
if (allowUrlEncodedDoubleSlash) {
urlBlacklistsRemoveAll(FORBIDDEN_DOUBLE_FORWARDSLASH);
} else {
urlBlacklistsAddAll(FORBIDDEN_DOUBLE_FORWARDSLASH);
}
}

/**
* <p>
* Determines if a period "." that is URL encoded "%2E" should be allowed in the path
Expand Down Expand Up @@ -412,10 +432,6 @@ private static boolean isNormalized(String path) {
return true;
}

if (path.indexOf("//") > -1) {
return false;
}

for (int j = path.length(); j > 0;) {
int i = path.lastIndexOf('/', j - 1);
int gap = j - i;
Expand All @@ -433,4 +449,21 @@ private static boolean isNormalized(String path) {
return true;
}

/**
* Provides the existing encoded url blacklist which can add/remove entries from
*
* @return the existing encoded url blacklist, never null
*/
public Set<String> getEncodedUrlBlacklist() {
return encodedUrlBlacklist;
}

/**
* Provides the existing decoded url blacklist which can add/remove entries from
*
* @return the existing decoded url blacklist, never null
*/
public Set<String> getDecodedUrlBlacklist() {
return decodedUrlBlacklist;
}
}
96 changes: 96 additions & 0 deletions web/src/test/java/org/springframework/security/web/firewall/StrictHttpFirewallTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -428,4 +428,100 @@ public void getFirewalledRequestWhenAllowUrlEncodedSlashAndUppercaseEncodedPathT

this.firewall.getFirewalledRequest(request);
}

@Test
public void getFirewalledRequestWhenAllowUrlLowerCaseEncodedDoubleSlashThenNoException() throws Exception {
this.firewall.setAllowUrlEncodedSlash(true);
this.firewall.setAllowUrlEncodedDoubleSlash(true);
MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
request.setRequestURI("/context-root/a/b%2f%2fc");
request.setContextPath("/context-root");
request.setServletPath("");
request.setPathInfo("/a/b//c");
assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException();
}

@Test
public void getFirewalledRequestWhenAllowUrlUpperCaseEncodedDoubleSlashThenNoException() throws Exception {
this.firewall.setAllowUrlEncodedSlash(true);
this.firewall.setAllowUrlEncodedDoubleSlash(true);
MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
request.setRequestURI("/context-root/a/b%2F%2Fc");
request.setContextPath("/context-root");
request.setServletPath("");
request.setPathInfo("/a/b//c");
assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException();
}

@Test
public void getFirewalledRequestWhenAllowUrlLowerCaseAndUpperCaseEncodedDoubleSlashThenNoException()
throws Exception {
this.firewall.setAllowUrlEncodedSlash(true);
this.firewall.setAllowUrlEncodedDoubleSlash(true);
MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
request.setRequestURI("/context-root/a/b%2f%2Fc");
request.setContextPath("/context-root");
request.setServletPath("");
request.setPathInfo("/a/b//c");
assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException();
}

@Test
public void getFirewalledRequestWhenAllowUrlUpperCaseAndLowerCaseEncodedDoubleSlashThenNoException()
throws Exception {
this.firewall.setAllowUrlEncodedSlash(true);
this.firewall.setAllowUrlEncodedDoubleSlash(true);
MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
request.setRequestURI("/context-root/a/b%2F%2fc");
request.setContextPath("/context-root");
request.setServletPath("");
request.setPathInfo("/a/b//c");
assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException();
}

@Test
public void getFirewalledRequestWhenRemoveFromUpperCaseEncodedUrlBlacklistThenNoException() throws Exception {
this.firewall.setAllowUrlEncodedSlash(true);
MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
request.setRequestURI("/context-root/a/b%2F%2Fc");
this.firewall.getEncodedUrlBlacklist().removeAll(Arrays.asList("%2F%2F"));
assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException();
}

@Test
public void getFirewalledRequestWhenRemoveFromLowerCaseEncodedUrlBlacklistThenNoException() throws Exception {
this.firewall.setAllowUrlEncodedSlash(true);
MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
request.setRequestURI("/context-root/a/b%2f%2fc");
this.firewall.getEncodedUrlBlacklist().removeAll(Arrays.asList("%2f%2f"));
assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException();
}

@Test
public void getFirewalledRequestWhenRemoveFromLowerCaseAndUpperCaseEncodedUrlBlacklistThenNoException()
throws Exception {
this.firewall.setAllowUrlEncodedSlash(true);
MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
request.setRequestURI("/context-root/a/b%2f%2Fc");
this.firewall.getEncodedUrlBlacklist().removeAll(Arrays.asList("%2f%2F"));
assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException();
}

@Test
public void getFirewalledRequestWhenRemoveFromUpperCaseAndLowerCaseEncodedUrlBlacklistThenNoException()
throws Exception {
this.firewall.setAllowUrlEncodedSlash(true);
MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
request.setRequestURI("/context-root/a/b%2F%2fc");
this.firewall.getEncodedUrlBlacklist().removeAll(Arrays.asList("%2F%2f"));
assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException();
}

@Test
public void getFirewalledRequestWhenRemoveFromDecodedUrlBlacklistThenNoException() throws Exception {
MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
request.setPathInfo("/a/b//c");
this.firewall.getDecodedUrlBlacklist().removeAll(Arrays.asList("//"));
assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException();
}
}

0 comments on commit 1469a2c

Please sign in to comment.