php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #81727 $_COOKIE names string replacement (. -> _): cookie integrity vulnerabilities
Submitted: 2022-08-12 09:44 UTC Modified: 2022-09-29 18:57 UTC
From: squarcina at gmail dot com Assigned: derick (profile)
Status: Closed Package: HTTP related
PHP Version: Irrelevant OS: Any
Private report: No CVE-ID: 2022-31629
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If this is not your bug, you can add a comment by following this link.
If this is your bug, but you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: squarcina at gmail dot com
New email:
PHP Version: OS:

 

 [2022-08-12 09:44 UTC] squarcina at gmail dot com
Description:
------------
Note:
- Tested with PHP-8.1.5, but I believe all PHP versions are affected
- I had to use `[:]` in all links to avoid being flagged as spam by the bug submission platform

To ensure backward compatibility with `register_globals`, PHP replaces spaces (` `), dots (`.`) and open square brackets (`[`) with underscores (`_`) in the keys of `$_POST` and `$_GET` superglobal arrays. The same string transformation applies to the keys of the $_COOKIE superglobal array.

As a result, a cookie sent by the browser as `Cookie: ..Host-test=foo` is treated by PHP as `Cookie: __Host-test=foo`. Notice that cookies with `__Host-` and `__Secure-` prefixes have a special semantic (see [MDN: Set-Cookie, Cookie Prefixes](https[:]//developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#cookie_prefixes)) which ensures, for instance, that `__Host-`-cookies sent by the browser cannot be overwritten by a network attacker or by a [malicious sibling domain](https[:]//canitakeyoursubdomain.name/).

This vulnerability enables network and same-site attackers to set a standard insecure cookie in the victim's browser which is treated as a `__Host-` or `__Secure-` cookie by PHP applications. The reported issue is related to [CVE-2020-7070](https[:]//bugs.php.net/bug.php?id=79699).

Notice that this issue raises further integrity concerns when legitimate cookies contain the underscore symbol. For instance, non-secure origins can use this bug to "overwrite" secure cookies, violating point 16 of the [Cookies Storage Model (Sec. 5.5)](https[:]//datatracker.ietf.org/doc/html/draft-ietf-httpbis-rfc6265bis-10#section-5.5).


### Vulnerable Example
Consider a website at https[:]//bank.com which adopts the double-submit CSRF protection together with `__Host-`-cookies to ensure high integrity of POST requests, as described in the [Cross-Site Request Forgery Prevention Cheat Sheet](https[:]//cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#cookie-with-__host-prefix). This solution works by matching the value of a special cookie (`__Host-xsrf=<token1>`) against a POST variable (`_xsrf=<token2>`). If the values are equivalent, i.e., `token1 === token2`, then the request is accepted, otherwise it is rejected. Given that the token value is secret and attackers have no way to forge a request with an arbitrary `__Host-xsrf` cookie, the solution is effective against CSRF even in presence of a powerful attacker.

Due to the reported vulnerability, a same-site attacker can lure a victim into visiting a page at http[:]//test.bank.com which:
1. Sets the cookie `..Host-xsrf=eviltoken; domain=bank.com; Path=/app`
2. Executes the HTTP POST request to https[:]//bank.com/app/transfer with parameters:
   - `to=<attacker>`
   - `amount=1000`
   - `_xsrf=eviltoken`
3. The PHP backend at https[:]//bank.com receives the HTTP header `Cookie: ..Host-xsrf=eviltoken; __Host-xsrf=goodtoken` which is stored in the `$_COOKIE` superglobal array as `Array( [__Host-xsrf] => eviltoken )`
4. Given that the values of the `__Host-xsrf` cookie and `_xsrf` parameter match, the request is accepted, bypassing the CSRF protection.

### Proposed Solution
Cookies should be parsed according to the standard [rfc6265bis](https[:]//datatracker.ietf.org/doc/html/draft-ietf-httpbis-rfc6265bis-10) without being affected by additional string transformations.


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2022-08-16 13:30 UTC] cmb@php.net
Indeed, this looks like a real issue.  Thanks for reporting!

> Cookies should be parsed according to the standard rfc6265bis
> without being affected by additional string transformations.

Besides that is just a draft for now, I don't think we can do this
for BC reasons.  While the topic to no longer replace spaces and
dots with underscores already came up in the past (and might be
realized sometime), we almost certainly can't get rid of our
special array handling even in the long run.

To be able to fix the reported issue, and to mostly keep full BC,
it seems to me that we may make special provisions only for
reading cookies whose name would result in __Host- or __Secure-
*after* mangling (but not before); probably it would be okay-ish
to ignore such cookies.
 [2022-08-16 16:50 UTC] squarcina at gmail dot com
Thanks for checking out the report!

> that is just a draft for now

That's absolutely correct, but rfc6265 lacks important security attributes of cookies (e.g., SameSite, __Host-), and rfc6265bis is the closest to what browsers are implementing. That said, also rfc6265bis has inconsistencies that I recently reported https[:]//github.com/httpwg/http-extensions/issues/2229

I think the change you propose is an effective mitigation against injection of cookie prefixes. Basically, if there is a discrepancy before and after mangling, PHP should discard cookies whose names start with either __Host- or __Secure- after mangling.

This fix would still leave PHP vulnerable to the injection of "insecure" cookies trying to spoof the name of Secure ones, but that's less of an issue compared to cookie prefixes.
 [2022-09-07 15:01 UTC] derick@php.net
I have made a patch for this: https://gist.github.com/derickr/00aeb592f57b1440e2888db42200c7ab (secret GIST, so don't share URL). It's fairly naive.

It also rejects these names for GET/POST variables, but I think that consistency is good.
 [2022-09-27 13:08 UTC] derick@php.net
-CVE-ID: +CVE-ID: 2022-31629
 [2022-09-27 13:54 UTC] derick@php.net
-Status: Open +Status: Closed -Assigned To: +Assigned To: derick
 [2022-09-27 13:54 UTC] derick@php.net
Thank you for your bug report. This issue has already been fixed
in the latest released version of PHP, which you can download at
http://www.php.net/downloads.php


 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Apr 18 17:01:28 2024 UTC