diff --git a/web/src/main/java/org/springframework/security/web/firewall/FirewalledResponse.java b/web/src/main/java/org/springframework/security/web/firewall/FirewalledResponse.java index afe0a58b0b..e831e0d189 100644 --- a/web/src/main/java/org/springframework/security/web/firewall/FirewalledResponse.java +++ b/web/src/main/java/org/springframework/security/web/firewall/FirewalledResponse.java @@ -37,7 +37,7 @@ class FirewalledResponse extends HttpServletResponseWrapper { @Override public void sendRedirect(String location) throws IOException { - // TODO: implement pluggable validation, instead of simple blacklisting. + // TODO: implement pluggable validation, instead of simple blocklist. // SEC-1790. Prevent redirects containing CRLF validateCrlf(LOCATION_HEADER, location); super.sendRedirect(location); 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 b7c752445b..c26b27daf6 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 @@ -98,23 +98,23 @@ public class StrictHttpFirewall implements HttpFirewall { private static final List FORBIDDEN_BACKSLASH = Collections.unmodifiableList(Arrays.asList("\\", "%5c", "%5C")); - private Set encodedUrlBlacklist = new HashSet<>(); + private Set encodedUrlBlocklist = new HashSet<>(); - private Set decodedUrlBlacklist = new HashSet<>(); + private Set decodedUrlBlocklist = new HashSet<>(); private Set allowedHttpMethods = createDefaultAllowedHttpMethods(); private Predicate allowedHostnames = hostname -> true; public StrictHttpFirewall() { - urlBlacklistsAddAll(FORBIDDEN_SEMICOLON); - urlBlacklistsAddAll(FORBIDDEN_FORWARDSLASH); - urlBlacklistsAddAll(FORBIDDEN_DOUBLE_FORWARDSLASH); - urlBlacklistsAddAll(FORBIDDEN_BACKSLASH); - - this.encodedUrlBlacklist.add(ENCODED_PERCENT); - this.encodedUrlBlacklist.addAll(FORBIDDEN_ENCODED_PERIOD); - this.decodedUrlBlacklist.add(PERCENT); + urlBlocklistsAddAll(FORBIDDEN_SEMICOLON); + urlBlocklistsAddAll(FORBIDDEN_FORWARDSLASH); + urlBlocklistsAddAll(FORBIDDEN_DOUBLE_FORWARDSLASH); + urlBlocklistsAddAll(FORBIDDEN_BACKSLASH); + + this.encodedUrlBlocklist.add(ENCODED_PERCENT); + this.encodedUrlBlocklist.addAll(FORBIDDEN_ENCODED_PERIOD); + this.decodedUrlBlocklist.add(PERCENT); } /** @@ -185,9 +185,9 @@ public class StrictHttpFirewall implements HttpFirewall { */ public void setAllowSemicolon(boolean allowSemicolon) { if (allowSemicolon) { - urlBlacklistsRemoveAll(FORBIDDEN_SEMICOLON); + urlBlocklistsRemoveAll(FORBIDDEN_SEMICOLON); } else { - urlBlacklistsAddAll(FORBIDDEN_SEMICOLON); + urlBlocklistsAddAll(FORBIDDEN_SEMICOLON); } } @@ -208,9 +208,9 @@ public class StrictHttpFirewall implements HttpFirewall { */ public void setAllowUrlEncodedSlash(boolean allowUrlEncodedSlash) { if (allowUrlEncodedSlash) { - urlBlacklistsRemoveAll(FORBIDDEN_FORWARDSLASH); + urlBlocklistsRemoveAll(FORBIDDEN_FORWARDSLASH); } else { - urlBlacklistsAddAll(FORBIDDEN_FORWARDSLASH); + urlBlocklistsAddAll(FORBIDDEN_FORWARDSLASH); } } @@ -225,9 +225,9 @@ public class StrictHttpFirewall implements HttpFirewall { */ public void setAllowUrlEncodedDoubleSlash(boolean allowUrlEncodedDoubleSlash) { if (allowUrlEncodedDoubleSlash) { - urlBlacklistsRemoveAll(FORBIDDEN_DOUBLE_FORWARDSLASH); + urlBlocklistsRemoveAll(FORBIDDEN_DOUBLE_FORWARDSLASH); } else { - urlBlacklistsAddAll(FORBIDDEN_DOUBLE_FORWARDSLASH); + urlBlocklistsAddAll(FORBIDDEN_DOUBLE_FORWARDSLASH); } } @@ -250,9 +250,9 @@ public class StrictHttpFirewall implements HttpFirewall { */ public void setAllowUrlEncodedPeriod(boolean allowUrlEncodedPeriod) { if (allowUrlEncodedPeriod) { - this.encodedUrlBlacklist.removeAll(FORBIDDEN_ENCODED_PERIOD); + this.encodedUrlBlocklist.removeAll(FORBIDDEN_ENCODED_PERIOD); } else { - this.encodedUrlBlacklist.addAll(FORBIDDEN_ENCODED_PERIOD); + this.encodedUrlBlocklist.addAll(FORBIDDEN_ENCODED_PERIOD); } } @@ -275,9 +275,9 @@ public class StrictHttpFirewall implements HttpFirewall { */ public void setAllowBackSlash(boolean allowBackSlash) { if (allowBackSlash) { - urlBlacklistsRemoveAll(FORBIDDEN_BACKSLASH); + urlBlocklistsRemoveAll(FORBIDDEN_BACKSLASH); } else { - urlBlacklistsAddAll(FORBIDDEN_BACKSLASH); + urlBlocklistsAddAll(FORBIDDEN_BACKSLASH); } } @@ -297,11 +297,11 @@ public class StrictHttpFirewall implements HttpFirewall { */ public void setAllowUrlEncodedPercent(boolean allowUrlEncodedPercent) { if (allowUrlEncodedPercent) { - this.encodedUrlBlacklist.remove(ENCODED_PERCENT); - this.decodedUrlBlacklist.remove(PERCENT); + this.encodedUrlBlocklist.remove(ENCODED_PERCENT); + this.decodedUrlBlocklist.remove(PERCENT); } else { - this.encodedUrlBlacklist.add(ENCODED_PERCENT); - this.decodedUrlBlacklist.add(PERCENT); + this.encodedUrlBlocklist.add(ENCODED_PERCENT); + this.decodedUrlBlocklist.add(PERCENT); } } @@ -320,20 +320,20 @@ public class StrictHttpFirewall implements HttpFirewall { this.allowedHostnames = allowedHostnames; } - private void urlBlacklistsAddAll(Collection values) { - this.encodedUrlBlacklist.addAll(values); - this.decodedUrlBlacklist.addAll(values); + private void urlBlocklistsAddAll(Collection values) { + this.encodedUrlBlocklist.addAll(values); + this.decodedUrlBlocklist.addAll(values); } - private void urlBlacklistsRemoveAll(Collection values) { - this.encodedUrlBlacklist.removeAll(values); - this.decodedUrlBlacklist.removeAll(values); + private void urlBlocklistsRemoveAll(Collection values) { + this.encodedUrlBlocklist.removeAll(values); + this.decodedUrlBlocklist.removeAll(values); } @Override public FirewalledRequest getFirewalledRequest(HttpServletRequest request) throws RequestRejectedException { rejectForbiddenHttpMethod(request); - rejectedBlacklistedUrls(request); + rejectedBlocklistedUrls(request); rejectedUntrustedHosts(request); if (!isNormalized(request)) { @@ -363,13 +363,13 @@ public class StrictHttpFirewall implements HttpFirewall { } } - private void rejectedBlacklistedUrls(HttpServletRequest request) { - for (String forbidden : this.encodedUrlBlacklist) { + private void rejectedBlocklistedUrls(HttpServletRequest request) { + for (String forbidden : this.encodedUrlBlocklist) { if (encodedUrlContains(request, forbidden)) { throw new RequestRejectedException("The request was rejected because the URL contained a potentially malicious String \"" + forbidden + "\""); } } - for (String forbidden : this.decodedUrlBlacklist) { + for (String forbidden : this.decodedUrlBlocklist) { if (decodedUrlContains(request, forbidden)) { throw new RequestRejectedException("The request was rejected because the URL contained a potentially malicious String \"" + forbidden + "\""); } @@ -481,20 +481,41 @@ public class StrictHttpFirewall implements HttpFirewall { } /** - * Provides the existing encoded url blacklist which can add/remove entries from + * Provides the existing encoded url blocklist which can add/remove entries from * - * @return the existing encoded url blacklist, never null + * @return the existing encoded url blocklist, never null */ + public Set getEncodedUrlBlocklist() { + return this.encodedUrlBlocklist; + } + + /** + * Provides the existing decoded url blocklist which can add/remove entries from + * + * @return the existing decoded url blocklist, never null + */ + public Set getDecodedUrlBlocklist() { + return this.decodedUrlBlocklist; + } + + /** + * Provides the existing encoded url blocklist which can add/remove entries from + * + * @return the existing encoded url blocklist, never null + * @deprecated Use {@link #getEncodedUrlBlocklist()} instead + */ + @Deprecated public Set getEncodedUrlBlacklist() { - return encodedUrlBlacklist; + return getEncodedUrlBlocklist(); } /** - * Provides the existing decoded url blacklist which can add/remove entries from + * Provides the existing decoded url blocklist which can add/remove entries from + * + * @return the existing decoded url blocklist, never null * - * @return the existing decoded url blacklist, never null */ public Set getDecodedUrlBlacklist() { - return decodedUrlBlacklist; + return getDecodedUrlBlocklist(); } } 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 8695ea7fa6..3de4acc59e 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 @@ -522,6 +522,52 @@ public class StrictHttpFirewallTests { assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException(); } + // blocklist + + @Test + public void getFirewalledRequestWhenRemoveFromUpperCaseEncodedUrlBlocklistThenNoException() { + this.firewall.setAllowUrlEncodedSlash(true); + MockHttpServletRequest request = new MockHttpServletRequest("GET", ""); + request.setRequestURI("/context-root/a/b%2F%2Fc"); + this.firewall.getEncodedUrlBlocklist().removeAll(Arrays.asList("%2F%2F")); + assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException(); + } + + @Test + public void getFirewalledRequestWhenRemoveFromLowerCaseEncodedUrlBlocklistThenNoException() { + this.firewall.setAllowUrlEncodedSlash(true); + MockHttpServletRequest request = new MockHttpServletRequest("GET", ""); + request.setRequestURI("/context-root/a/b%2f%2fc"); + this.firewall.getEncodedUrlBlocklist().removeAll(Arrays.asList("%2f%2f")); + assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException(); + } + + @Test + public void getFirewalledRequestWhenRemoveFromLowerCaseAndUpperCaseEncodedUrlBlocklistThenNoException() { + this.firewall.setAllowUrlEncodedSlash(true); + MockHttpServletRequest request = new MockHttpServletRequest("GET", ""); + request.setRequestURI("/context-root/a/b%2f%2Fc"); + this.firewall.getEncodedUrlBlocklist().removeAll(Arrays.asList("%2f%2F")); + assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException(); + } + + @Test + public void getFirewalledRequestWhenRemoveFromUpperCaseAndLowerCaseEncodedUrlBlocklistThenNoException() { + this.firewall.setAllowUrlEncodedSlash(true); + MockHttpServletRequest request = new MockHttpServletRequest("GET", ""); + request.setRequestURI("/context-root/a/b%2F%2fc"); + this.firewall.getEncodedUrlBlocklist().removeAll(Arrays.asList("%2F%2f")); + assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException(); + } + + @Test + public void getFirewalledRequestWhenRemoveFromDecodedUrlBlocklistThenNoException() { + MockHttpServletRequest request = new MockHttpServletRequest("GET", ""); + request.setPathInfo("/a/b//c"); + this.firewall.getDecodedUrlBlocklist().removeAll(Arrays.asList("//")); + assertThatCode(() -> this.firewall.getFirewalledRequest(request)).doesNotThrowAnyException(); + } + @Test public void getFirewalledRequestWhenTrustedDomainThenNoException() { this.request.addHeader("Host", "example.org");