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 ad5cc2558f..75075be342 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 @@ -107,6 +107,15 @@ public class StrictHttpFirewall implements HttpFirewall { private static final List FORBIDDEN_NULL = Collections.unmodifiableList(Arrays.asList("\0", "%00")); + private static final List FORBIDDEN_LF = Collections.unmodifiableList(Arrays.asList("\n", "%0a", "%0A")); + + private static final List FORBIDDEN_CR = Collections.unmodifiableList(Arrays.asList("\r", "%0d", "%0D")); + + private static final List FORBIDDEN_LINE_SEPARATOR = Collections.unmodifiableList(Arrays.asList("\u2028")); + + private static final List FORBIDDEN_PARAGRAPH_SEPARATOR = Collections + .unmodifiableList(Arrays.asList("\u2029")); + private Set encodedUrlBlocklist = new HashSet<>(); private Set decodedUrlBlocklist = new HashSet<>(); @@ -135,10 +144,14 @@ public class StrictHttpFirewall implements HttpFirewall { urlBlocklistsAddAll(FORBIDDEN_DOUBLE_FORWARDSLASH); urlBlocklistsAddAll(FORBIDDEN_BACKSLASH); urlBlocklistsAddAll(FORBIDDEN_NULL); + urlBlocklistsAddAll(FORBIDDEN_LF); + urlBlocklistsAddAll(FORBIDDEN_CR); this.encodedUrlBlocklist.add(ENCODED_PERCENT); this.encodedUrlBlocklist.addAll(FORBIDDEN_ENCODED_PERIOD); this.decodedUrlBlocklist.add(PERCENT); + this.decodedUrlBlocklist.addAll(FORBIDDEN_LINE_SEPARATOR); + this.decodedUrlBlocklist.addAll(FORBIDDEN_PARAGRAPH_SEPARATOR); } /** @@ -345,6 +358,69 @@ public class StrictHttpFirewall implements HttpFirewall { } } + /** + * Determines if a URL encoded Carriage Return is allowed in the path or not. The + * default is not to allow this behavior because it is a frequent source of security + * exploits. + * @param allowUrlEncodedCarriageReturn if URL encoded Carriage Return is allowed in + * the URL or not. Default is false. + */ + public void setAllowUrlEncodedCarriageReturn(boolean allowUrlEncodedCarriageReturn) { + if (allowUrlEncodedCarriageReturn) { + urlBlocklistsRemoveAll(FORBIDDEN_CR); + } + else { + urlBlocklistsAddAll(FORBIDDEN_CR); + } + } + + /** + * Determines if a URL encoded Line Feed is allowed in the path or not. The default is + * not to allow this behavior because it is a frequent source of security exploits. + * @param allowUrlEncodedLineFeed if URL encoded Line Feed is allowed in the URL or + * not. Default is false. + */ + public void setAllowUrlEncodedLineFeed(boolean allowUrlEncodedLineFeed) { + if (allowUrlEncodedLineFeed) { + urlBlocklistsRemoveAll(FORBIDDEN_LF); + } + else { + urlBlocklistsAddAll(FORBIDDEN_LF); + } + } + + /** + * Determines if a URL encoded paragraph separator is allowed in the path or not. The + * default is not to allow this behavior because it is a frequent source of security + * exploits. + * @param allowUrlEncodedParagraphSeparator if URL encoded paragraph separator is + * allowed in the URL or not. Default is false. + */ + public void setAllowUrlEncodedParagraphSeparator(boolean allowUrlEncodedParagraphSeparator) { + if (allowUrlEncodedParagraphSeparator) { + this.decodedUrlBlocklist.removeAll(FORBIDDEN_PARAGRAPH_SEPARATOR); + } + else { + this.decodedUrlBlocklist.addAll(FORBIDDEN_PARAGRAPH_SEPARATOR); + } + } + + /** + * Determines if a URL encoded line separator is allowed in the path or not. The + * default is not to allow this behavior because it is a frequent source of security + * exploits. + * @param allowUrlEncodedLineSeparator if URL encoded line separator is allowed in the + * URL or not. Default is false. + */ + public void setAllowUrlEncodedLineSeparator(boolean allowUrlEncodedLineSeparator) { + if (allowUrlEncodedLineSeparator) { + this.decodedUrlBlocklist.removeAll(FORBIDDEN_LINE_SEPARATOR); + } + else { + this.decodedUrlBlocklist.addAll(FORBIDDEN_LINE_SEPARATOR); + } + } + /** *

* Determines which header names should be allowed. The default is to reject header @@ -432,9 +508,6 @@ public class StrictHttpFirewall implements HttpFirewall { throw new RequestRejectedException("The request was rejected because the URL was not normalized."); } rejectNonPrintableAsciiCharactersInFieldName(request.getRequestURI(), "requestURI"); - rejectNonPrintableAsciiCharactersInFieldName(request.getServletPath(), "servletPath"); - rejectNonPrintableAsciiCharactersInFieldName(request.getPathInfo(), "pathInfo"); - rejectNonPrintableAsciiCharactersInFieldName(request.getContextPath(), "contextPath"); return new StrictFirewalledRequest(request); } 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 e484ab6c04..1cbcdea4a9 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 @@ -342,6 +342,12 @@ public class StrictHttpFirewallTests { this.firewall.getFirewalledRequest(this.request); } + @Test + public void getFirewalledRequestWhenJapaneseCharacterThenNoException() { + this.request.setServletPath("/\u3042"); + this.firewall.getFirewalledRequest(this.request); + } + @Test public void getFirewalledRequestWhenExceedsUpperboundAsciiThenException() { this.request.setRequestURI("/\u007f"); @@ -363,6 +369,20 @@ public class StrictHttpFirewallTests { .isThrownBy(() -> this.firewall.getFirewalledRequest(this.request)); } + @Test + public void getFirewalledRequestWhenContainsLowercaseEncodedLineFeedThenException() { + this.request.setRequestURI("/something%0a/"); + assertThatExceptionOfType(RequestRejectedException.class) + .isThrownBy(() -> this.firewall.getFirewalledRequest(this.request)); + } + + @Test + public void getFirewalledRequestWhenContainsUppercaseEncodedLineFeedThenException() { + this.request.setRequestURI("/something%0A/"); + assertThatExceptionOfType(RequestRejectedException.class) + .isThrownBy(() -> this.firewall.getFirewalledRequest(this.request)); + } + @Test public void getFirewalledRequestWhenContainsLineFeedThenException() { this.request.setRequestURI("/something\n/"); @@ -377,6 +397,20 @@ public class StrictHttpFirewallTests { .isThrownBy(() -> this.firewall.getFirewalledRequest(this.request)); } + @Test + public void getFirewalledRequestWhenContainsLowercaseEncodedCarriageReturnThenException() { + this.request.setRequestURI("/something%0d/"); + assertThatExceptionOfType(RequestRejectedException.class) + .isThrownBy(() -> this.firewall.getFirewalledRequest(this.request)); + } + + @Test + public void getFirewalledRequestWhenContainsUppercaseEncodedCarriageReturnThenException() { + this.request.setRequestURI("/something%0D/"); + assertThatExceptionOfType(RequestRejectedException.class) + .isThrownBy(() -> this.firewall.getFirewalledRequest(this.request)); + } + @Test public void getFirewalledRequestWhenContainsCarriageReturnThenException() { this.request.setRequestURI("/something\r/"); @@ -391,6 +425,96 @@ public class StrictHttpFirewallTests { .isThrownBy(() -> this.firewall.getFirewalledRequest(this.request)); } + @Test + public void getFirewalledRequestWhenServletPathContainsLineSeparatorThenException() { + this.request.setServletPath("/something\u2028/"); + assertThatExceptionOfType(RequestRejectedException.class) + .isThrownBy(() -> this.firewall.getFirewalledRequest(this.request)); + } + + @Test + public void getFirewalledRequestWhenServletPathContainsParagraphSeparatorThenException() { + this.request.setServletPath("/something\u2029/"); + assertThatExceptionOfType(RequestRejectedException.class) + .isThrownBy(() -> this.firewall.getFirewalledRequest(this.request)); + } + + @Test + public void getFirewalledRequestWhenContainsLowercaseEncodedLineFeedAndAllowedThenNoException() { + this.firewall.setAllowUrlEncodedLineFeed(true); + this.request.setRequestURI("/something%0a/"); + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenContainsUppercaseEncodedLineFeedAndAllowedThenNoException() { + this.firewall.setAllowUrlEncodedLineFeed(true); + this.request.setRequestURI("/something%0A/"); + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenContainsLineFeedAndAllowedThenException() { + this.firewall.setAllowUrlEncodedLineFeed(true); + this.request.setRequestURI("/something\n/"); + // Expected an error because the line feed is decoded in an encoded part of the + // URL + assertThatExceptionOfType(RequestRejectedException.class) + .isThrownBy(() -> this.firewall.getFirewalledRequest(this.request)); + } + + @Test + public void getFirewalledRequestWhenServletPathContainsLineFeedAndAllowedThenNoException() { + this.firewall.setAllowUrlEncodedLineFeed(true); + this.request.setServletPath("/something\n/"); + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenContainsLowercaseEncodedCarriageReturnAndAllowedThenNoException() { + this.firewall.setAllowUrlEncodedCarriageReturn(true); + this.request.setRequestURI("/something%0d/"); + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenContainsUppercaseEncodedCarriageReturnAndAllowedThenNoException() { + this.firewall.setAllowUrlEncodedCarriageReturn(true); + this.request.setRequestURI("/something%0D/"); + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenContainsCarriageReturnAndAllowedThenNoException() { + this.firewall.setAllowUrlEncodedCarriageReturn(true); + this.request.setRequestURI("/something\r/"); + // Expected an error because the carriage return is decoded in an encoded part of + // the URL + assertThatExceptionOfType(RequestRejectedException.class) + .isThrownBy(() -> this.firewall.getFirewalledRequest(this.request)); + } + + @Test + public void getFirewalledRequestWhenServletPathContainsCarriageReturnAndAllowedThenNoException() { + this.firewall.setAllowUrlEncodedCarriageReturn(true); + this.request.setServletPath("/something\r/"); + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenServletPathContainsLineSeparatorAndAllowedThenNoException() { + this.firewall.setAllowUrlEncodedLineSeparator(true); + this.request.setServletPath("/something\u2028/"); + this.firewall.getFirewalledRequest(this.request); + } + + @Test + public void getFirewalledRequestWhenServletPathContainsParagraphSeparatorAndAllowedThenNoException() { + this.firewall.setAllowUrlEncodedParagraphSeparator(true); + this.request.setServletPath("/something\u2029/"); + this.firewall.getFirewalledRequest(this.request); + } + /** * On WebSphere 8.5 a URL like /context-root/a/b;%2f1/c can bypass a rule on /a/b/c * because the pathInfo is /a/b;/1/c which ends up being /a/b/1/c while Spring MVC