1471502 - A response with multiple Access-Control-Allow-Origin headers produces a misleading console error message

Status: VERIFIED FIXED

(Core :: DOM: Security, defect, P2)

Description

[Stuart Cook]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:61.0) Gecko/20100101 Firefox/61.0
Build ID: 20180621125625

Steps to reproduce:

Set up a server that incorrectly responds with multiple Access-Control-Allow-Origin headers, and make a CORS request to it.


Actual results:

The response is blocked, and a misleading error message appears in the Web Console:

Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at <url>. (Reason: CORS header ‘Access-Control-Allow-Origin’ does not match ‘(null)’).


Expected results:

The error message should indicate that the response was blocked due to having multiple Access-Control-Allow-Origin headers.

Comment 1

[Stuart Cook]

It looks like the fix for https://bugzilla.mozilla.org/show_bug.cgi?id=1279324 recycled an existing error string, but that existing string isn't a good match for the actual failure.

Comment 2

[Stuart Cook]

The current message is particularly confusing when the error is caused by two identical Access-Control-Allow-Origin headers that both have the correct value. It implies that the header value did not match the origin, when in fact that check never took place at all, due to the duplicated header.

Comment 3

[Daniel Bodea [:danibodea]]

Hi Stuart,

Please provide more detailed and clear steps to reproduce so we can confirm it; Most of the team responsible with bug triage does not have such a high technical skill.

Thank you for your contribution!

Comment 4

[Stuart Cook]

The relevant code is at <https://dxr.mozilla.org/mozilla-central/rev/23885c14f025b61bb74d85845ac4018f05eb39f8/netwerk/protocol/http/nsCORSListenerProxy.cpp#587>.

It uses CheckOriginHeader to check whether the response has more than one "Access-Control-Allow-Origin" header, and then uses "CORSAllowOriginNotMatchingOrigin" to indicate an error if it does.

Via <https://dxr.mozilla.org/mozilla-central/rev/23885c14f025b61bb74d85845ac4018f05eb39f8/dom/locales/en-US/chrome/security/security.properties#14>, that string translates to the observed error message: "Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at %1$S. (Reason: CORS header ‘Access-Control-Allow-Origin’ does not match ‘%2$S’).".

However, that message text isn't appropriate for the failed check that caused the message to be displayed.

Comment 5

[Daniel Bodea [:danibodea]]

I cannot confirm this issue with the information provided, but I will set it's component as DevTools: Console and let developers decide whether this issue is valid or not.

Thank you for your contribution, Stuart!

Comment 6

[Nicolas Chevobbe [:nchevobbe]]

The person that introduced this doesn't seem to be active in bugzilla for 2 years now, so let's ask the reviewer (ckerschb) what they think about this.

Comment 7

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #6)
> The person that introduced this doesn't seem to be active in bugzilla for 2
> years now, so let's ask the reviewer (ckerschb) what they think about this.

Yeah, I can see that we could do a better job in reporting that CORS error. Vino, you recently did a bunch of improvements to the CORS console logging within Bug 1442551. Can you take a look? Probably worth adding a new message for that error (see comment 0 and 4).

@Stuart, thanks for reporting and adding so much detail - highly appreciated!

Comment 8

[Vinothkumar Nagasayanan [:vinoth]]

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #7)
> (In reply to Nicolas Chevobbe [:nchevobbe] from comment #6)
> > The person that introduced this doesn't seem to be active in bugzilla for 2
> > years now, so let's ask the reviewer (ckerschb) what they think about this.
> 
> Yeah, I can see that we could do a better job in reporting that CORS error.
> Vino, you recently did a bunch of improvements to the CORS console logging
> within Bug 1442551. Can you take a look? Probably worth adding a new message
> for that error (see comment 0 and 4).
> 
> @Stuart, thanks for reporting and adding so much detail - highly appreciated!

Yes I will work on this. Thanks.

Comment 9

[Vinothkumar Nagasayanan [:vinoth]]

Attachment: Bug 1471502 - Misleading console error message fixed

Comment 10

[Vinothkumar Nagasayanan [:vinoth]]

Comment on attachment 8989926 [details]
Bug 1471502 - Misleading console error message fixed

Please kindly review the patch and let me know if changes are needed.

Corresponding TRY server push,
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b468752c89035500c58a55237b18c57af9b046d5

Comment 11

[Phabricator Automation]

Comment on attachment 8989926 [details]
Bug 1471502 - Misleading console error message fixed

Christoph Kerschbaumer [:ckerschb] has approved the revision.

https://phabricator.services.mozilla.com/D1965

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/84262b86904d
Misleading console error message fixed r=ckerschb

Comment 13

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/84262b86904d

Comment 14

[Daniel Bodea [:danibodea]]

Hello Stuart and Vinothkumar,

I see that this code line has been added: "CORSMultipleAllowOriginNotAllowed=Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at %1$S. (Reason: Multiple CORS header ‘Access-Control-Allow-Origin’ not allowed)."
along with the already existing:
"CORSMissingAllowOrigin=Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at %1$S. (Reason: CORS header ‘Access-Control-Allow-Origin’ missing)."

I would like to verify this, but I don't have the technical knowledge to do it. How do I set up a server that incorrectly responds with multiple Access-Control-Allow-Origin headers, and make a CORS request to it?

Thanks!

Comment 15

[Vinothkumar Nagasayanan [:vinoth]]

Attachment: server.py

Comment 16

[Vinothkumar Nagasayanan [:vinoth]]

Attachment: contactserver.html

(In reply to Bodea Daniel [:danibodea] from comment #14)
> Hello Stuart and Vinothkumar,
> 
> I see that this code line has been added:
> "CORSMultipleAllowOriginNotAllowed=Cross-Origin Request Blocked: The Same
> Origin Policy disallows reading the remote resource at %1$S. (Reason:
> Multiple CORS header ‘Access-Control-Allow-Origin’ not allowed)."
> along with the already existing:
> "CORSMissingAllowOrigin=Cross-Origin Request Blocked: The Same Origin Policy
> disallows reading the remote resource at %1$S. (Reason: CORS header
> ‘Access-Control-Allow-Origin’ missing)."
> 
> I would like to verify this, but I don't have the technical knowledge to do
> it. How do I set up a server that incorrectly responds with multiple
> Access-Control-Allow-Origin headers, and make a CORS request to it?
> 
> Thanks!

You can run the server.py file in some server using 'python ./server.py' and  please change the ip address of that server in contactserver.html. Contactserver.html sends a xhr request to the server, which responds with multiple Access-Control-Allow-Origin headers.

Comment 17

[Ton [:Tonnes]]

> +CORSMultipleAllowOriginNotAllowed=Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at %1$S. (Reason: Multiple CORS header ‘Access-Control-Allow-Origin’ not allowed).

Shouldn’t this include "Multiple CORS headerS" instead?

Comment 18

[Daniel Bodea [:danibodea]]

I have tested the fix using the following steps:
Precondition: Have Python 2.7 installed and have the path included in System Variables.
1. Start the "server.py" server provided by the reporter on port 8000 
 - "python server.py 8000" command in the folder where the server.py is file is downloaded
2. Start "SimpleHTTPServer" server included in Python2.7 on port 8888
 - "python -m SimpleHTTPServer 8888" command in a different terminal and in the same folder
3. Open browser, open the Network Developer Tools and reach "localhost:8888/contanctserver.html" page.
4. Notice the 2 GET events prformed to domain "localhost:8888";
5. Open Browser Console (CTRL+SHIFT+J);
6. Click on the "Change Content" button.
7. Notice the POST event to domain "localhost:8000".
8. Notice error displayed in Browser Console!


I have reproduced this issue on Nightly 63.0a1 (2018-07-01) (64-bit). Message displayed:
"Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at http://localhost:8000/. (Reason: CORS header ‘Access-Control-Allow-Origin’ does not match ‘(null)’)."

I have confirmed the fix on Nightly 63.0a1 (2018-07-30) (64-bit). Message displayed:
"Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at http://localhost:8000/. (Reason: Multiple CORS header ‘Access-Control-Allow-Origin’ not allowed)."

Considering all above, this issue is verified!

Comment 19

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Nicolas, can you request uplift here to unblock the uplift of bug 1475391?

Comment 20

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8989926 [details]
Bug 1471502 - Misleading console error message fixed

Approval Request Comment
[Feature/Bug causing the regression]: - (not a regression)
[User impact if declined]: Misleading CORS error message
[Is this code covered by automated tests?]: not here, but in Bug 1475391, where an uplift was requested as well.
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: -
[List of other uplifts needed for the feature/fix]: -
[Is the change risky?]: No
[Why is the change risky/not risky?]: Because it's a 2 line change: instead of sending an existing string for the error message, we send a new, more detailed one
[String changes made/needed]:`CORSMultipleAllowOriginNotAllowed` was added in dom/locales/en-US/chrome/security/security.properties

Comment 21

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8989926 [details]
Bug 1471502 - Misleading console error message fixed

Additional fix for error message link changes, let's uplift for beta 17.

Comment 22

[Liz Henry (:lizzard) (relman/hg->git project)]

This adds a new string, flod, can the l10n team handle it or should we reconsider this uplift?

Comment 23

[Liz Henry (:lizzard) (relman/hg->git project)]

From discussion with flod, it is ok, and we have some locales already done (in nightly).

Comment 24

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/a59e5c3dda04

Comment 25

[Abe - QA (:Abe_LV)]

I tested this on beta 62.0b17 using the steps in comment 18. 
In beta 62.0b16, browser console shows "Reason: CORS header ‘Access-Control-Allow-Origin’ does not match ‘(null)’)".
Beta 62.0b17 (has the fix) shows "Reason: Multiple CORS header ‘Access-Control-Allow-Origin’ not allowed)".

Based on this, the bug is verified as fixed on beta as well.

Test Environments:
Version 	62.0b17
Build ID 	20180813190114
Update Channel 	beta
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:62.0) Gecko/20100101 Firefox/62.0