diff --git a/system/Cookie/Cookie.php b/system/Cookie/Cookie.php index 27ffc908cb38..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 @@ -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(); } 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..694c9cde6392 100644 --- a/tests/system/Cookie/CookieTest.php +++ b/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 + */ + 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()); + } + + #[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 + */ + 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 f13a3e4b2b49..1b25db3dfa75 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 @@ -42,6 +44,7 @@ Bugs Fixed - **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 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 99b3103e45de..774240c40d8d 100644 --- a/user_guide_src/source/libraries/cookies.rst +++ b/user_guide_src/source/libraries/cookies.rst @@ -104,9 +104,27 @@ 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 =============================== +The cookie prefix must not contain control characters, spaces, tabs, or separator characters +(``= , ; \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 using the ``__Host-`` prefix, cookies must exhibit the following: