Harden RFC 10025 cookie parsing

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_NO_FF);
+			$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_NO_FF);
-		$value = trim($value, Trim::WHITESPACE_CHARS_NO_FF);
+		$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_NO_FF);
+					$part_value                  = trim($part_value, Trim::WHITESPACE_CHARS_RFC10025);
 				}
 
-				$part_key              = trim($part_key, Trim::WHITESPACE_CHARS_NO_FF);
+				$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

src/Utility/Trim.php

@@ -40,6 +40,20 @@ final class Trim {
 	 */
 	const WHITESPACE_CHARS_NO_FF = " \n\r\t\v\x00";
 
+	/**
+	 * Whitespace characters used to normalize cookie name/value data by RFC 10025.
+	 *
+	 * 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/rfc10025#section-5.6
+	 * @link https://www.rfc-editor.org/rfc/rfc5234#appendix-B.1
+	 *
+	 * @var string
+	 */
+	const WHITESPACE_CHARS_RFC10025 = " \t";
+
 	/**
 	 * The ASCII whitespace characters, including the form feed character, and the NUL byte.
 	 *

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.
 	 *
@@ -66,7 +100,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 +235,11 @@ public static function dataBasicNameValueParsing() {
 				'name'     => '',
 				'expected' => ['name' => 'foo', 'value' => 'bar'],
 			],
+			'RFC 10025 WSP includes horizontal tab' => [
+				'header'   => "\tfoo\t=\tbar\t",
+				'name'     => '',
+				'expected' => ['name' => 'foo', 'value' => 'bar'],
+			],
 		];
 	}
 
@@ -565,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.
 	 *