fix(Cookie): validate cookie path and domain attributes (#10527)

Author: gr8man

system/Cookie/Cookie.php

@@ -127,7 +127,14 @@ class Cookie implements ArrayAccess, CloneableCookieInterface
      * @see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#attributes
      * @see https://tools.ietf.org/html/rfc2616#section-2.2
      */
-    private static string $reservedCharsList = "=,; \t\r\n\v\f()<>@:\\\"/[]?{}";
+    private static string $reservedCharsList = "=,; \t\r\n\v\f\0()<>@:\\\"/[]?{}";
+
+    /**
+     * Prohibited characters in cookie prefix and name per PHP setcookie() constraints.
+     *
+     * @see https://www.php.net/manual/en/function.setcookie.php
+     */
+    private static string $reservedPrefixCharsList = "=,; \t\r\n\v\f\0";
 
     /**
      * @see https://www.php.net/manual/en/function.setrawcookie.php
@@ -232,11 +239,11 @@ public static function fromHeaderString(string $cookie, bool $raw = false)
      * @param string $name  The cookie's name
      * @param string $value The cookie's value
      * @param array{
-     *   prefix?: string,
+     *   prefix?: string|null,
      *   max-age?: int|numeric-string,
      *   expires?: DateTimeInterface|int|string,
-     *   path?: string,
-     *   domain?: string,
+     *   path?: string|null,
+     *   domain?: string|null,
      *   secure?: bool,
      *   httponly?: bool,
      *   samesite?: string,
@@ -258,9 +265,9 @@ final public function __construct(string $name, string $value = '', array $optio
         }
 
         // to preserve backward compatibility with array-based cookies in previous CI versions
-        $prefix = ($options['prefix'] === '') ? self::$defaults['prefix'] : $options['prefix'];
-        $path   = ($options['path'] === '') ? self::$defaults['path'] : $options['path'];
-        $domain = ($options['domain'] === '') ? self::$defaults['domain'] : $options['domain'];
+        $prefix = in_array($options['prefix'], [null, ''], true) ? self::$defaults['prefix'] : $options['prefix'];
+        $path   = in_array($options['path'], [null, '', '0'], true) ? self::$defaults['path'] : $options['path'];
+        $domain = in_array($options['domain'], [null, ''], true) ? self::$defaults['domain'] : $options['domain'];
 
         // empty string SameSite should use the default for browsers
         $samesite = ($options['samesite'] === '') ? self::$defaults['samesite'] : $options['samesite'];
@@ -271,6 +278,8 @@ final public function __construct(string $name, string $value = '', array $optio
 
         $this->validateName($name, $raw);
         $this->validateValue($value, $raw);
+        $this->validatePath($path);
+        $this->validateDomain($domain);
         $this->validatePrefix($prefix, $secure, $path, $domain);
         $this->validateSameSite($samesite, $secure);
 
@@ -515,6 +524,7 @@ public function withExpired()
     public function withPath(?string $path)
     {
         $path = in_array($path, [null, '', '0'], true) ? self::$defaults['path'] : $path;
+        $this->validatePath($path);
         $this->validatePrefix($this->prefix, $this->secure, $path, $this->domain);
 
         $cookie = clone $this;
@@ -530,6 +540,7 @@ public function withPath(?string $path)
     public function withDomain(?string $domain)
     {
         $domain ??= self::$defaults['domain'];
+        $this->validateDomain($domain);
         $this->validatePrefix($this->prefix, $this->secure, $this->path, $domain);
 
         $cookie = clone $this;
@@ -791,12 +802,41 @@ protected function validateValue(string $value, bool $raw): void
     }
 
     /**
-     * Validates the special prefixes if some attribute requirements are met.
+     * Validates the cookie path per PHP setcookie() constraints.
+     *
+     * @throws CookieException
+     */
+    protected function validatePath(string $path): void
+    {
+        if (strpbrk($path, self::$reservedValueCharsList) !== false) {
+            throw CookieException::forInvalidCookiePath();
+        }
+    }
+
+    /**
+     * Validates the cookie domain per PHP setcookie() constraints.
+     *
+     * @throws CookieException
+     */
+    protected function validateDomain(string $domain): void
+    {
+        if ($domain !== '' && strpbrk($domain, self::$reservedValueCharsList) !== false) {
+            throw CookieException::forInvalidCookieDomain();
+        }
+    }
+
+    /**
+     * Validates the special prefixes if some attribute requirements are met,
+     * and ensures the prefix contains no PHP-prohibited characters.
      *
      * @throws CookieException
      */
     protected function validatePrefix(string $prefix, bool $secure, string $path, string $domain): void
     {
+        if (strpbrk($prefix, self::$reservedPrefixCharsList) !== false) {
+            throw CookieException::forInvalidCookieName($prefix);
+        }
+
         if (str_starts_with($prefix, '__Secure-') && ! $secure) {
             throw CookieException::forInvalidSecurePrefix();
         }

system/Cookie/Exceptions/CookieException.php

@@ -70,6 +70,26 @@ public static function forInvalidCookieValue()
         return new static(lang('Cookie.invalidCookieValue'));
     }
 
+    /**
+     * Thrown when the cookie path contains invalid characters.
+     *
+     * @return static
+     */
+    public static function forInvalidCookiePath()
+    {
+        return new static(lang('Cookie.invalidCookiePath'));
+    }
+
+    /**
+     * Thrown when the cookie domain contains invalid characters.
+     *
+     * @return static
+     */
+    public static function forInvalidCookieDomain()
+    {
+        return new static(lang('Cookie.invalidCookieDomain'));
+    }
+
     /**
      * Thrown when using the `__Secure-` prefix but the `Secure` attribute
      * is not set to true.

system/Language/en/Cookie.php

@@ -17,6 +17,8 @@
     'invalidExpiresValue'   => 'The cookie expiration time is not valid.',
     'invalidCookieName'     => 'The cookie name "{0}" contains invalid characters.',
     'invalidCookieValue'    => 'The cookie value contains invalid characters.',
+    'invalidCookiePath'     => 'The cookie path contains invalid characters.',
+    'invalidCookieDomain'   => 'The cookie domain 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

@@ -443,4 +443,239 @@ public function testNonRawCookieSafelyEncodesCRLF(): void
         $this->assertStringNotContainsString("\r", $result);
         $this->assertStringNotContainsString("\n", $result);
     }
+
+    #[DataProvider('provideValidationOfCookiePath')]
+    public function testValidationOfCookiePath(string $path): void
+    {
+        $this->expectException(CookieException::class);
+        $this->expectExceptionMessage(lang('Cookie.invalidCookiePath'));
+        new Cookie('test', 'value', ['path' => $path]);
+    }
+
+    #[DataProvider('provideValidationOfCookiePath')]
+    public function testValidationOfCookiePathInWithPath(string $path): void
+    {
+        $this->expectException(CookieException::class);
+        $this->expectExceptionMessage(lang('Cookie.invalidCookiePath'));
+        $cookie = new Cookie('test', 'value');
+        $cookie->withPath($path);
+    }
+
+    /**
+     * @return iterable<string, array{string}>
+     */
+    public static function provideValidationOfCookiePath(): iterable
+    {
+        yield 'comma' => ['/path,comma'];
+
+        yield 'semicolon' => ['/path;semicolon'];
+
+        yield 'space' => ['/path with space'];
+
+        yield 'tab' => ["/path\twith_tab"];
+
+        yield 'carriage return' => ["/path\rcarriage"];
+
+        yield 'newline' => ["/path\nnewline"];
+
+        yield 'vertical tab' => ["/path\vvertical_tab"];
+
+        yield 'form feed' => ["/path\fform_feed"];
+
+        yield 'null byte' => ["/path\0null_byte"];
+
+        yield 'CRLF' => ["/path\r\nwith_crlf"];
+    }
+
+    #[DataProvider('provideFromHeaderStringValidationOfCookiePath')]
+    public function testFromHeaderStringValidationOfCookiePath(string $path): void
+    {
+        $this->expectException(CookieException::class);
+        $this->expectExceptionMessage(lang('Cookie.invalidCookiePath'));
+        Cookie::fromHeaderString("test=value; Path={$path}");
+    }
+
+    /**
+     * @return iterable<string, array{string}>
+     */
+    public static function provideFromHeaderStringValidationOfCookiePath(): iterable
+    {
+        foreach (self::provideValidationOfCookiePath() as $name => $case) {
+            if ($name === 'semicolon') {
+                continue;
+            }
+
+            yield $name => $case;
+        }
+    }
+
+    #[DataProvider('provideValidationOfCookieDomain')]
+    public function testValidationOfCookieDomain(string $domain): void
+    {
+        $this->expectException(CookieException::class);
+        $this->expectExceptionMessage(lang('Cookie.invalidCookieDomain'));
+        new Cookie('test', 'value', ['domain' => $domain]);
+    }
+
+    #[DataProvider('provideValidationOfCookieDomain')]
+    public function testValidationOfCookieDomainInWithDomain(string $domain): void
+    {
+        $this->expectException(CookieException::class);
+        $this->expectExceptionMessage(lang('Cookie.invalidCookieDomain'));
+        $cookie = new Cookie('test', 'value');
+        $cookie->withDomain($domain);
+    }
+
+    /**
+     * @return iterable<string, array{string}>
+     */
+    public static function provideValidationOfCookieDomain(): iterable
+    {
+        yield 'comma' => ['domain,comma.com'];
+
+        yield 'semicolon' => ['domain;semicolon.com'];
+
+        yield 'space' => ['domain with space.com'];
+
+        yield 'tab' => ["domain\twith_tab.com"];
+
+        yield 'carriage return' => ["domain\rcarriage.com"];
+
+        yield 'newline' => ["domain\nnewline.com"];
+
+        yield 'vertical tab' => ["domain\vvertical_tab.com"];
+
+        yield 'form feed' => ["domain\fform_feed.com"];
+
+        yield 'null byte' => ["domain\0null_byte.com"];
+
+        yield 'CRLF' => ["domain\r\nwith_crlf.com"];
+    }
+
+    #[DataProvider('provideFromHeaderStringValidationOfCookieDomain')]
+    public function testFromHeaderStringValidationOfCookieDomain(string $domain): void
+    {
+        $this->expectException(CookieException::class);
+        $this->expectExceptionMessage(lang('Cookie.invalidCookieDomain'));
+        Cookie::fromHeaderString("test=value; Domain={$domain}");
+    }
+
+    /**
+     * @return iterable<string, array{string}>
+     */
+    public static function provideFromHeaderStringValidationOfCookieDomain(): iterable
+    {
+        foreach (self::provideValidationOfCookieDomain() as $name => $case) {
+            if ($name === 'semicolon') {
+                continue;
+            }
+
+            yield $name => $case;
+        }
+    }
+
+    public function testNullPathAndDomainDefaultProperly(): void
+    {
+        $cookie = new Cookie('test', 'val', ['path' => null, 'domain' => null, 'prefix' => null]);
+
+        $this->assertSame('/', $cookie->getPath());
+        $this->assertSame('', $cookie->getDomain());
+        $this->assertSame('', $cookie->getPrefix());
+
+        $cookie2 = $cookie->withPath(null)->withDomain(null)->withPrefix('');
+        $this->assertSame('/', $cookie2->getPath());
+        $this->assertSame('', $cookie2->getDomain());
+        $this->assertSame('', $cookie2->getPrefix());
+    }
+
+    public function testValidCookiePathAndDomain(): void
+    {
+        $cookie = new Cookie('test', 'val', ['path' => '/sub/dir/', 'domain' => 'example.com']);
+        $this->assertSame('/sub/dir/', $cookie->getPath());
+        $this->assertSame('example.com', $cookie->getDomain());
+
+        $cookie2 = $cookie->withPath('/another/path')->withDomain('.example.com');
+        $this->assertSame('/another/path', $cookie2->getPath());
+        $this->assertSame('.example.com', $cookie2->getDomain());
+
+        $cookie3 = new Cookie('test', 'val', ['path' => '/', 'domain' => '']);
+        $this->assertSame('/', $cookie3->getPath());
+        $this->assertSame('', $cookie3->getDomain());
+    }
+
+    #[DataProvider('provideValidationOfCookiePrefix')]
+    public function testValidationOfCookiePrefix(string $prefix): void
+    {
+        $this->expectException(CookieException::class);
+        $this->expectExceptionMessage(lang('Cookie.invalidCookieName', [$prefix]));
+        new Cookie('test', 'val', ['prefix' => $prefix]);
+    }
+
+    #[DataProvider('provideValidationOfCookiePrefix')]
+    public function testValidationOfCookiePrefixInWithPrefix(string $prefix): void
+    {
+        $this->expectException(CookieException::class);
+        $this->expectExceptionMessage(lang('Cookie.invalidCookieName', [$prefix]));
+        $cookie = new Cookie('test', 'val');
+        $cookie->withPrefix($prefix);
+    }
+
+    #[DataProvider('provideValidationOfCookiePrefix')]
+    public function testValidationOfRawCookiePrefix(string $prefix): void
+    {
+        $this->expectException(CookieException::class);
+        $this->expectExceptionMessage(lang('Cookie.invalidCookieName', [$prefix]));
+        new Cookie('test', 'val', ['prefix' => $prefix, 'raw' => true]);
+    }
+
+    #[DataProvider('provideValidationOfCookiePrefix')]
+    public function testValidationOfRawCookiePrefixInWithPrefix(string $prefix): void
+    {
+        $this->expectException(CookieException::class);
+        $this->expectExceptionMessage(lang('Cookie.invalidCookieName', [$prefix]));
+        $cookie = new Cookie('test', 'val', ['raw' => true]);
+        $cookie->withPrefix($prefix);
+    }
+
+    /**
+     * @return iterable<string, array{string}>
+     */
+    public static function provideValidationOfCookiePrefix(): iterable
+    {
+        yield 'equals' => ['prefix='];
+
+        yield 'comma' => ['prefix,'];
+
+        yield 'semicolon' => ['prefix;'];
+
+        yield 'space' => ['prefix '];
+
+        yield 'tab' => ["prefix\t"];
+
+        yield 'carriage return' => ["prefix\r"];
+
+        yield 'newline' => ["prefix\n"];
+
+        yield 'vertical tab' => ["prefix\v"];
+
+        yield 'form feed' => ["prefix\f"];
+
+        yield 'null byte' => ["prefix\0"];
+
+        yield 'CRLF' => ["prefix\r\n"];
+    }
+
+    public function testValidCookiePrefixAllowedSeparators(): void
+    {
+        $cookie = new Cookie('test', 'val', ['prefix' => 'ci:session/']);
+        $this->assertSame('ci:session/', $cookie->getPrefix());
+        $this->assertSame('ci:session/test', $cookie->getPrefixedName());
+
+        $cookie2 = $cookie->withPrefix('my-app:v1/');
+        $this->assertSame('my-app:v1/', $cookie2->getPrefix());
+        $this->assertSame('my-app:v1/test', $cookie2->getPrefixedName());
+
+        $cookie3 = new Cookie('test', 'val', ['prefix' => 'ci:session/', 'raw' => false]);
+        $this->assertSame('ci:session/test=val; Path=/; HttpOnly; SameSite=Lax', $cookie3->toHeaderString());
+    }
 }