'=' prepended to set-cookie header can result in __Secure- and __Host- prefix bypasses
Categories
(Core :: Networking: Cookies, defect, P2)
Tracking
()
People
(Reporter: haxatron1, Assigned: valentin)
References
Details
(Keywords: sec-moderate, Whiteboard: [reporter-external] [client-bounty-form][necko-triaged][post-critsmash-triage] [necko-priority-queue][adv-main105+][adv-esr102.3+])
Attachments
(2 files, 1 obsolete file)
48 bytes,
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta+
RyanVM
:
approval-mozilla-esr102+
|
Details | Review |
333 bytes,
text/plain
|
Details |
- Go to http://httpbin.org
- Execute document.cookie = "= __Host-test=a" in the console
- Go to http://httpbin.org/headers and see Cookie with cookie: 'Cookie "__Host-test=a"'. This shouldn't happen as __Secure- and __Host- should only be set from a secure context
Additional notes: Also reproduces with Set-Cookie header
The pertinent issue is that when document.cookie = "=__Host-test=a" is executed, the name of the cookie is empty, and the value of the cookie is "=__Host-test=a". The unexpected result is that "__Host-test=a" value gets sent instead of "=__Host-test=a".
I recommend rejecting cookies with empty names as a fix for this problem
Updated•2 years ago
|
Comment 3•2 years ago
|
||
(In reply to haxatron1 from comment #0)
- Go to http://httpbin.org
- Execute document.cookie = "= __Host-test=a" in the console
- Go to http://httpbin.org/headers and see Cookie with cookie: 'Cookie "__Host-test=a"'. This shouldn't happen as __Secure- and __Host- should only be set from a secure context
Reproduced.
Just adding here the attack scenario:
"The attack scenario is that a malicious MITM or a compromised subdomain can set these prefixed cookies for a victim site to perform session fixation, effectively bypassing what this prefixes are meant to protect against."
There is also the case of a compromised subdomain being able to set a cookie for the parent domain, bypassing what the __Host- prefix is meant to protect against. In that scenario no MITM is involved.
Updated•2 years ago
|
Comment 6•2 years ago
|
||
This seems wrong even apart from bypassing __Host and __Secure. It's actually valid for a cookie to not have a name (or not have a value). We should not ever turn part of a value into a name.
Comment 7•2 years ago
|
||
Turns out this has been discussed as part of the ongoing cookie spec update (issue #1210), and although no one liked this behavior, there are worries about breaking sites that appear to depend on it. The end result was a recommendation that servers not do this. Obviously, hackers will ignore that recommendation if it's useful to do so.
Faking a prefixed cookie wasn't specifically mentioned, but it's a subset of this known ability. As far as I can tell Chrome also behaves this same way, which makes sense because there are web platform tests that try to help keep implementations in sync and help make sure our specs match reality.
There was a proposal to preserve the leading "=" in the case of a nameless cookie with an embedded "=" (PR #1592). Seems sensible to me, but was rejected in favor of the weaker spec "recommendation" language. An alternate solution is in the proposed Cookie Store API (an experimental feature in Chrome) which says nameless cookies with an embedded '=" should be flat-out rejected.
I quite like the solution in PR #1592, but if we can't have that maybe we could do something specifically about prefixed cookies. We could still do the "start with =" solution if a nameless cookie has an embedded '=' in the value and "looks like" a prefixed cookie, or we could block the setting of those cookies. "looks like a prefixed cookie" could be broadly "starts with __" or more narrowly "starts with either of the defined prefixes."
Yes, Chromium behaves this way, and this is being discussed there as well.
I have cc'ed @morlovich who is one of the Chromium engineers working on the report. Though it seems you have cc'ed @bingler too.
Summary of discussion on solutions on the report:
- Currently the solution @bingler suggested was to reject any cookie values with the prefix:
BWS "__Secure-" *cookie-octet "="
or
BWS "__Host-" *cookie-octet "="
- I have also suggested:
If the cookie pair starts with an = character, and there are at least two = characters in the cookie pair, and there is at least one cookie octet in between the leading = character and the second = character, then remove the leading = character before splitting the cookie pair into cookie name and cookie value.
This is a vulnerability in the RFC6265 spec itself as well, so its likely that the spec will get updated with whatever solution is decided as well.
*Correction for 1
if the cookie name is empty, reject any cookie values with the prefix:
BWS "__Secure-" *cookie-octet "="
or
BWS "__Host-" *cookie-octet "="
Reporter | ||
Comment 10•2 years ago
|
||
Nvm... I also saw this from the https://github.com/httpwg/http-extensions/issues/1210
since Set-Cookie: =foo=1 and Set-Cookie: =bar=2 are the same cookie (the nameless cookie) but with different values, and they overwrite each other, a site could be relying on getting the output Cookie: foo=1 or Cookie: bar=2 (but never both) as a way of (sort of) setting two mutually exclusive cookies named foo and bar.
So it looks like solution 1 is the best way to go about it.
Comment 11•2 years ago
|
||
Could we get a link to the Chromium bug, please? Even if it's inaccessible to us as a security bug the reference is handy to have in our conversations with our Chrome/Edge colleagues, and will be a useful historical cross-reference when both bugs are made public in the future.
This is a vulnerability in the RFC6265 spec itself as well,
The Technically not: rfc6265 §5.4 step 4.1 says
Output the cookie's name, the %x3D ("=") character, and the cookie's value.
For an empty-name cookie that would yield =__Host-foo=fakevalue
. That was changed in the ongoing and still technically unofficial rfc6265-bis spec update in issue #1144 because in practice most browsers didn't do that.
so its likely that the spec will get updated with whatever solution is decided as well.
Absolutely. In fact we'd want to get agreement on a spec change before we fix this in Firefox. We all want browsers to follow the spec and the spec to match reality. Inconsistent behavior leads to sites breaking in one browser or another, or in the worst case leads to a site with a security bug but only for users of some browsers and not others.
Reporter | ||
Comment 12•2 years ago
|
||
The link is https://bugs.chromium.org/p/chromium/issues/detail?id=1345193, as mentioned, it is currently inaccessible without permissions and I there isn't any cc feature available. Would you like me to ask if they could cc you there?
Comment 13•2 years ago
|
||
Thanks for the link. I'll email them and ask; no need to clutter their bug with that request.
If we're only going to protect prefixed cookies then any of the suggestions work, really, whether that's rejecting the cookies or prefixing the returned cookie value with "=". I'm unhappy protecting only prefixed cookies, however: those aren't the only ones with special handling. For example, normally an attacker can't overwrite an HttpOnly cookie from a non-HTTP context (e.g. document.cookie
), and we don't let a non-secure cookie overwrite a secure cookie. This trick doesn't literally overwrite them, but it can create spoofing copies. Depending on the attackers position (plaintext MITM? XSS on a sibling domain?) and how the website is set up the attacker might be able to do that anyway,
The hypothetical usage in comment 10 is cute, but do we have any evidence of sites depending on that too-clever behavior?
Something is better than nothing, though. Although it adds more special-case cruft to the spec, the proposal to block such cookies only if the values start with a known prefix value is surgically targeted at a known case that can't possibly have been an innocent coincidence. Lord knows how many times we've been bitten in the ass by the smallest of seemingly inconsequential cookie changes. Philosophically I like your proposed option 2 better because it's a general solution, but it will change observable behavior in far more cases and that's unpredictable. Realistically Firefox can't take that risk unless the chromium team believes in that solution and is willing the forge the path.
Comment 14•2 years ago
|
||
Since we referenced the original rfc6265 § 5.4 above as a historical note, I should note that the Chrome bug discussion points out that rfc6265 § 5.2 strictly disallows nameless cookies in step 5 of parse a "set-cookie-string"
. Step 3 before that is a little misleading because it mentions that the name and value are "possibly empty" -- but that's just step 3.
The (possibly empty) name string consists of the characters up to, but not including, the first %x3D ("=") character, and the (possibly empty) value string consists of the characters after the first %x3D ("=") character.
Remove any leading or trailing WSP characters from the name string and the value string.
If the name string is empty, ignore the set-cookie-string entirely.
That's just historical interest. Browsers have been pursuing rfc6265-bis for a long time now—including features like the Cookie Prefixes that are being attacked here—and that's what we need to be concerned with.
This isn't going to make next week's release (already testing release candidate builds) so let's target 104. Also affects ESR-91 that I think we support until ~fx105.
Updated•2 years ago
|
Comment 15•2 years ago
|
||
Chrome's patch for this is https://chromium.googlesource.com/chromium/src/+/f9580905b45edb8dfe7da6cd5f26421ab2b5c285
Comment 16•2 years ago
|
||
The bug is marked as tracked for firefox104 (beta). However, the bug still isn't assigned.
:ghess, could you please find an assignee for this tracked bug? If you disagree with the tracking decision, please talk with the release managers.
For more information, please visit auto_nag documentation.
Comment 17•2 years ago
|
||
This is a reminder regarding comment #16!
The bug is marked as tracked for firefox104 (beta). We have limited time to fix this, the soft freeze is in 14 days. However, the bug still isn't assigned.
Comment 18•2 years ago
|
||
This is a reminder regarding comment #16!
The bug is marked as tracked for firefox104 (beta). We have limited time to fix this, the soft freeze is in 13 days. However, the bug still isn't assigned.
Comment 19•2 years ago
|
||
This is a reminder regarding comment #16!
The bug is marked as tracked for firefox104 (beta). We have limited time to fix this, the soft freeze is in 10 days. However, the bug still isn't assigned.
Comment 20•2 years ago
|
||
Clearly not getting fixed in the 104 timeframe.
Comment 21•2 years ago
|
||
In the second chrome bug squarcina posted a nice bit of investigation of how the initial Chrome fix can by bypassed with at least one cloud platform:
I discovered some additional issues: PHP replaces spaces, dots and open square brackets with the underscore symbol in the keys of the $_COOKIE superglobal array; other frameworks perform the URL decode of the cookie name. These server-side mistakes effectively violate the security assumptions that browsers make on cookie prefixes. I reported these issues to the mantainers of the affected projects.
Then, I quickly investigated Amazon API Gateway, in particular AWS Lambda proxy integration. From the linked page:
Format 2.0 includes a new cookies field. All cookie headers in the request are combined with commas and added to the cookies field. In the response to the client, each cookie becomes a set-cookie header.
According to my limited set of tests, this specific parser seems to be rfc6265bis-compliant. The problem with nameless cookies and the rfc serialization algorithm is that sending the header:
Cookie: =__Host-test=foo
as a result of setting a nameless cookie via, e.g., document.cookie = '==__Host-test=foo' (which is accepted by the latest Chrome Canary build), is further serialized by the AWS Lambda proxy integration as:
{ "version": "2.0", "routeKey": "ANY /{proxy+}", "rawPath": "/", "rawQueryString": "", "cookies": [ "__Host-test=foo" ], ...
resulting indistinguishable from a legitimate cookie named '__Host-test' with value 'foo'.
In summary: chaining compliant parsers can introduce issues not mitigated by the current Chrome patch.
Reporter | ||
Comment 22•2 years ago
|
||
Wow. Very interesting research @squarcina!
This comes down to the same problem I elaborated in the other thread. There isn't a commonly defined spec for servers to parse the Cookie header, which can open up a whole can of worms regarding bypassing these cookie prefixes which have occurred time and time again in the past (with percent-encoded issues and what not). =__Host-test=foo is actually the technically correct thing Chrome and Firefox should do in this case, but even with that, some platforms still interpret as __Host-test=foo as pointed out by @squarcina.
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Comment 24•2 years ago
|
||
Assignee | ||
Comment 25•2 years ago
|
||
Comment 26•2 years ago
|
||
Reject cookies with no name and a __Secure- or __Host- prefix r=necko-reviewers,kershaw
https://hg.mozilla.org/mozilla-central/rev/f55ef6188041
Assignee | ||
Comment 27•2 years ago
|
||
Comment on attachment 9293299 [details]
Bug 1779993 - Reject cookies with no name and a __Secure- or __Host- prefix r=#necko
Beta/Release Uplift Approval Request
- User impact if declined: Potential bypass of __Host- and __Secure- cookie prefix restrictions
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: No
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): The change blocks cookies with no name and a value starting with either __Host- or __Secure-
This corner case should be really rare, and shouldn't cause any breakage. - String changes made/needed:
- Is Android affected?: Yes
ESR Uplift Approval Request
- If this is not a sec:{high,crit} bug, please state case for ESR consideration:
- User impact if declined:
- Fix Landed on Version: 106
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky):
Updated•2 years ago
|
Comment 28•2 years ago
•
|
||
Comment on attachment 9293299 [details]
Bug 1779993 - Reject cookies with no name and a __Secure- or __Host- prefix r=#necko
Approved for 105.0b9 and 102.3esr.
Comment 29•2 years ago
|
||
uplift |
Reporter | ||
Comment 30•2 years ago
|
||
I can confirm the fix works on the latest Nightly build; Entering document.cookie = "=__Host-test=a" will now throw the error "Cookie “” has been rejected for invalid prefix." I can also confirm that the Set-Cookie headers have also been fixed.
Comment 31•2 years ago
|
||
uplift |
Updated•2 years ago
|
Updated•2 years ago
|
Comment 32•2 years ago
|
||
Reporter | ||
Comment 33•2 years ago
|
||
Hi, could you credit me as the following instead? -
Axel Chong (@Haxatron)
Updated•2 years ago
|
Reporter | ||
Comment 34•2 years ago
|
||
(In reply to haxatron1 from comment #33)
Hi, could you credit me as the following instead? -
Axel Chong (@Haxatron)
@freddy, jic you didn't see my previous message.
Comment 35•2 years ago
|
||
Done. Thanks for working with us!
Updated•2 years ago
|
Updated•1 year ago
|
Updated•11 months ago
|
Description
•