From 1539867e38a20d0267499660b6e4438a8fea414c Mon Sep 17 00:00:00 2001 From: Bogdan Date: Sat, 5 Sep 2026 22:45:27 +0200 Subject: [PATCH 1/3] fix(Cookie): validate cookie path and domain attributes --- system/Cookie/Cookie.php | 49 ++++- system/Cookie/Exceptions/CookieException.php | 20 +++ system/Language/en/Cookie.php | 2 + tests/system/Cookie/CookieTest.php | 179 +++++++++++++++++++ user_guide_src/source/changelogs/v4.7.5.rst | 4 +- user_guide_src/source/libraries/cookies.rst | 14 ++ 6 files changed, 261 insertions(+), 7 deletions(-) diff --git a/system/Cookie/Cookie.php b/system/Cookie/Cookie.php index 27ffc908cb38..e715d0e0f99f 100644 --- a/system/Cookie/Cookie.php +++ b/system/Cookie/Cookie.php @@ -232,11 +232,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 +258,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']; @@ -270,7 +270,12 @@ final public function __construct(string $name, string $value = '', array $optio $httponly = $options['httponly']; $this->validateName($name, $raw); + if ($prefix !== '') { + $this->validateName($prefix, $raw); + } $this->validateValue($value, $raw); + $this->validatePath($path); + $this->validateDomain($domain); $this->validatePrefix($prefix, $secure, $path, $domain); $this->validateSameSite($samesite, $secure); @@ -449,6 +454,9 @@ public function getOptions(): array public function withPrefix(string $prefix = '') { $this->validatePrefix($prefix, $this->secure, $this->path, $this->domain); + if ($prefix !== '') { + $this->validateName($prefix, $this->raw); + } $cookie = clone $this; @@ -515,6 +523,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 +539,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; @@ -586,6 +596,9 @@ public function withSameSite(string $samesite) public function withRaw(bool $raw = true) { $this->validateName($this->name, $raw); + if ($this->prefix !== '') { + $this->validateName($this->prefix, $raw); + } $this->validateValue($this->value, $raw); $cookie = clone $this; @@ -790,6 +803,30 @@ protected function validateValue(string $value, bool $raw): void } } + /** + * Validates the cookie path per RFC 6265 and 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 RFC 6265 and 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. * diff --git a/system/Cookie/Exceptions/CookieException.php b/system/Cookie/Exceptions/CookieException.php index 3cc7e32e6eff..b05aa579e61b 100644 --- a/system/Cookie/Exceptions/CookieException.php +++ b/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. diff --git a/system/Language/en/Cookie.php b/system/Language/en/Cookie.php index 4f4294ff0fc5..7fde16cd8434 100644 --- a/system/Language/en/Cookie.php +++ b/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 "/".', diff --git a/tests/system/Cookie/CookieTest.php b/tests/system/Cookie/CookieTest.php index 07991be006f6..28035f8bbb71 100644 --- a/tests/system/Cookie/CookieTest.php +++ b/tests/system/Cookie/CookieTest.php @@ -443,4 +443,183 @@ 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 + */ + 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 + */ + 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 + */ + 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 + */ + 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()); + } + + public function testValidationOfRawCookiePrefix(): void + { + $this->expectException(CookieException::class); + new Cookie('test', 'val', ['prefix' => "bad\r\n", 'raw' => true]); + } + + public function testValidationOfRawCookiePrefixInWithPrefix(): void + { + $this->expectException(CookieException::class); + $cookie = new Cookie('test', 'val', ['raw' => true]); + $cookie->withPrefix("bad\r\n"); + } + + public function testValidationOfRawCookiePrefixInWithRaw(): void + { + $this->expectException(CookieException::class); + $cookie = new Cookie('test', 'val', ['prefix' => "bad\r\n", 'raw' => false]); + $cookie->withRaw(true); + } } diff --git a/user_guide_src/source/changelogs/v4.7.5.rst b/user_guide_src/source/changelogs/v4.7.5.rst index f13a3e4b2b49..90e3ea533d22 100644 --- a/user_guide_src/source/changelogs/v4.7.5.rst +++ b/user_guide_src/source/changelogs/v4.7.5.rst @@ -19,6 +19,8 @@ Message Changes *************** - Added the ``Cookie.invalidCookieValue`` language string. +- Added the ``Cookie.invalidCookiePath`` language string. +- Added the ``Cookie.invalidCookieDomain`` language string. ******* Changes @@ -41,7 +43,7 @@ Bugs Fixed - **CLIRequest:** Fixed a bug where ``parseCommand()`` could throw a TypeError when ``argv`` is missing. - **CodeIgniter:** Fixed a bug where ``gatherOutput()`` could be called twice when ``startController()`` returned a ``ResponseInterface`` (e.g., from filter attributes or closure routes). - **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()``. +- **Cookie:** Fixed a bug where ``Cookie`` instances allowed invalid characters in cookie values (with ``raw: true``), path, and domain attributes rejected by ``setrawcookie()`` and RFC 6265. - **Database:** Fixed a bug where rebuilding a SQLite3 table (e.g., ``Forge::dropColumn()``, ``Forge::modifyColumn()``, ``Forge::dropForeignKey()`` and ``Forge::dropPrimaryKey()``) corrupted the table names referenced by its foreign keys when ``DBPrefix`` was set. - **Files:** Fixed a bug where ``File::move()`` and ``UploadedFile::move()`` set executable and overly permissive file permissions (``0777 & ~umask()`` instead of ``0666 & ~umask()``), and ``UploadedFile::move()`` targeted the parent directory instead of the destination file for ``chmod()``. - **Helpers:** Fixed a bug where ``get_dir_file_info()`` returned incomplete entries for subdirectories and missing files instead of omitting them. diff --git a/user_guide_src/source/libraries/cookies.rst b/user_guide_src/source/libraries/cookies.rst index 99b3103e45de..b0913c2e1289 100644 --- a/user_guide_src/source/libraries/cookies.rst +++ b/user_guide_src/source/libraries/cookies.rst @@ -104,6 +104,20 @@ If setting the ``$raw`` parameter to ``true``, the cookie value will also be val It must not contain control characters, spaces, tabs, or separator characters (``, ;``) as `setrawcookie() `_ will reject them. +Validating the Path Attribute +============================= + +The cookie path must not contain control characters, spaces, tabs, or separator characters +(``, ;``) as `setcookie() `_ and +`setrawcookie() `_ will reject them. + +Validating the Domain Attribute +=============================== + +The cookie domain must not contain control characters, spaces, tabs, or separator characters +(``, ;``) as `setcookie() `_ and +`setrawcookie() `_ will reject them. + Validating the Prefix Attribute =============================== From 6a650d2471248f4af5fa3f1a532563070b2ee642 Mon Sep 17 00:00:00 2001 From: Bogdan Date: Sun, 6 Sep 2026 21:35:47 +0200 Subject: [PATCH 2/3] fix(Cookie): validate cookie prefix independently of raw mode --- system/Cookie/Cookie.php | 6 +++--- tests/system/Cookie/CookieTest.php | 24 +++++++++++++++------ user_guide_src/source/changelogs/v4.7.5.rst | 2 +- user_guide_src/source/libraries/cookies.rst | 4 ++++ 4 files changed, 25 insertions(+), 11 deletions(-) diff --git a/system/Cookie/Cookie.php b/system/Cookie/Cookie.php index e715d0e0f99f..5f7935bc93a7 100644 --- a/system/Cookie/Cookie.php +++ b/system/Cookie/Cookie.php @@ -271,7 +271,7 @@ final public function __construct(string $name, string $value = '', array $optio $this->validateName($name, $raw); if ($prefix !== '') { - $this->validateName($prefix, $raw); + $this->validateName($prefix, true); } $this->validateValue($value, $raw); $this->validatePath($path); @@ -455,7 +455,7 @@ public function withPrefix(string $prefix = '') { $this->validatePrefix($prefix, $this->secure, $this->path, $this->domain); if ($prefix !== '') { - $this->validateName($prefix, $this->raw); + $this->validateName($prefix, true); } $cookie = clone $this; @@ -597,7 +597,7 @@ public function withRaw(bool $raw = true) { $this->validateName($this->name, $raw); if ($this->prefix !== '') { - $this->validateName($this->prefix, $raw); + $this->validateName($this->prefix, true); } $this->validateValue($this->value, $raw); diff --git a/tests/system/Cookie/CookieTest.php b/tests/system/Cookie/CookieTest.php index 28035f8bbb71..43cd40a3c34e 100644 --- a/tests/system/Cookie/CookieTest.php +++ b/tests/system/Cookie/CookieTest.php @@ -603,23 +603,33 @@ public function testValidCookiePathAndDomain(): void $this->assertSame('', $cookie3->getDomain()); } - public function testValidationOfRawCookiePrefix(): void + public function testValidationOfCookiePrefix(): void { $this->expectException(CookieException::class); - new Cookie('test', 'val', ['prefix' => "bad\r\n", 'raw' => true]); + $this->expectExceptionMessage(lang('Cookie.invalidCookieName', ["bad\r\n"])); + new Cookie('test', 'val', ['prefix' => "bad\r\n"]); } - public function testValidationOfRawCookiePrefixInWithPrefix(): void + public function testValidationOfCookiePrefixInWithPrefix(): void { $this->expectException(CookieException::class); - $cookie = new Cookie('test', 'val', ['raw' => true]); + $this->expectExceptionMessage(lang('Cookie.invalidCookieName', ["bad\r\n"])); + $cookie = new Cookie('test', 'val'); $cookie->withPrefix("bad\r\n"); } - public function testValidationOfRawCookiePrefixInWithRaw(): void + public function testValidationOfRawCookiePrefix(): void { $this->expectException(CookieException::class); - $cookie = new Cookie('test', 'val', ['prefix' => "bad\r\n", 'raw' => false]); - $cookie->withRaw(true); + $this->expectExceptionMessage(lang('Cookie.invalidCookieName', ["bad\r\n"])); + new Cookie('test', 'val', ['prefix' => "bad\r\n", 'raw' => true]); + } + + public function testValidationOfRawCookiePrefixInWithPrefix(): void + { + $this->expectException(CookieException::class); + $this->expectExceptionMessage(lang('Cookie.invalidCookieName', ["bad\r\n"])); + $cookie = new Cookie('test', 'val', ['raw' => true]); + $cookie->withPrefix("bad\r\n"); } } diff --git a/user_guide_src/source/changelogs/v4.7.5.rst b/user_guide_src/source/changelogs/v4.7.5.rst index 90e3ea533d22..81b435eea307 100644 --- a/user_guide_src/source/changelogs/v4.7.5.rst +++ b/user_guide_src/source/changelogs/v4.7.5.rst @@ -43,7 +43,7 @@ Bugs Fixed - **CLIRequest:** Fixed a bug where ``parseCommand()`` could throw a TypeError when ``argv`` is missing. - **CodeIgniter:** Fixed a bug where ``gatherOutput()`` could be called twice when ``startController()`` returned a ``ResponseInterface`` (e.g., from filter attributes or closure routes). - **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 allowed invalid characters in cookie values (with ``raw: true``), path, and domain attributes rejected by ``setrawcookie()`` and RFC 6265. +- **Cookie:** Fixed a bug where ``Cookie`` instances allowed invalid characters in cookie values (with ``raw: true``), path, domain, and prefix attributes rejected by ``setcookie()``, ``setrawcookie()``, and RFC 6265. - **Database:** Fixed a bug where rebuilding a SQLite3 table (e.g., ``Forge::dropColumn()``, ``Forge::modifyColumn()``, ``Forge::dropForeignKey()`` and ``Forge::dropPrimaryKey()``) corrupted the table names referenced by its foreign keys when ``DBPrefix`` was set. - **Files:** Fixed a bug where ``File::move()`` and ``UploadedFile::move()`` set executable and overly permissive file permissions (``0777 & ~umask()`` instead of ``0666 & ~umask()``), and ``UploadedFile::move()`` targeted the parent directory instead of the destination file for ``chmod()``. - **Helpers:** Fixed a bug where ``get_dir_file_info()`` returned incomplete entries for subdirectories and missing files instead of omitting them. diff --git a/user_guide_src/source/libraries/cookies.rst b/user_guide_src/source/libraries/cookies.rst index b0913c2e1289..7e0fe08394c0 100644 --- a/user_guide_src/source/libraries/cookies.rst +++ b/user_guide_src/source/libraries/cookies.rst @@ -121,6 +121,10 @@ The cookie domain must not contain control characters, spaces, tabs, or separato Validating the Prefix Attribute =============================== +The cookie prefix must not contain control characters, spaces, tabs, or separator characters +(``= , ; \t \r \n \v \f ( ) < > @ : \" / [ ] ? { }``) as `setcookie()` and +`setrawcookie() `_ will reject them. + When using the ``__Secure-`` prefix, cookies must be set with the ``$secure`` flag set to ``true``. If using the ``__Host-`` prefix, cookies must exhibit the following: From ba58d55a9118e47005ca64a0b2e0613979b496d0 Mon Sep 17 00:00:00 2001 From: Bogdan Date: Mon, 7 Sep 2026 20:37:33 +0200 Subject: [PATCH 3/3] fix(Cookie): refine prefix validation and update documentation - Prohibit only PHP-rejected characters and NUL in cookie prefixes to preserve compatibility with separators like ':' and '/' - Update cookie prefix validation documentation in User Guide - Separate Cookie raw value fix and path/domain/prefix validation entries in changelog and remove RFC 6265 reference - Add test coverage for PHP-prohibited prefix characters and allowed separators --- system/Cookie/Cookie.php | 29 +++++---- tests/system/Cookie/CookieTest.php | 70 +++++++++++++++++---- user_guide_src/source/changelogs/v4.7.5.rst | 3 +- user_guide_src/source/libraries/cookies.rst | 2 +- 4 files changed, 77 insertions(+), 27 deletions(-) diff --git a/system/Cookie/Cookie.php b/system/Cookie/Cookie.php index 5f7935bc93a7..da95f18805d4 100644 --- a/system/Cookie/Cookie.php +++ b/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 @@ -270,9 +277,6 @@ final public function __construct(string $name, string $value = '', array $optio $httponly = $options['httponly']; $this->validateName($name, $raw); - if ($prefix !== '') { - $this->validateName($prefix, true); - } $this->validateValue($value, $raw); $this->validatePath($path); $this->validateDomain($domain); @@ -454,9 +458,6 @@ public function getOptions(): array public function withPrefix(string $prefix = '') { $this->validatePrefix($prefix, $this->secure, $this->path, $this->domain); - if ($prefix !== '') { - $this->validateName($prefix, true); - } $cookie = clone $this; @@ -596,9 +597,6 @@ public function withSameSite(string $samesite) public function withRaw(bool $raw = true) { $this->validateName($this->name, $raw); - if ($this->prefix !== '') { - $this->validateName($this->prefix, true); - } $this->validateValue($this->value, $raw); $cookie = clone $this; @@ -804,7 +802,7 @@ protected function validateValue(string $value, bool $raw): void } /** - * Validates the cookie path per RFC 6265 and PHP setcookie() constraints. + * Validates the cookie path per PHP setcookie() constraints. * * @throws CookieException */ @@ -816,7 +814,7 @@ protected function validatePath(string $path): void } /** - * Validates the cookie domain per RFC 6265 and PHP setcookie() constraints. + * Validates the cookie domain per PHP setcookie() constraints. * * @throws CookieException */ @@ -828,12 +826,17 @@ protected function validateDomain(string $domain): void } /** - * Validates the special prefixes if some attribute requirements are met. + * 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(); } diff --git a/tests/system/Cookie/CookieTest.php b/tests/system/Cookie/CookieTest.php index 43cd40a3c34e..694c9cde6392 100644 --- a/tests/system/Cookie/CookieTest.php +++ b/tests/system/Cookie/CookieTest.php @@ -603,33 +603,79 @@ public function testValidCookiePathAndDomain(): void $this->assertSame('', $cookie3->getDomain()); } - public function testValidationOfCookiePrefix(): void + #[DataProvider('provideValidationOfCookiePrefix')] + public function testValidationOfCookiePrefix(string $prefix): void { $this->expectException(CookieException::class); - $this->expectExceptionMessage(lang('Cookie.invalidCookieName', ["bad\r\n"])); - new Cookie('test', 'val', ['prefix' => "bad\r\n"]); + $this->expectExceptionMessage(lang('Cookie.invalidCookieName', [$prefix])); + new Cookie('test', 'val', ['prefix' => $prefix]); } - public function testValidationOfCookiePrefixInWithPrefix(): void + #[DataProvider('provideValidationOfCookiePrefix')] + public function testValidationOfCookiePrefixInWithPrefix(string $prefix): void { $this->expectException(CookieException::class); - $this->expectExceptionMessage(lang('Cookie.invalidCookieName', ["bad\r\n"])); + $this->expectExceptionMessage(lang('Cookie.invalidCookieName', [$prefix])); $cookie = new Cookie('test', 'val'); - $cookie->withPrefix("bad\r\n"); + $cookie->withPrefix($prefix); } - public function testValidationOfRawCookiePrefix(): void + #[DataProvider('provideValidationOfCookiePrefix')] + public function testValidationOfRawCookiePrefix(string $prefix): void { $this->expectException(CookieException::class); - $this->expectExceptionMessage(lang('Cookie.invalidCookieName', ["bad\r\n"])); - new Cookie('test', 'val', ['prefix' => "bad\r\n", 'raw' => true]); + $this->expectExceptionMessage(lang('Cookie.invalidCookieName', [$prefix])); + new Cookie('test', 'val', ['prefix' => $prefix, 'raw' => true]); } - public function testValidationOfRawCookiePrefixInWithPrefix(): void + #[DataProvider('provideValidationOfCookiePrefix')] + public function testValidationOfRawCookiePrefixInWithPrefix(string $prefix): void { $this->expectException(CookieException::class); - $this->expectExceptionMessage(lang('Cookie.invalidCookieName', ["bad\r\n"])); + $this->expectExceptionMessage(lang('Cookie.invalidCookieName', [$prefix])); $cookie = new Cookie('test', 'val', ['raw' => true]); - $cookie->withPrefix("bad\r\n"); + $cookie->withPrefix($prefix); + } + + /** + * @return iterable + */ + 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()); } } diff --git a/user_guide_src/source/changelogs/v4.7.5.rst b/user_guide_src/source/changelogs/v4.7.5.rst index 81b435eea307..1b25db3dfa75 100644 --- a/user_guide_src/source/changelogs/v4.7.5.rst +++ b/user_guide_src/source/changelogs/v4.7.5.rst @@ -43,7 +43,8 @@ Bugs Fixed - **CLIRequest:** Fixed a bug where ``parseCommand()`` could throw a TypeError when ``argv`` is missing. - **CodeIgniter:** Fixed a bug where ``gatherOutput()`` could be called twice when ``startController()`` returned a ``ResponseInterface`` (e.g., from filter attributes or closure routes). - **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 allowed invalid characters in cookie values (with ``raw: true``), path, domain, and prefix attributes rejected by ``setcookie()``, ``setrawcookie()``, and RFC 6265. +- **Cookie:** Fixed a bug where ``Cookie`` instances created with ``raw: true`` allowed invalid characters in cookie values rejected by ``setrawcookie()``. +- **Cookie:** Fixed a bug where ``Cookie`` instances allowed invalid characters in path, domain, and prefix attributes rejected by ``setcookie()`` and ``setrawcookie()``. - **Database:** Fixed a bug where rebuilding a SQLite3 table (e.g., ``Forge::dropColumn()``, ``Forge::modifyColumn()``, ``Forge::dropForeignKey()`` and ``Forge::dropPrimaryKey()``) corrupted the table names referenced by its foreign keys when ``DBPrefix`` was set. - **Files:** Fixed a bug where ``File::move()`` and ``UploadedFile::move()`` set executable and overly permissive file permissions (``0777 & ~umask()`` instead of ``0666 & ~umask()``), and ``UploadedFile::move()`` targeted the parent directory instead of the destination file for ``chmod()``. - **Helpers:** Fixed a bug where ``get_dir_file_info()`` returned incomplete entries for subdirectories and missing files instead of omitting them. diff --git a/user_guide_src/source/libraries/cookies.rst b/user_guide_src/source/libraries/cookies.rst index 7e0fe08394c0..774240c40d8d 100644 --- a/user_guide_src/source/libraries/cookies.rst +++ b/user_guide_src/source/libraries/cookies.rst @@ -122,7 +122,7 @@ Validating the Prefix Attribute =============================== The cookie prefix must not contain control characters, spaces, tabs, or separator characters -(``= , ; \t \r \n \v \f ( ) < > @ : \" / [ ] ? { }``) as `setcookie()` and +(``= , ; \t \r \n \v \f \0``) as `setcookie() `_ and `setrawcookie() `_ will reject them. When using the ``__Secure-`` prefix, cookies must be set with the ``$secure`` flag set to ``true``. If