php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #79699 PHP parses encoded cookie names so malicious `__Host-` cookies can be sent
Submitted: 2020-06-14 19:37 UTC Modified: 2020-09-29 06:12 UTC
From: fletchto99 at gmail dot com Assigned: stas (profile)
Status: Closed Package: HTTP related
PHP Version: 7.4.7 OS: macOS (but should affect any)
Private report: No CVE-ID: 2020-7070
 [2020-06-14 19:37 UTC] fletchto99 at gmail dot com
Description:
------------
Version: 7.4.7 (likely affects all PHP versions)

The PHP cookie parser parses the HTTP_COOKIE string percent decoding the entire string. This allows a malicious attacker to set a second cookie with the name being percent encoded. Typically it would be expected that we cannot trust cookies and in _most_ cases that's true. However in a couple of cases certain expectations are set. Cookies allow for [cookie prefixes](https://textslashplain.com/2015/10/09/duct-tape-and-baling-wirecookie-prefixes/) on the cookie name to indicate to the browser certain attributes. In this case there are 2 special attributes we care about: `__Secure-` and `__Host-`. When the browser sends these cookies to the server certain [assumptions](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#Attributes) are be made around these cookies:

1. `__Secure-` prefix: Cookies names starting with `__Secure-` (dash is part of the prefix) must be set with the secure flag from a secure page (HTTPS).
2. `__Host-` prefix: Cookies with names starting with `__Host-` must be set with the secure flag, must be from a secure page (HTTPS), must not have a domain specified (and therefore aren't sent to subdomains) and the path must be `/`

The flaw in PHP allows for a `__%48ost-` or `__%53ecure-` cookie to be set **without** the required attributes (I.e. set without HTTPS, from root domain, or from a secure page). This means a malicious cookie set by an attacker could set a `__%48ost-` cookie from a subdomain knowing that Rack would parse it as `__Host-`. Furthermore, since the browser won't enforce the `HostOnly` attribute to `__%48ost-` cookies an attacker could control the `__Host-` prefixed cookie from a subdomain by setting a wildcard domain on the `__%48ost-` cookie. This is breaking the core trust that __Host- cookies _must_ be set from the root domain & secured and also __Secure- cookies must be set from a secure (https) domain.

It should be noted that while the [cookie spec](https://tools.ietf.org/html/rfc6265#section-4.1.1) recommends encoding for the value of a cookie it doesn't make any suggestions around the encoding of the name of a cookie. However the [mozilla documentation](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#Attributes) does indicate how a browser would expect the cookies to behave in these cases:

> A <cookie-name> can be any US-ASCII characters, except control characters, spaces, or tabs. It also must not contain a separator character like the following: ( ) < > @ , ; : \ " / [ ] ? = { }.

Based on the above an attacker could potentially set the cookie from a malicious script on a subdomain like so, bypassing any expectations around the attributes of the cookie:
```
document.cookie = "__%48ost-evil=evil; domain=.example.com";
```

Test script:
---------------
--TEST--
Cookies Security Bug
--INI--
max_input_vars=1000
filter.default=unsafe_raw
--COOKIE--
__%48ost-evil=evil; __Host-evil=good; %66oo=baz;foo=bar
--FILE--
<?php
var_dump($_COOKIE);
?>
--EXPECT--
array(4) {
  ["__%48ost-evil=evil"]=>
  string(4) "evil"
  ["__Host-evil=good"]=>
  string(4) "good"
  ["%66oo"]=>
  string(3) "baz"
  ["foo"]=>
  string(3) "bar"
}


Expected result:
----------------
array(4) {
  ["__%48ost-evil=evil"]=>
  string(4) "evil"
  ["__Host-evil=good"]=>
  string(4) "good"
  ["%66oo"]=>
  string(3) "baz"
  ["foo"]=>
  string(3) "bar"
}


Actual result:
--------------
array(2) {
  ["__Host-evil"]=>
  string(4) "evil"
  ["foo"]=>
  string(3) "baz"
}

Patches

fix-urldecode (last revision 2020-09-21 01:06 UTC by stas@php.net)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2020-06-14 19:40 UTC] fletchto99 at gmail dot com
I should probably mention, PHP isn't the only language/webserver with this issue. I've been working at filing bugs with the appropriate languages during the weekend. Unfortunately I'm not familiar enough with PHP's codebase to submit a patch, sorry about that!
 [2020-06-15 14:03 UTC] pollita@php.net
No including a patch is fine, but what *would* you do to address this?  I hope the answer isn't "Simply don't decode % sequences." as that's non-trivial from a BC point of view.

The more pragmatic approach may involve detecting when "special" cookies result from decoding and validate that all required attributes for that cookie are applied (rejecting the cookie otherwise), but that smells a bit brittle.
 [2020-06-15 17:45 UTC] fletchto99 at gmail dot com
> No[t] including a patch is fine, but what *would* you do to address this?  I hope the answer isn't "Simply don't decode % sequences." as that's non-trivial from a BC point of view.

I would not percent decode the names of cookies. While the spec doesn't indicate that the value should be decoded or not, many implementations do. This would provide the most compatibility while addressing the vulnerability decoding the cookie names introduces.

See more about cookie name/value decoding on the moz docs: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#Attributes
 [2020-06-15 17:50 UTC] stas@php.net
I am not sure what's the supposed attack scenario here. Surely I can just connect to HTTP socket and send any HTTP request with any cookie headers I want. Why would one need to bother with percent-encoding cookies in a roundabout way if I can just directly send HTTP request with the same cookies?
 [2020-06-15 18:04 UTC] fletchto99 at gmail dot com
> I am not sure what's the supposed attack scenario here.

While likely low severity this allows an attacker to break the contract that the browser defines with `HostOnly` and `Secure` cookies. 

For example: Cookies with names starting with `__Host-` must be set with the secure flag, must be from a secure page (HTTPS), must not have a domain specified (and therefore aren't sent to subdomains) and the path must be `/`.

However a malicious script would be able to abuse this bug to set a `__%48ost-` cookie from a sub-domain which PHP may interpret that as a `__Host-` cookie, thus trusting that it was secure and set via the root domain. Essentially breaking the contract that `__Host-` cookies cannot be set from a subdomain. 

Essentially an attacker can overwrite cookies from a location they should not be able to, and this is because the browser only enforces the attributes for `__Host-` and not `__%48ost-` cookies.

The above is also true for `__Secure-` cookies, where they could potentially be set on an un-secure page (non-https).
 [2020-06-15 19:05 UTC] stas@php.net
> While likely low severity this allows an attacker to break the contract that the browser defines with `HostOnly` and `Secure` cookies. 

Not sure what this has to do with browsers. HTTP protocol as far as I know has no concept of "browser" - HTTP request is HTTP request. Of course, browsers exist and have their conventions, but nothing prevents one from composing HTTP requests without use of any browser at all, so I am not sure which "contract" you are talking about - there's absolutely no guarantee that HTTP request contains any particular arrangement of data, beyond HTTP protocol itself.

> For example: Cookies with names starting with `__Host-` must be set with the secure flag, must be from a secure page (HTTPS), must not have a domain specified (and therefore aren't sent to subdomains) and the path must be `/`.

I don't remember such requirement in the HTTP protocol, could you please point to the RFC that defines this? 

If you are talking about browser convention, then this is nice but has little to do with HTTP server and PHP role here is the HTTP server. 

> However a malicious script would be able to abuse this bug to set a `__%48ost-` cookie from a sub-domain which PHP may interpret that as a `__Host-` cookie, thus trusting that it was secure

If your server code thinks that cookie named __Host- is secure just because of the name, this code is trivially broken - I could use any of the thousands of tools to generate HTTP request which has the header with the same name but is not secure. So I am not sure what is the new issue here. 

> The above is also true for `__Secure-` cookies, where they could potentially be set on an un-secure page (non-https).

Again, I could use any tool to generate HTTP request with any `__Secure-` cookie, and there wouldn't even be any "page" - so what? Server code can't rely just on cookie name for anything, because you can create any name you like. 

Am I missing some important point here?
 [2020-06-15 20:16 UTC] fletchto99 at gmail dot com
The RFC draft can be found here: https://tools.ietf.org/html/draft-ietf-httpbis-cookie-prefixes-00

Currently cookie prefixes are supported by all browsers excluding IE, see "Cookie Prefixes" in the compatibility table here: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#Browser_compatibility

> Am I missing some important point here?

The attack isn't so much that _I_ can modify my requests but rather that if there was an XSS vulnerability on a subdomain the attacker could make use of that to change the value of the `__Host-` cookie on the root domain, when typically that cookie shouldn't even be accessible from said subdomain:

```
document.cookie = "__%48ost-evil=evil; domain=.example.com";
```

This would allow the attacker to set a cookie with the `__%48ost-` prefix which PHP would interpret at `__Host-`. The whole point of that prefix is so that a subdomain _can't_ set that cookie but because of the non-standard decoding this protection can be bypassed.

This article has a pretty good explanation why cookie prefixes are valuable: https://www.sjoerdlangkemper.nl/2017/02/09/cookie-prefixes/
 [2020-06-15 20:18 UTC] fletchto99 at gmail dot com
To summarize it bypasses protection mechanisms of `__Host-` prefixed cookies which help in preventing these types of attacks:

- The attacker can write a known value to the session ID. The client logs in and the attacker now has a working session. This works if the application is vulnerable to session fixation.
- The attacker overwrites the session ID after log in. Alice thinks she is logged in as Alice, but she is actually logged in as the attacker.
- The application checks the CSRF token in forms against a cookie. By overwriting that cookie, the attacker can perform CSRF requests.
 [2020-06-15 20:33 UTC] stas@php.net
> The RFC draft can be found here

This RFC, besides being an expired draft as it seems, seems to describe browser behavior, not server behavior. So I am not sure why it is relevant to HTTP server case. The server takes no part in decision of which cookies to accept and which to reject. 

> The attacker can write a known value to the session ID

Surely, this is session fixation attack, but these are well known and have nothing to do with special cookie names. I am still not sure how prefixing cookie name with __Host- changes anything here.

> This would allow the attacker to set a cookie with the `__%48ost-` prefix which PHP would interpret at `__Host-`.

But why the attacker wouldn't just set cookie with name `__Host-` in the first place? Do you imply the browser would prevent it from being set? Then it should also prevent the `__%48ost-` from being set by the same considerations, or encode the cookie name properly. That seems to be entirely on the browser side, I am still not sure what the server has to do with it.
 [2020-06-15 20:46 UTC] fletchto99 at gmail dot com
> But why the attacker wouldn't just set cookie with name `__Host-` in the first place? Do you imply the browser would prevent it from being set?

Yep exactly. The browser wouldn't allow you to set the `__Host-` cookie from a subdomain. However the browser knows nothing about`__%48ost-` cookies so it will not enforce anything regarding that cookie.

For example, let's say an attacker set `__%48ost-session` via XSS on sub.example.com then the user went to login from example.com, they would have the `__%48ost-session` cookie sent to PHP which would be interpreted as `__Host-session`. If the server relied on that cookie and trusts that it had the `__Host-` prefix then it can falsely assume the cookie has the `HostOnly` attribute. However because PHP treats `__%48ost-session` and `__Host-session` as the same it gives the attacker the ability to change that cookie from a subdomain potentially enabling one of the attacks listed earlier.

A few other web frameworks/languages were vulnerable to this attack and are in the process of publishing CVEs & fixes. Once they're published I can forward the links along and perhaps that can offer some more clarity?
 [2020-06-15 22:37 UTC] fletchto99 at gmail dot com
The rails / rack team has published their advisory as well as a fix today.

Advisory: https://groups.google.com/g/rubyonrails-security/c/OWtmozPH9Ak/m/4m00yHPCBAAJ?pli=1

Patch: https://github.com/rack/rack/commit/1f5763de6a9fe515ff84992b343d63c88104654c
 [2020-06-25 20:30 UTC] fletchto99 at gmail dot com
Hello,

I just wanted to follow up on this submission to see if there's been a consensus regarding the issue? Thanks!
 [2020-08-04 21:20 UTC] fletchto99 at gmail dot com
Hello,

I figured I'd send one more follow up...

Dotnet is also fixing this vulnerability in their 5.0 release: https://github.com/dotnet/aspnetcore/pull/24264

If the PHP team doesn't believe this is a security issue would it be possible to open this issue as a bug to receive further community feedback? 

Kind Regards
 [2020-08-04 21:23 UTC] stas@php.net
-Assigned To: +Assigned To: stas
 [2020-08-04 21:23 UTC] stas@php.net
I am still not 100% convinced it's our responsibility to fix browser not encoding things correctly, but I guess if other server tools standardize on not decoding cookie names it's ok for us to do that too. I'll make a patch for it, but probably not in time for release this Thursday, so it would be in the next one.
 [2020-09-21 01:06 UTC] stas@php.net
The following patch has been added/updated:

Patch Name: fix-urldecode
Revision:   1600650364
URL:        https://bugs.php.net/patch-display.php?bug=79699&patch=fix-urldecode&revision=1600650364
 [2020-09-21 01:13 UTC] stas@php.net
I've added the patch but I am not sure whether I should merge it as it fails some tests. This is easy to fix but I wonder if it can't lead to more breakage. I'll raise a question on discussion lists and see what people say.
 [2020-09-27 06:17 UTC] stas@php.net
-CVE-ID: +CVE-ID: 2020-7070
 [2020-09-29 06:12 UTC] stas@php.net
-Status: Assigned +Status: Closed
 [2020-09-29 06:12 UTC] stas@php.net
The fix for this bug has been committed.
If you are still experiencing this bug, try to check out latest source from https://github.com/php/php-src and re-test.
Thank you for the report, and for helping us make PHP better.


 
PHP Copyright © 2001-2020 The PHP Group
All rights reserved.
Last updated: Fri Dec 04 08:01:23 2020 UTC