fix(Cookie): validate raw cookie values (#10516)

Author: gr8man

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.
      *

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.

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 "/".',

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<string, array{string}>
+     */
+    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<string, array{string}>
+     */
+    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);
+    }
 }

user_guide_src/source/changelogs/v4.7.5.rst

@@ -18,6 +18,8 @@ BREAKING
 Message Changes
 ***************
 
+- Added the ``Cookie.invalidCookieValue`` language string.
+
 *******
 Changes
 *******
@@ -38,6 +40,7 @@ Bugs Fixed
   ANSI color codes in the prompt (e.g., option defaults) are wrapped in readline's non-printing markers under GNU readline so cursor positioning stays accurate.
 - **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.

user_guide_src/source/libraries/cookies.rst

@@ -97,6 +97,13 @@ and `setrawcookie() <https://www.php.net/manual/en/function.setrawcookie.php>`_
 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() <https://www.php.net/manual/en/function.setrawcookie.php>`_ will reject them.
+
 Validating the Prefix Attribute
 ===============================