From c71103aee0de5ca1ecd1768ee4e7d09db93626e1 Mon Sep 17 00:00:00 2001 From: Bogdan Date: Wed, 2 Sep 2026 20:22:35 +0200 Subject: [PATCH] fix(Cookie): validate raw cookie values --- system/Cookie/Cookie.php | 24 +++++ system/Cookie/Exceptions/CookieException.php | 10 ++ system/Language/en/Cookie.php | 1 + tests/system/Cookie/CookieTest.php | 102 +++++++++++++++++++ user_guide_src/source/changelogs/v4.7.5.rst | 3 + user_guide_src/source/libraries/cookies.rst | 7 ++ 6 files changed, 147 insertions(+) diff --git a/system/Cookie/Cookie.php b/system/Cookie/Cookie.php index b807e0885df4..27ffc908cb38 100644 --- a/system/Cookie/Cookie.php +++ b/system/Cookie/Cookie.php @@ -129,6 +129,11 @@ class Cookie implements ArrayAccess, CloneableCookieInterface */ private static string $reservedCharsList = "=,; \t\r\n\v\f()<>@:\\\"/[]?{}"; + /** + * @see https://www.php.net/manual/en/function.setrawcookie.php + */ + private static string $reservedValueCharsList = ",; \t\r\n\v\f\0"; + /** * Set the default attributes to a Cookie instance by injecting * the values from the `CookieConfig` config or an array. @@ -265,6 +270,7 @@ final public function __construct(string $name, string $value = '', array $optio $httponly = $options['httponly']; $this->validateName($name, $raw); + $this->validateValue($value, $raw); $this->validatePrefix($prefix, $secure, $path, $domain); $this->validateSameSite($samesite, $secure); @@ -470,6 +476,8 @@ public function withName(string $name) */ public function withValue(string $value) { + $this->validateValue($value, $this->raw); + $cookie = clone $this; $cookie->value = $value; @@ -578,6 +586,7 @@ public function withSameSite(string $samesite) public function withRaw(bool $raw = true) { $this->validateName($this->name, $raw); + $this->validateValue($this->value, $raw); $cookie = clone $this; @@ -766,6 +775,21 @@ protected function validateName(string $name, bool $raw): void } } + /** + * Validates the cookie value. + * + * If `$raw` is true, values should not contain invalid characters + * as `setrawcookie()` will reject this. + * + * @throws CookieException + */ + protected function validateValue(string $value, bool $raw): void + { + if ($raw && strpbrk($value, self::$reservedValueCharsList) !== false) { + throw CookieException::forInvalidCookieValue(); + } + } + /** * Validates the special prefixes if some attribute requirements are met. * diff --git a/system/Cookie/Exceptions/CookieException.php b/system/Cookie/Exceptions/CookieException.php index 8b6c7575983a..3cc7e32e6eff 100644 --- a/system/Cookie/Exceptions/CookieException.php +++ b/system/Cookie/Exceptions/CookieException.php @@ -60,6 +60,16 @@ public static function forEmptyCookieName() return new static(lang('Cookie.emptyCookieName')); } + /** + * Thrown when the cookie value contains invalid characters. + * + * @return static + */ + public static function forInvalidCookieValue() + { + return new static(lang('Cookie.invalidCookieValue')); + } + /** * Thrown when using the `__Secure-` prefix but the `Secure` attribute * is not set to true. diff --git a/system/Language/en/Cookie.php b/system/Language/en/Cookie.php index c880f032935e..4f4294ff0fc5 100644 --- a/system/Language/en/Cookie.php +++ b/system/Language/en/Cookie.php @@ -16,6 +16,7 @@ 'invalidExpiresTime' => 'Invalid "{0}" type for "Expires" attribute. Expected: string, integer, DateTimeInterface object.', 'invalidExpiresValue' => 'The cookie expiration time is not valid.', 'invalidCookieName' => 'The cookie name "{0}" contains invalid characters.', + 'invalidCookieValue' => 'The cookie value contains invalid characters.', 'emptyCookieName' => 'The cookie name cannot be empty.', 'invalidSecurePrefix' => 'Using the "__Secure-" prefix requires setting the "Secure" attribute.', 'invalidHostPrefix' => 'Using the "__Host-" prefix must be set with the "Secure" flag, must not have a "Domain" attribute, and the "Path" is set to "/".', diff --git a/tests/system/Cookie/CookieTest.php b/tests/system/Cookie/CookieTest.php index cade48b24f52..07991be006f6 100644 --- a/tests/system/Cookie/CookieTest.php +++ b/tests/system/Cookie/CookieTest.php @@ -341,4 +341,106 @@ public function testCannotUnsetPropertyViaArrayAccess(): void $cookie = new Cookie('cookie', 'monster'); unset($cookie['path']); } + + #[DataProvider('provideValidationOfRawCookieValue')] + public function testValidationOfRawCookieValue(string $value): void + { + $this->expectException(CookieException::class); + new Cookie('test', $value, ['raw' => true]); + } + + /** + * @return iterable + */ + public static function provideValidationOfRawCookieValue(): iterable + { + yield 'comma' => ['value,comma']; + + yield 'semicolon' => ['value;semicolon']; + + yield 'space' => ['value with space']; + + yield 'tab' => ["value\twith_tab"]; + + yield 'carriage return' => ["value\rcarriage"]; + + yield 'newline' => ["value\nnewline"]; + + yield 'vertical tab' => ["value\vvertical_tab"]; + + yield 'form feed' => ["value\fform_feed"]; + + yield 'null byte' => ["value\0null_byte"]; + + yield 'CRLF' => ["value\r\nwith_crlf"]; + } + + #[DataProvider('provideFromHeaderStringValidationOfRawCookieValue')] + public function testFromHeaderStringValidationOfRawCookieValue(string $value): void + { + $this->expectException(CookieException::class); + Cookie::fromHeaderString("test={$value}; Path=/", true); + } + + /** + * @return iterable + */ + public static function provideFromHeaderStringValidationOfRawCookieValue(): iterable + { + foreach (self::provideValidationOfRawCookieValue() as $name => $case) { + if ($name === 'semicolon') { + continue; + } + + yield $name => $case; + } + } + + public function testFromHeaderStringWithRawTrue(): void + { + $cookie = Cookie::fromHeaderString('test=valid_raw_value=123; Path=/', true); + + $this->assertTrue($cookie->isRaw()); + $this->assertSame('valid_raw_value=123', $cookie->getValue()); + } + + public function testFromHeaderStringWithRawFalseDecodesValue(): void + { + $cookie = Cookie::fromHeaderString('test=value%20with%20space; Path=/', false); + + $this->assertFalse($cookie->isRaw()); + $this->assertSame('value with space', $cookie->getValue()); + } + + public function testValidationOfRawCookieValueInWithValue(): void + { + $this->expectException(CookieException::class); + $cookie = new Cookie('test', 'valid_value', ['raw' => true]); + $cookie->withValue("injected\r\nvalue"); + } + + public function testValidationOfRawCookieValueInWithRaw(): void + { + $this->expectException(CookieException::class); + $cookie = new Cookie('test', "injected\r\nvalue", ['raw' => false]); + $cookie->withRaw(true); + } + + public function testValidRawCookieRetainsValueWithoutEncoding(): void + { + $cookie = new Cookie('test', 'valid_raw_value=123', ['raw' => true]); + + $this->assertSame('valid_raw_value=123', $cookie->getValue()); + $this->assertStringContainsString('test=valid_raw_value=123', (string) $cookie); + } + + public function testNonRawCookieSafelyEncodesCRLF(): void + { + $cookie = new Cookie('test', "value\r\nwith_crlf", ['raw' => false]); + $result = (string) $cookie; + + $this->assertStringContainsString('%0D%0A', $result); + $this->assertStringNotContainsString("\r", $result); + $this->assertStringNotContainsString("\n", $result); + } } diff --git a/user_guide_src/source/changelogs/v4.7.5.rst b/user_guide_src/source/changelogs/v4.7.5.rst index 47d447dfbbc3..6705029db943 100644 --- a/user_guide_src/source/changelogs/v4.7.5.rst +++ b/user_guide_src/source/changelogs/v4.7.5.rst @@ -18,6 +18,8 @@ BREAKING Message Changes *************** +- Added the ``Cookie.invalidCookieValue`` language string. + ******* Changes ******* @@ -36,6 +38,7 @@ Bugs Fixed - **CLIRequest:** Fixed a bug where ``parseCommand()`` could throw a TypeError when ``argv`` is missing. - **Content Security Policy:** Fixed a bug where empty ``Content-Security-Policy``, ``Content-Security-Policy-Report-Only``, and ``Reporting-Endpoints`` response headers were generated when no corresponding values existed. +- **Cookie:** Fixed a bug where ``Cookie`` instances created with ``raw: true`` allowed invalid characters in cookie values rejected by ``setrawcookie()``. - **Helpers:** Fixed a bug where ``get_dir_file_info()`` returned incomplete entries for subdirectories and missing files instead of omitting them. - **Honeypot:** Fixed a bug where bot detection returned an HTTP 500 response instead of 403 (Forbidden). - **Logger:** Fixed a bug where interpolating a log message with array or non-stringable context values could raise PHP warnings or errors. diff --git a/user_guide_src/source/libraries/cookies.rst b/user_guide_src/source/libraries/cookies.rst index 00239b0911f7..99b3103e45de 100644 --- a/user_guide_src/source/libraries/cookies.rst +++ b/user_guide_src/source/libraries/cookies.rst @@ -97,6 +97,13 @@ and `setrawcookie() `_ will reject cookies with invalid names. Additionally, cookie names cannot be an empty string. +Validating the Value Attribute +============================== + +If setting the ``$raw`` parameter to ``true``, the cookie value will also be validated. +It must not contain control characters, spaces, tabs, or separator characters +(``, ;``) as `setrawcookie() `_ will reject them. + Validating the Prefix Attribute ===============================