From 77bb01179aa2c69e8b654aba4155e4a7cbdf4f14 Mon Sep 17 00:00:00 2001 From: Asllan Maciel Date: Thu, 17 Sep 2026 08:14:59 -0300 Subject: [PATCH 1/2] Harden --- src/Cookie.php | 10 +++++----- src/Utility/Trim.php | 13 +++++++++++++ tests/Cookie/ParseTest.php | 14 +++++++++++++- 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/src/Cookie.php b/src/Cookie.php index 9075c25e7..d297584ee 100644 --- a/src/Cookie.php +++ b/src/Cookie.php @@ -441,7 +441,7 @@ public static function parse($cookie_header, $name = '', $reference_time = null) } if (is_string($name)) { - $name = trim($name, Trim::WHITESPACE_CHARS_NO_FF); + $name = trim($name, Trim::WHITESPACE_CHARS_RFC6265); } if ($name !== '' && InputValidator::is_valid_rfc2616_token($name) === false) { @@ -465,8 +465,8 @@ public static function parse($cookie_header, $name = '', $reference_time = null) list($name, $value) = explode('=', $kvparts, 2); } - $name = trim($name, Trim::WHITESPACE_CHARS_NO_FF); - $value = trim($value, Trim::WHITESPACE_CHARS_NO_FF); + $name = trim($name, Trim::WHITESPACE_CHARS_RFC6265); + $value = trim($value, Trim::WHITESPACE_CHARS_RFC6265); if ($name !== '' && InputValidator::is_valid_rfc2616_token($name) === false) { throw InvalidArgument::create(2, '$name', 'integer|string and conform to RFC 2616', gettype($name)); @@ -482,10 +482,10 @@ public static function parse($cookie_header, $name = '', $reference_time = null) $part_value = true; } else { list($part_key, $part_value) = explode('=', $part, 2); - $part_value = trim($part_value, Trim::WHITESPACE_CHARS_NO_FF); + $part_value = trim($part_value, Trim::WHITESPACE_CHARS_RFC6265); } - $part_key = trim($part_key, Trim::WHITESPACE_CHARS_NO_FF); + $part_key = trim($part_key, Trim::WHITESPACE_CHARS_RFC6265); $attributes[$part_key] = $part_value; } } diff --git a/src/Utility/Trim.php b/src/Utility/Trim.php index 862cb1ac9..996ea7fa8 100644 --- a/src/Utility/Trim.php +++ b/src/Utility/Trim.php @@ -40,6 +40,19 @@ final class Trim { */ const WHITESPACE_CHARS_NO_FF = " \n\r\t\v\x00"; + /** + * Whitespace characters allowed around cookie name/value data by RFC 6265. + * + * Section 5.2 requires leading and trailing WSP to be removed, where WSP is + * defined by RFC 5234 as SP / HTAB. Other control characters are not WSP. + * + * @link https://www.rfc-editor.org/rfc/rfc6265#section-5.2 + * @link https://www.rfc-editor.org/rfc/rfc5234#appendix-B.1 + * + * @var string + */ + const WHITESPACE_CHARS_RFC6265 = " \t"; + /** * The ASCII whitespace characters, including the form feed character, and the NUL byte. * diff --git a/tests/Cookie/ParseTest.php b/tests/Cookie/ParseTest.php index 2ad488a26..278387453 100644 --- a/tests/Cookie/ParseTest.php +++ b/tests/Cookie/ParseTest.php @@ -66,7 +66,9 @@ public function testParseInvalidName($input) { */ public static function dataParseInvalidName() { $data = TypeProviderHelper::getAllExcept(TypeProviderHelper::GROUP_INT, TypeProviderHelper::GROUP_STRING); - $data['Valid string, but not a valid RFC 2616 token'] = ["some\ntext\rwith\tcontrol\echaracters\fin\vit"]; + $data['Valid string, but not a valid RFC 2616 token'] = ["some\ntext\rwith\tcontrol\echaracters\fin\vit"]; + $data['Valid token surrounded by LF is not valid cookie whitespace'] = ["\nvalid-name\n"]; + $data['Valid token surrounded by VT is not valid cookie whitespace'] = ["\vvalid-name\v"]; return $data; } @@ -199,6 +201,16 @@ public static function dataBasicNameValueParsing() { 'name' => '', 'expected' => ['name' => 'foo', 'value' => 'bar'], ], + 'RFC 6265 WSP includes horizontal tab' => [ + 'header' => "\tfoo\t=\tbar\t", + 'name' => '', + 'expected' => ['name' => 'foo', 'value' => 'bar'], + ], + 'Non-WSP control characters are not stripped from cookie values' => [ + 'header' => "foo=\vbar\v", + 'name' => '', + 'expected' => ['name' => 'foo', 'value' => "\vbar\v"], + ], ]; } From 9bdaa17b81d8daa7c7850d8386f2af53ac6db2c1 Mon Sep 17 00:00:00 2001 From: Asllan Maciel Date: Fri, 18 Sep 2026 08:21:52 -0300 Subject: [PATCH 2/2] Harden Set-Cookie control character handling --- src/Cookie.php | 35 ++++++++++++++++---- src/Utility/Trim.php | 9 +++--- tests/Cookie/ParseTest.php | 65 ++++++++++++++++++++++++++++++++++---- 3 files changed, 92 insertions(+), 17 deletions(-) diff --git a/src/Cookie.php b/src/Cookie.php index d297584ee..906c76207 100644 --- a/src/Cookie.php +++ b/src/Cookie.php @@ -420,12 +420,25 @@ public function format_for_set_cookie() { return $header_value; } + /** + * Check whether a Set-Cookie string contains a control character disallowed by RFC 10025. + * + * HTAB (%x09) is intentionally excluded: it is valid WSP and is normalized separately. + * + * @param string $cookie_header Cookie header value. + * + * @return bool + */ + private static function contains_disallowed_cookie_control_character($cookie_header) { + return preg_match('/[\x00-\x08\x0A-\x1F\x7F]/', $cookie_header) === 1; + } + /** * Parse a cookie string into a cookie object * * Based on Mozilla's parsing code in Firefox and related projects, which - * is an intentional deviation from RFC 2109 and RFC 2616. RFC 6265 - * specifies some of this handling, but not in a thorough manner. + * is an intentional deviation from RFC 2109 and RFC 2616. RFC 10025 + * defines Set-Cookie parsing rules; Requests retains documented compatibility deviations where needed. * * @param int|string $cookie_header Cookie header value (from a Set-Cookie header) * @param string $name @@ -440,8 +453,12 @@ public static function parse($cookie_header, $name = '', $reference_time = null) throw InvalidArgument::create(1, '$cookie_header', 'string', gettype($cookie_header)); } + if (self::contains_disallowed_cookie_control_character($cookie_header)) { + throw new InvalidArgument('Cookie header contains a disallowed control character per RFC 10025'); + } + if (is_string($name)) { - $name = trim($name, Trim::WHITESPACE_CHARS_RFC6265); + $name = trim($name, Trim::WHITESPACE_CHARS_RFC10025); } if ($name !== '' && InputValidator::is_valid_rfc2616_token($name) === false) { @@ -465,8 +482,8 @@ public static function parse($cookie_header, $name = '', $reference_time = null) list($name, $value) = explode('=', $kvparts, 2); } - $name = trim($name, Trim::WHITESPACE_CHARS_RFC6265); - $value = trim($value, Trim::WHITESPACE_CHARS_RFC6265); + $name = trim($name, Trim::WHITESPACE_CHARS_RFC10025); + $value = trim($value, Trim::WHITESPACE_CHARS_RFC10025); if ($name !== '' && InputValidator::is_valid_rfc2616_token($name) === false) { throw InvalidArgument::create(2, '$name', 'integer|string and conform to RFC 2616', gettype($name)); @@ -482,10 +499,10 @@ public static function parse($cookie_header, $name = '', $reference_time = null) $part_value = true; } else { list($part_key, $part_value) = explode('=', $part, 2); - $part_value = trim($part_value, Trim::WHITESPACE_CHARS_RFC6265); + $part_value = trim($part_value, Trim::WHITESPACE_CHARS_RFC10025); } - $part_key = trim($part_key, Trim::WHITESPACE_CHARS_RFC6265); + $part_key = trim($part_key, Trim::WHITESPACE_CHARS_RFC10025); $attributes[$part_key] = $part_value; } } @@ -515,6 +532,10 @@ public static function parse_from_headers(Headers $headers, $origin = null, $tim $cookies = []; foreach ($cookie_headers as $header) { + if (self::contains_disallowed_cookie_control_character($header)) { + continue; + } + $parsed = self::parse($header, '', $time); // Default domain/path attributes diff --git a/src/Utility/Trim.php b/src/Utility/Trim.php index 996ea7fa8..cc3b7437a 100644 --- a/src/Utility/Trim.php +++ b/src/Utility/Trim.php @@ -41,17 +41,18 @@ final class Trim { const WHITESPACE_CHARS_NO_FF = " \n\r\t\v\x00"; /** - * Whitespace characters allowed around cookie name/value data by RFC 6265. + * Whitespace characters used to normalize cookie name/value data by RFC 10025. * - * Section 5.2 requires leading and trailing WSP to be removed, where WSP is + * Section 5.6 requires leading and trailing WSP to be removed after disallowed + * control characters are rejected. WSP is * defined by RFC 5234 as SP / HTAB. Other control characters are not WSP. * - * @link https://www.rfc-editor.org/rfc/rfc6265#section-5.2 + * @link https://www.rfc-editor.org/rfc/rfc10025#section-5.6 * @link https://www.rfc-editor.org/rfc/rfc5234#appendix-B.1 * * @var string */ - const WHITESPACE_CHARS_RFC6265 = " \t"; + const WHITESPACE_CHARS_RFC10025 = " \t"; /** * The ASCII whitespace characters, including the form feed character, and the NUL byte. diff --git a/tests/Cookie/ParseTest.php b/tests/Cookie/ParseTest.php index 278387453..d54ed7a8f 100644 --- a/tests/Cookie/ParseTest.php +++ b/tests/Cookie/ParseTest.php @@ -32,6 +32,40 @@ public function testParseInvalidCookieHeader($input) { Cookie::parse($input); } + /** + * Tests receiving an exception for Set-Cookie strings containing control characters disallowed by RFC 10025. + * + * @dataProvider dataInvalidCookieHeaderControlCharacters + * + * @covers ::parse + * + * @param string $input Cookie header containing a disallowed control character. + * + * @return void + */ + public function testParseInvalidCookieHeaderControlCharacter($input) { + $this->expectException(InvalidArgument::class); + $this->expectExceptionMessage('disallowed control character'); + + Cookie::parse($input); + } + + /** + * Data provider. + * + * @return array + */ + public static function dataInvalidCookieHeaderControlCharacters() { + $data = []; + $codepoints = array_merge(range(0x00, 0x08), range(0x0A, 0x1F), [0x7F]); + + foreach ($codepoints as $codepoint) { + $data[sprintf('CTL 0x%02X', $codepoint)] = [sprintf('foo=ba%sr', chr($codepoint))]; + } + + return $data; + } + /** * Data Provider. * @@ -201,16 +235,11 @@ public static function dataBasicNameValueParsing() { 'name' => '', 'expected' => ['name' => 'foo', 'value' => 'bar'], ], - 'RFC 6265 WSP includes horizontal tab' => [ + 'RFC 10025 WSP includes horizontal tab' => [ 'header' => "\tfoo\t=\tbar\t", 'name' => '', 'expected' => ['name' => 'foo', 'value' => 'bar'], ], - 'Non-WSP control characters are not stripped from cookie values' => [ - 'header' => "foo=\vbar\v", - 'name' => '', - 'expected' => ['name' => 'foo', 'value' => "\vbar\v"], - ], ]; } @@ -577,6 +606,30 @@ public static function dataParsingHeaderWithOrigin() { ]; } + /** + * Verify Set-Cookie headers containing disallowed control characters are ignored. + * + * RFC 10025 section 5.6 requires user agents to ignore an entire Set-Cookie + * string containing CTLs other than HTAB. + * + * @covers ::parse_from_headers + * + * @return void + */ + public function testParsingHeaderIgnoresDisallowedControlCharacters() { + $headers = new Headers(); + $headers['Set-Cookie'] = 'valid=first'; + $headers['Set-Cookie'] = "invalid=va\vlue"; + $headers['Set-Cookie'] = 'another=valid'; + + $parsed = Cookie::parse_from_headers($headers); + + $this->assertCount(2, $parsed); + $this->assertArrayHasKey('valid', $parsed); + $this->assertArrayHasKey('another', $parsed); + $this->assertArrayNotHasKey('invalid', $parsed); + } + /** * Verify handling of Headers object with multiple `Set-Cookie` headers. *