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 dfb0d73bfb..aa63e0dd2f 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 @@ -75,6 +75,7 @@ import org.springframework.util.Assert; * * @author Rob Winch * @author Eddú Meléndez + * @author Jinwoo Bae * @since 4.2.4 * @see DefaultHttpFirewall */ @@ -134,13 +135,21 @@ public class StrictHttpFirewall implements HttpFirewall { private static final Predicate HEADER_VALUE_PREDICATE = (s) -> HEADER_VALUE_PATTERN.matcher(s).matches(); - private Predicate allowedHeaderNames = ASSIGNED_AND_NOT_ISO_CONTROL_PREDICATE; + private Predicate allowedHeaderNames = ALLOWED_HEADER_NAMES; - private Predicate allowedHeaderValues = HEADER_VALUE_PREDICATE; + public static final Predicate ALLOWED_HEADER_NAMES = ASSIGNED_AND_NOT_ISO_CONTROL_PREDICATE; - private Predicate allowedParameterNames = ASSIGNED_AND_NOT_ISO_CONTROL_PREDICATE; + private Predicate allowedHeaderValues = ALLOWED_HEADER_VALUES; - private Predicate allowedParameterValues = (value) -> true; + public static final Predicate ALLOWED_HEADER_VALUES = HEADER_VALUE_PREDICATE; + + private Predicate allowedParameterNames = ALLOWED_PARAMETER_NAMES; + + public static final Predicate ALLOWED_PARAMETER_NAMES = ASSIGNED_AND_NOT_ISO_CONTROL_PREDICATE; + + private Predicate allowedParameterValues = ALLOWED_PARAMETER_VALUES; + + public static final Predicate ALLOWED_PARAMETER_VALUES = (value) -> true; public StrictHttpFirewall() { urlBlocklistsAddAll(FORBIDDEN_SEMICOLON); 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 98b398a91a..f5406f0094 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 @@ -31,6 +31,7 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType; /** * @author Rob Winch * @author Eddú Meléndez + * @author Jinwoo Bae */ public class StrictHttpFirewallTests { @@ -723,6 +724,14 @@ public class StrictHttpFirewallTests { assertThatExceptionOfType(RequestRejectedException.class).isThrownBy(() -> request.getHeader("bad name")); } + @Test + public void getFirewalledRequestWhenHeaderNameNotAllowedWithAugmentedHeaderNamesThenException() { + this.firewall + .setAllowedHeaderNames(StrictHttpFirewall.ALLOWED_HEADER_NAMES.and((name) -> !name.equals("bad name"))); + HttpServletRequest request = this.firewall.getFirewalledRequest(this.request); + assertThatExceptionOfType(RequestRejectedException.class).isThrownBy(() -> request.getHeader("bad name")); + } + @Test public void getFirewalledRequestGetHeaderWhenNotAllowedHeaderValueThenException() { this.request.addHeader("good name", "bad value"); @@ -731,6 +740,15 @@ public class StrictHttpFirewallTests { assertThatExceptionOfType(RequestRejectedException.class).isThrownBy(() -> request.getHeader("good name")); } + @Test + public void getFirewalledRequestWhenHeaderValueNotAllowedWithAugmentedHeaderValuesThenException() { + this.request.addHeader("good name", "bad value"); + this.firewall.setAllowedHeaderValues( + StrictHttpFirewall.ALLOWED_HEADER_VALUES.and((value) -> !value.equals("bad value"))); + HttpServletRequest request = this.firewall.getFirewalledRequest(this.request); + assertThatExceptionOfType(RequestRejectedException.class).isThrownBy(() -> request.getHeader("good name")); + } + @Test public void getFirewalledRequestGetDateHeaderWhenControlCharacterInHeaderNameThenException() { this.request.addHeader("Bad\0Name", "some value"); @@ -840,6 +858,16 @@ public class StrictHttpFirewallTests { .isThrownBy(() -> request.getParameterValues("Something")); } + @Test + public void getFirewalledRequestWhenParameterValueNotAllowedWithAugmentedParameterValuesThenException() { + this.request.addParameter("Something", "bad value"); + this.firewall.setAllowedParameterValues( + StrictHttpFirewall.ALLOWED_PARAMETER_VALUES.and((value) -> !value.equals("bad value"))); + HttpServletRequest request = this.firewall.getFirewalledRequest(this.request); + assertThatExceptionOfType(RequestRejectedException.class) + .isThrownBy(() -> request.getParameterValues("Something")); + } + @Test public void getFirewalledRequestGetParameterValuesWhenNotAllowedInParameterNameThenException() { this.firewall.setAllowedParameterNames((value) -> !value.equals("bad name")); @@ -849,6 +877,16 @@ public class StrictHttpFirewallTests { .isThrownBy(() -> request.getParameterValues("bad name")); } + @Test + public void getFirewalledRequestWhenParameterNameNotAllowedWithAugmentedParameterNamesThenException() { + this.request.addParameter("bad name", "good value"); + this.firewall.setAllowedParameterNames( + StrictHttpFirewall.ALLOWED_PARAMETER_NAMES.and((value) -> !value.equals("bad name"))); + HttpServletRequest request = this.firewall.getFirewalledRequest(this.request); + assertThatExceptionOfType(RequestRejectedException.class) + .isThrownBy(() -> request.getParameterValues("bad name")); + } + // gh-9598 @Test public void getFirewalledRequestGetParameterWhenNameIsNullThenIllegalArgumentException() {