From c0e66a9ba1a0615a7a96535c62aa5f7213b97cb2 Mon Sep 17 00:00:00 2001 From: guo fei Date: Tue, 15 Jan 2019 17:08:54 +0800 Subject: [PATCH] 1. add customization support for double forwardslash in StrickHttpFirewall 2. add getEncodedUrlBlacklist() and getDecodedUrlBlacklist() method in StrickHttpFirewall Fixes gh-6292 --- .../web/firewall/StrictHttpFirewall.java | 41 +++++++- .../web/firewall/StrictHttpFirewallTests.java | 96 +++++++++++++++++++ 2 files changed, 133 insertions(+), 4 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/firewall/StrictHttpFirewall.java b/web/src/main/java/org/springframework/security/web/firewall/StrictHttpFirewall.java index 03eaeeec16..4d6953e797 100644 --- a/web/src/main/java/org/springframework/security/web/firewall/StrictHttpFirewall.java +++ b/web/src/main/java/org/springframework/security/web/firewall/StrictHttpFirewall.java @@ -88,6 +88,8 @@ public class StrictHttpFirewall implements HttpFirewall { private static final List FORBIDDEN_FORWARDSLASH = Collections.unmodifiableList(Arrays.asList("%2f", "%2F")); + private static final List FORBIDDEN_DOUBLE_FORWARDSLASH = Collections.unmodifiableList(Arrays.asList("//", "%2f%2f", "%2f%2F", "%2F%2f", "%2F%2F")); + private static final List FORBIDDEN_BACKSLASH = Collections.unmodifiableList(Arrays.asList("\\", "%5c", "%5C")); private Set encodedUrlBlacklist = new HashSet(); @@ -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); @@ -203,6 +206,23 @@ public class StrictHttpFirewall implements HttpFirewall { } } + /** + *

+ * Determines if double slash "//" that is URL encoded "%2F%2F" should be allowed in the path or + * not. The default is to not allow. + *

+ * + * @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); + } + } + /** *

* Determines if a period "." that is URL encoded "%2E" should be allowed in the path @@ -412,10 +432,6 @@ public class StrictHttpFirewall implements HttpFirewall { 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; @@ -433,4 +449,21 @@ public class StrictHttpFirewall implements HttpFirewall { return true; } + /** + * Provides the existing encoded url blacklist which can add/remove entries from + * + * @return the existing encoded url blacklist, never null + */ + public Set 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 getDecodedUrlBlacklist() { + return decodedUrlBlacklist; + } } diff --git a/web/src/test/java/org/springframework/security/web/firewall/StrictHttpFirewallTests.java b/web/src/test/java/org/springframework/security/web/firewall/StrictHttpFirewallTests.java index 27e2c0c380..4de38e44c7 100644 --- a/web/src/test/java/org/springframework/security/web/firewall/StrictHttpFirewallTests.java +++ b/web/src/test/java/org/springframework/security/web/firewall/StrictHttpFirewallTests.java @@ -428,4 +428,100 @@ public class StrictHttpFirewallTests { 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(); + } }