CORS Authorization Mishandling
Categories
(Core :: DOM: Networking, defect, P2)
Tracking
()
People
(Reporter: steven.hartz, Assigned: kershaw)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-complete, sec-moderate, Whiteboard: [necko-triaged][secdom:spec], [wptsync upstream][adv-main115+])
Attachments
(6 files)
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:83.0) Gecko/20100101 Firefox/83.0
Steps to reproduce:
- Extract echo.py and test.html from the tarball
- Run echo.py, which will start a simple echo server on http://localhost:8081
- Open test.html in a new tab
- Open the developer tools Network tab
- Click the "Send Request" button
- Observe that the XHR is sent with the Authorization header in the Network tab
- Switch to the Console tab. Observe that JavaScript can access the application's response and log it in the Console.
HTTP Traffic:
CORS Pre-flight Request:
OPTIONS / HTTP/1.1
Host: localhost:8081
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:83.0) Gecko/20100101 Firefox/83.0
Accept: /
Accept-Language: en-US,en;q=0.5
Accept-Encoding: gzip, deflate
Access-Control-Request-Method: POST
Access-Control-Request-Headers: authorization,content-type
Origin: null
Connection: close
CORS Pre-flight Response:
HTTP/1.0 200 OK
Server: BaseHTTP/0.6 Python/3.8.5
Date: Mon, 18 Jan 2021 19:17:09 GMT
Content-type: text/html
Access-Control-Allow-Origin: *
Access-Control-Allow-Headers: *
=========
CORS Request:
POST / HTTP/1.1
Host: localhost:8081
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:83.0) Gecko/20100101 Firefox/83.0
Accept: /
Accept-Language: en-US,en;q=0.5
Accept-Encoding: gzip, deflate
Content-Type: application/json
Authorization: eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c
Content-Length: 31
Origin: null
Connection: close
{
"some_json": "contents"
}
CORS Response:
HTTP/1.0 200 OK
Server: BaseHTTP/0.6 Python/3.8.5
Date: Mon, 18 Jan 2021 19:17:09 GMT
Content-type: text/html
Access-Control-Allow-Origin: *
Access-Control-Allow-Headers: *
POST request for /
Actual results:
The Authorization header with a JSON Web Token (JWT) is sent on an XmlHttpRequest (XHR) without Access-Control-Allow-Credentials, without xhr.withCredentials=true, and without Authorization explicitly listed in Access-Control-Allow-Headers.
Expected results:
Authorization should not be sent when ACAH is wildcard (*):
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Headers#directives
Credentialed request should not be sent when ACAO is wildcard (*)
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin#directives
Credentialed requests should not be sent without Access-Control-Allow-Credentials, nor should be sent when the XHR object's withCredentials property has not been set to true:
Updated•4 years ago
|
Comment 1•4 years ago
|
||
This was implemented in bug 1309358 but looking at the patches there I don't see the behavior that is warranted for the Authorization header. Unfortunately there is no test coverage for that either that I can see.
Kershaw, can you take a look?
Comment 2•4 years ago
|
||
I believe this is sec-moderate, looking at our severity ratings page, but open to be convinced otherwise.
Assignee | ||
Comment 3•4 years ago
|
||
So, I take a look at our implementation of CORS check
. The only thing we missed is about Access-Control-Allow-Headers
. The problem is at this line, which allow all headers when the header value of Access-Control-Allow-Headers
is *
. We should add a check to Authorization
header.
About Access-Control-Allow-Origin
and Access-Control-Allow-Credentials
, I think our [implementation]((https://searchfox.org/mozilla-central/rev/4dac9993b609fccc87e82682614faf2a44cda306/netwerk/protocol/http/nsCORSListenerProxy.cpp#567-601) is correct. When Access-Control-Allow-Origin
is *
, credentialed request are not allowed.
Assignee | ||
Comment 4•4 years ago
|
||
Hi Anne,
I have a question about Access-Control-Expose-Headers
. It seems the spec doesn't mention that Authorization
header can't be wildcarded, but mdn does.
I think mdn is not correct, right? Or do I interpret the spec wrongly?
Thanks.
Comment 5•4 years ago
•
|
||
Yeah, -Expose-Headers
does not handle it and I don't think it should as Authorization
is a request header. I'll add dev-doc-needed
so that can be corrected. It's probably worth adding a test for though.
Assignee | ||
Comment 6•4 years ago
|
||
Comment 7•4 years ago
|
||
Authorization header can't be wildcarded for Access-Control-Allow-Headers r=necko-reviewers,dragana
https://hg.mozilla.org/integration/autoland/rev/4ffa919d35f99eb2120e255da5a6d35b7672e26b
Comment 8•4 years ago
|
||
Comment 9•4 years ago
|
||
Is this something we need to consider for backport?
Assignee | ||
Comment 10•4 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM] from comment #9)
Is this something we need to consider for backport?
I'm inclined not to uplift this for the following reasons.
- This is not a sec-high bug. However, I am not sure what vulnerability could be exploited by sending 'authorization' header to the server.
@freddy, what do you think? - This patch could break some websites, like bug 1691824. Not sure if this is a good idea to uplift to ESR 78.
Comment 11•4 years ago
|
||
Steven, did you also file this bug against Chrome and Safari? Could you copy me on those bugs?
Comment 12•4 years ago
|
||
(In reply to Kershaw Chang [:kershaw] from comment #10)
(In reply to Ryan VanderMeulen [:RyanVM] from comment #9)
Is this something we need to consider for backport?
I'm inclined not to uplift this for the following reasons.
- This is not a sec-high bug. However, I am not sure what vulnerability could be exploited by sending 'authorization' header to the server.
@freddy, what do you think?- This patch could break some websites, like bug 1691824. Not sure if this is a good idea to uplift to ESR 78.
+1 for uplifting. Especially if this affects other browsers (which is a bit unclear to me. looking at comment 11).
Does it apply cleanly to ESR 78? If so, please see if you can uplift.
Comment 13•4 years ago
|
||
I don't think we should uplift this until we know more about the situation in other browsers. It seems like they don't enforce this either so uplifting would be best avoided in that case I think and we might even have to back this out if they don't plan on enforcing this around the same time. (I'm trying to find out.)
Comment 14•4 years ago
|
||
Thanks.
Updated•4 years ago
|
Comment 15•4 years ago
|
||
I created https://github.com/whatwg/fetch/security/advisories/GHSA-6rrj-jgxg-wf55 so we can discuss this with other browsers, who indeed do not seem to implement this. Contact me with your GitHub ID if you want to be added.
Chrome folks informed me there did not seem to be a bug reported against them so with all the information I have now I would recommend that we do not ship fix until we have more information.
Assignee | ||
Comment 16•4 years ago
|
||
(In reply to Anne (:annevk) from comment #15)
I created https://github.com/whatwg/fetch/security/advisories/GHSA-6rrj-jgxg-wf55 so we can discuss this with other browsers, who indeed do not seem to implement this. Contact me with your GitHub ID if you want to be added.
Chrome folks informed me there did not seem to be a bug reported against them so with all the information I have now I would recommend that we do not ship fix until we have more information.
Thanks. I'll request to back this out.
Comment 17•4 years ago
|
||
Backed out changeset 4ffa919d35f9 (Bug 1687364) as requested by kershaw.
Backout link: https://hg.mozilla.org/integration/autoland/rev/20982512637917c0a73129f8eb99cafbadff9901
Updated•4 years ago
|
Updated•4 years ago
|
Reporter | ||
Comment 18•4 years ago
|
||
(In reply to Anne (:annevk) from comment #11)
Steven, did you also file this bug against Chrome and Safari? Could you copy me on those bugs?
I have not done so yet, but it looks like you've already discovered that. Do you think I still should?
Comment 19•4 years ago
|
||
You could, but if you do please do mention that this is an issue in all browsers.
Reporter | ||
Comment 20•4 years ago
|
||
Reported to Chromium: https://bugs.chromium.org/p/chromium/issues/detail?id=1176753
Reported to Safari, no link to share.
Comment 21•4 years ago
|
||
Backout merged: https://hg.mozilla.org/mozilla-central/rev/209825126379
Comment 22•4 years ago
|
||
There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:kershaw, could you have a look please?
For more information, please visit auto_nag documentation.
Assignee | ||
Comment 23•4 years ago
|
||
(In reply to Release mgmt bot [:sylvestre / :calixte / :marco for bugbug] from comment #22)
There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:kershaw, could you have a look please?
For more information, please visit auto_nag documentation.
The patch was backed out.
Comment 24•3 years ago
|
||
Do you know the consensus with the other browsers? Chrome's issue is still hidden, and there's no link for Safari.
Reporter | ||
Comment 25•3 years ago
|
||
The Chromium bug now links to a whatwg github discussion I do not have access to. Link: https://github.com/whatwg/fetch/security/advisories/GHSA-6rrj-jgxg-wf55.
Additionally, they added a use counter for this, so they could determine impact: https://chromium.googlesource.com/chromium/src/+/2dfc61141303d4c4633a1366b8e34e32aefec614
I haven't seen any other updates on the Chromium bug tracker, but I'm not sure what is happening in the WHATWG discussion which may be relevant.
Comment 26•3 years ago
|
||
Chrome folks gathered some telemetry (see https://www.chromestatus.com/metrics/feature/timeline/popularity/3873), but otherwise nothing changed. I asked them about next steps.
Comment 27•3 years ago
|
||
As a follow-up, Chrome said they will try to deprecate this behavior and stop supporting it, despite the relatively high usage. I suggest we closely monitor that and follow once they ship (or try to ship around the same time).
Updated•3 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•1 years ago
|
Comment 28•1 year ago
|
||
I think this is public information at this point and the planned cross-browser deprecation would benefit from giving this issue more visibility. If noone objects, I suggest we make this bug public in the following week (week of May 22nd 2023).
Can we aim to land a patch with a console warning (and ideally Telemetry?) as a first step here? Ideally, this should happen soon.
Assignee | ||
Comment 29•1 year ago
|
||
(In reply to Frederik Braun [:freddy] from comment #28)
I think this is public information at this point and the planned cross-browser deprecation would benefit from giving this issue more visibility. If noone objects, I suggest we make this bug public in the following week (week of May 22nd 2023).
Can we aim to land a patch with a console warning (and ideally Telemetry?) as a first step here? Ideally, this should happen soon.
Sure. I am working on this.
Updated•1 year ago
|
Assignee | ||
Comment 30•1 year ago
|
||
Depends on D102932
Assignee | ||
Comment 31•1 year ago
|
||
Depends on D178043
Assignee | ||
Comment 32•1 year ago
|
||
Assignee | ||
Updated•1 year ago
|
Comment 33•1 year ago
|
||
Before I begin with the data review, is there a process by which this bug will become public when this data collection ships? Data Review must be done in the open, so if this bug must remain closed while data collection happens, we'll need a separate open bug for the review.
Assignee | ||
Comment 34•1 year ago
|
||
(In reply to Chris H-C :chutten from comment #33)
Before I begin with the data review, is there a process by which this bug will become public when this data collection ships? Data Review must be done in the open, so if this bug must remain closed while data collection happens, we'll need a separate open bug for the review.
Yes, please see comment #28. This bug will be public soon.
Comment 35•1 year ago
|
||
Comment on attachment 9334179 [details]
bug1687364_data_review.txt
PRELIMINARY NOTE:
Since this collection is Glean, you might find ./mach data-review <bug number>
to be of particular use to help generate your Data Collection Review Request for future requests.
DATA COLLECTION REVIEW RESPONSE:
Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate?
Yes.
Is there a control mechanism that allows the user to turn the data collection on and off?
Yes. This collection can be controlled through Firefox's Preferences.
If the request is for permanent data collection, is there someone who will monitor the data over time?
No. This collection will expire in Firefox 130.
Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?
Category 1, Technical.
Is the data collection request for default-on or default-off?
Default on for all channels.
Does the instrumentation include the addition of any new identifiers?
No.
Is the data collection covered by the existing Firefox privacy notice?
Yes.
Does the data collection use a third-party collection tool?
No.
Result: datareview+
Updated•1 year ago
|
Comment 36•1 year ago
|
||
Unhiding because the issue is public in fetch, and we're running into webcompat trying to fix it (as is Chrome) so clearly people need to know this is changing.
Comment 37•1 year ago
|
||
Comment 39•1 year ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/95f1e52ddf84
https://hg.mozilla.org/mozilla-central/rev/04c0e2ee9aa8
https://hg.mozilla.org/mozilla-central/rev/ff16d0fa8ed8
Comment 41•1 year ago
|
||
FF115 MDN docs for this can be tracked in https://github.com/mdn/content/issues/27230
Can you please confirm my understanding:
-
A server sends
Access-Control-Allow-Headers
in a response to a preflight request to indicate which non-CORS-safelisted headers it is OK to be included in the real request.- If used with a wildcard
*
mdn says any of the headers may be send in the real request exceptauthorization
, which must be explicitly specified. - However FF has been allowing
Authorization
in the actual response even ifAccess-Control-Allow-Headers is wildcard (*)
- So:
- this fix enforces the rule that
Authorization
must be included in the server response header, behind a prefnetwork.cors_preflight.authorization_covered_by_wildcard
which is default off.
- this fix enforces the rule that
- The intent would then be to turn this on at some point? For now this is an experimental feature.
- right now no one obeys the rule - right?
- If used with a wildcard
-
In a comment c4 above it sounds like there is another bug with MDN docs.
- Specifically Access-Control-Expose-Headers indicates also mentions that Authorization header can't be wildcarded.
- The bug is that this makes no sense -
Access-Control-Expose-Headers
indicates the response headers that can be exposed to user code, whileAuthorization
is a request header. So that comments should probably be stripped out.
Is that all correct?
Assignee | ||
Comment 42•1 year ago
|
||
(In reply to Hamish Willee from comment #41)
FF115 MDN docs for this can be tracked in https://github.com/mdn/content/issues/27230
Can you please confirm my understanding:
A server sends
Access-Control-Allow-Headers
in a response to a preflight request to indicate which non-CORS-safelisted headers it is OK to be included in the real request.
- If used with a wildcard
*
mdn says any of the headers may be send in the real request exceptauthorization
, which must be explicitly specified.- However FF has been allowing
Authorization
in the actual response even ifAccess-Control-Allow-Headers is wildcard (*)
- So:
- this fix enforces the rule that
Authorization
must be included in the server response header, behind a prefnetwork.cors_preflight.authorization_covered_by_wildcard
which is default off.- The intent would then be to turn this on at some point? For now this is an experimental feature.
- right now no one obeys the rule - right?
In a comment c4 above it sounds like there is another bug with MDN docs.
- Specifically Access-Control-Expose-Headers indicates also mentions that Authorization header can't be wildcarded.
- The bug is that this makes no sense -
Access-Control-Expose-Headers
indicates the response headers that can be exposed to user code, whileAuthorization
is a request header. So that comments should probably be stripped out.Is that all correct?
Yes, that's all correct.
Thanks!
Updated•1 year ago
|
Updated•1 year ago
|
Comment 43•1 year ago
|
||
Updated•1 year ago
|
Assignee | ||
Comment 44•1 year ago
|
||
Please note that the pref network.cors_preflight.authorization_covered_by_wildcard
is still true in Firefox 115, which means that Authorization header is still covered by wildcard.
I've filed bug 1841019 for turning it off, but the timeline is still unknown.
Updated•1 year ago
|
Description
•