Horizontal tab (0x09) in HTTP header values rejected by StrictHttpFirewall

[chbecker2]

Describe the bug
I have a problem where a customer is sending a HTTP request to my Spring application and gets rejected by the StrictHttpFirewall because the request contains a header value with a horizontal tab character (0x09).

According to the specification RFC 2616 (or the newer RFC 9110) HTTP header field values are allowed to contain HTABs. This seems to be a bug in the implementaion of StrictHttpFirewall.

RFC 9110, chapter 5.5 (https://datatracker.ietf.org/doc/html/rfc9110#name-field-values)

  field-value    = *field-content
  field-content  = field-vchar
                   [ 1*( SP / HTAB / field-vchar ) field-vchar ]
  field-vchar    = VCHAR / obs-text
  obs-text       = %x80-FF

To Reproduce
Send a HTTP request with a header value containing a tab (0x09) character to an application which is using Spring StrictHttpFirewall .

Expected behavior
The HTTP request shall not be rejected.

Sample
Here is a change proposal to allow HTAB characters in header values:

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 585ecd503f..09193b0f3c 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
@@ -130,9 +130,13 @@ public class StrictHttpFirewall implements HttpFirewall {
        private static final Predicate<String> ASSIGNED_AND_NOT_ISO_CONTROL_PREDICATE = (
                        s) -> ASSIGNED_AND_NOT_ISO_CONTROL_PATTERN.matcher(s).matches();

+       private static final Pattern HEADER_VALUE_PATTERN = Pattern.compile("[\\p{IsAssigned}&&[[^\\p{IsControl}]||\\t]]*");
+
+       private static final Predicate<String> HEADER_VALUE_PREDICATE = (s) -> HEADER_VALUE_PATTERN.matcher(s).matches();
+
        private Predicate<String> allowedHeaderNames = ASSIGNED_AND_NOT_ISO_CONTROL_PREDICATE;

-       private Predicate<String> allowedHeaderValues = ASSIGNED_AND_NOT_ISO_CONTROL_PREDICATE;
+       private Predicate<String> allowedHeaderValues = HEADER_VALUE_PREDICATE;

        private Predicate<String> allowedParameterNames = ASSIGNED_AND_NOT_ISO_CONTROL_PREDICATE;

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 6368faafe3..9c7b84d578 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
@@ -781,6 +781,13 @@ public class StrictHttpFirewallTests {
                assertThatExceptionOfType(RequestRejectedException.class).isThrownBy(() -> request.getHeader("Something"));
        }

+       @Test
+       public void getFirewalledRequestGetHeaderWhenHorizontalTabInHeaderValueThenNoException() {
+               this.request.addHeader("Something", "tab\tvalue");
+               HttpServletRequest request = this.firewall.getFirewalledRequest(this.request);
+               assertThat(request.getHeader("Something")).isEqualTo("tab\tvalue");
+       }
+
        @Test
        public void getFirewalledRequestGetHeaderWhenUndefinedCharacterInHeaderValueThenException() {
                this.request.addHeader("Something", "bad\uFFFEvalue");

[jzheaux]

Thanks for the suggestion, @chbecker2. Are you able to submit a PR that contains the described changes?

[chbecker2]

I added a PR for the main branch. Will this be automatically applied to all active branches or do I need to create separate PRs for each branch? I'm specifically interested in a patch for th 5.8.x branch.

[jzheaux]

Thanks, @chbecker2. Can you rebase it off of the 5.8.x branch please since that's the earliest branch where the bug exists? I'll forward port it to the other branches after merging.

Also, will you please do me the housekeeping favor of updating the copyright years in those two files to end in 2024?

[chbecker2]

Hi @jzheaux, I provided the same change in 5.8.x with #14590 and updated the copyright dates