1694679 - Extensions redirecting same-origin requests are blocked by CORS checks

Status: RESOLVED FIXED

(WebExtensions :: Request Handling, defect)

Description

[Jonathan Kingston [:jkt] he/him]

STR:

1. Add uBlock addon
2. Go to any site, open devtools
3. Create script element with crossorigin

var s = document.createElement('script')
s.crossOrigin = 'anonymous'
s.src = "https://google-analytics.com/ga.js"
document.body.appendChild(s)

4. See CORS error in console
   Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at moz-extension://d9a86155-a428-3e4f-9f56-6682fc1b19d4/web_accessible_resources/google-analytics_ga.js?secret=vi4xip. (Reason: CORS request not http).

---

• These request gets treated as a preflight and also the expectation of the Channel being HTTP is in several places within: nsCORSListenerProxy.cpp
• Without the crossOrigin attribute the code works as expected and this works in Chrome also.

My hunch is this code probably should be treated as same-site within nsCORSListenerProxy.cpp when the loadingPrincipal is from an expanded principal. As securityMode isn't writable there isn't an obvious place for this change immediately to myself.

Perhaps the much simpler thing is stop the channel in flight and do the special casing work within: WebRequest.jsm

This was found by Jason (cc'd) I just investigated the code flow within Firefox.

Comment 1

[Rob Wu [:robwu]]

This is marked as a regression of bug 1645204. Strictly speaking, that's true, but I guess that this has always been the behavior, but temporarily hidden by the presence of bug 1645204 (because that disabled enforcement of the CORS checks after redirects, which itself is a regression of bug 1450965).
Bug 1645204 tried to re-implement bug 1450965 without a gaping security hole, by relying on the usual CORS logic that applies to websites, at https://searchfox.org/mozilla-central/rev/a6db3bd67367aa9ddd9505690cab09b47e65a762/toolkit/components/extensions/webrequest/WebRequest.jsm#1005-1031

Ideally we want to disable the preflight for extension-triggered redirects, as we are currently hitting https://searchfox.org/mozilla-central/rev/a6db3bd67367aa9ddd9505690cab09b47e65a762/netwerk/protocol/http/nsCORSListenerProxy.cpp#979-998 .
Last time we relied on a flag, the flag stuck unexpectedly and caused a security issue (bug 1645204), so I'd be very cautious towards relaxing checks.
There is a allowInsecureRedirectToDataURI flag that is used by WebRequest.jsm - perhaps we can skip preflights for redirects to moz-extension and data:-URLs when the flag is set.

Perhaps the much simpler thing is stop the channel in flight and do the special casing work within: WebRequest.jsm

What do you have in mind? Do you know of a way to redirect a channel (before AND after a request has actually been sent) to a synthesized channel (with a custom redirection target or response body/headers)?

uBlock Origin (and some other add-ons such as Decentraleyes)'s objective here is not to redirect to a moz-extension:-principal, but to serve a custom response body for a request without involving the network. Ideally transparently (i.e. disabling other potentially breaking stuff such as SRI - bug 1321916).

Note: This also happens for redirects to data:-URLs (which are supposedly allowed). The observed result looks identical to the STR for this bug, at least in Firefox 87).

STR:

1. In an extension with "webRequest", "webRequestBlocking","<all_urls>" permissions (e.g. uBlock origin), run the following snippet (e.g. via the devtools of the background script):

browser.webRequest.onBeforeRequest.addListener(det => {
  console.log(det);
  return {redirectUrl: 'data:text/javascript,alert(origin)'};
}, {urls:['*://example.com/test']}, ['blocking']);

2. At example.net, run the following snippet (e.g. from the devtools):

var s = document.createElement('script')
s.crossOrigin = 'anonymous'
s.src = "https://example.com/test"
document.body.appendChild(s)

Expected: request succeeds, alert dialog
Actual: request blocked, errors in example.net's console:

• Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at data:text/javascript,alert(origin). (Reason: CORS request not http).
• Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at https://example.com/test. (Reason: CORS request did not succeed).
• The network tab of example.net's devtools shows NS_ERROR_DOM_BAD_URI (without the extension, CORS Missing Allow Origin is shown as expected)

Comment 2

[Jonathan Kingston [:jkt] he/him]

Bug 1645204 tried to re-implement bug 1450965 without a gaping security hole

I think the previous implementation was much closer to all other code Firefox has though, perhaps a different flag could be specified in WebRequest.jsm that only drops the need for credentials in securityMode when redirecting to moz-extension and be careful about dropping this flag. The difference being though is that even if the flag persisted the impact wouldn't be as severe.

There is a allowInsecureRedirectToDataURI flag that is used by WebRequest.jsm - perhaps we can skip preflights for redirects to moz-extension and data:-URLs when the flag is set.

Yeah I think the only thing that stopped this being the same security issue is the following: https://searchfox.org/mozilla-central/source/netwerk/protocol/http/HttpBaseChannel.cpp#1954

What do you have in mind? Do you know of a way to redirect a channel (before AND after a request has actually been sent) to a synthesized channel (with a custom redirection target or response body/headers)?

I hadn't given it much thought in all honesty, I know service workers have this special casing in their own file.

Ideally we want to disable the preflight for extension-triggered redirects, as we are currently hitting

There is also: https://searchfox.org/mozilla-central/source/netwerk/protocol/http/nsCORSListenerProxy.cpp#510-515 which I was seeing too. I gave up after having several special cases that were broken so we could talk it through more.

Comment 3

[Jonathan Kingston [:jkt] he/him]

uBlock Origin (and some other add-ons such as Decentraleyes)'s objective here is not to redirect to a moz-extension:-principal, but to serve a custom response body for a request without involving the network.

Also sure, but the principal could be set specifically from the extension policy meaning that other requests like evil.com can't redirect to moz-extension with the special casing behaviour. Flags are equally fine and I guess prevent the website potentially being able to manipulate the extension.

Comment 4

[Rob Wu [:robwu]]

(In reply to Jonathan Kingston [:jkt] he/him from comment #2)

Bug 1645204 tried to re-implement bug 1450965 without a gaping security hole

I think the previous implementation was much closer to all other code Firefox has though, perhaps a different flag could be specified in WebRequest.jsm that only drops the need for credentials in securityMode when redirecting to moz-extension and be careful about dropping this flag. The difference being though is that even if the flag persisted the impact wouldn't be as severe.

I think that multiple flags is too fragile. It's already difficult enough to keep track of one flag (as shown by bug 1645204 and bug 1694711). I believe that we should have a field to fully replace the response (either a new channel or a new field on an existing channel to queue for the right time to redirect the channel itself) specifically for use case of extensions / WebRequest.jsm.

There is a allowInsecureRedirectToDataURI flag that is used by WebRequest.jsm - perhaps we can skip preflights for redirects to moz-extension and data:-URLs when the flag is set.

Yeah I think the only thing that stopped this being the same security issue is the following: https://searchfox.org/mozilla-central/source/netwerk/protocol/http/HttpBaseChannel.cpp#1954

I thought that redirectTo was only used by extensions (e.g. in WebRequest.jsm, which sets the flag immediately after channel.redirectTo).
I wondered, is the flag correctly reset? Apparently not... I filed bug 1694711.

Ideally we want to disable the preflight for extension-triggered redirects, as we are currently hitting

There is also: https://searchfox.org/mozilla-central/source/netwerk/protocol/http/nsCORSListenerProxy.cpp#510-515 which I was seeing too. I gave up after having several special cases that were broken so we could talk it through more.

For future reference, permalink to nsCORSListenerProxy::CheckRequestApproved.

(In reply to Jonathan Kingston [:jkt] he/him from comment #3)

uBlock Origin (and some other add-ons such as Decentraleyes)'s objective here is not to redirect to a moz-extension:-principal, but to serve a custom response body for a request without involving the network.

Also sure, but the principal could be set specifically from the extension policy meaning that other requests like evil.com can't redirect to moz-extension with the special casing behaviour. Flags are equally fine and I guess prevent the website potentially being able to manipulate the extension.

The extension protocol handler already enforces a policy that extension resources can be read iff declared in web_accessible_resources. moz-extension:-URLs have a unique UUID as the host name (which has pros and cons - bug 1405971), so websites cannot easily redirect to an extension resource. Two issues that I can think of are loading extension resources to bypass restrictions (e.g. CSP) or privacy (bug 1405971). These two issues do not require a redirect, so I don't think that there is a significant issue here.

... in fact, after writing all about this, I guess that a way to fix this bug is to just allow redirects to any moz-extension:-URL...

Comment 5

[(inactive) Nihanth Subramanya [:nhnt11]]

Rob, you seem to have a good understanding of what is going on here. Can you share whether you think this is a bug that needs immediate attention, or better yet, assign a priority?

Comment 6

[Rob Wu [:robwu]]

I'll move this to the WebExtensions component for triaging. The code change may be in network or DOM (security) code, but the decision on the desired behavior is for extensions.

In comment 1 I already referred to bug 1321916, I've added a "See also" for better visibility.

Comment 7

[Jonathan Kingston [:jkt] he/him]

Doesn't this impact the shims here: https://searchfox.org/mozilla-central/source/browser/extensions/webcompat/shims ?

Comment 8

[Thomas Wisniewski [:twisniewski]]

Good question! I presume that it is, given that I simply redirect to a getURL in the extension to do the shimming, here: https://searchfox.org/mozilla-central/source/browser/extensions/webcompat/lib/shims.js#417

However it's not a problem right now, because even if that request is blocked, the end result will just be that the shim doesn't work -- it won't break anything more than strict mode blocking the request will have done. It would be very nice to avoid that happening of course. And if we ever wish to use shims on requests which wouldn't normally be blocked by strict ETP (for webcompat especially), I imagine that this become an issue.

Also note that fixing this would really help with webcompat diagnoses, as addons like Tinker Tester Developer Spy are currently affected by CORS (as is Developer in the Middle potentially). I'm using a pretty nasty hack to rewrite CORS headers in TTDS to avoid most of the CORS issues, but it would be great to avoid doing that. (Of course, that would mean allowing any addon-driven redirect, rather than just ones to web-extension: URLs, and I'm not sure if we're willing to go that far).

Comment 9

[Rob Wu [:robwu]]

P3 S3 from the WebExtensions perspective.

Shane is going to take another look to see if there is a specific product/component to move this bug to.

Comment 11

[Rob Wu [:robwu]]

This is not only affecting redirects of same-origin requests, but also redirects of cross-origin requests, such as fonts. See example-font-direct.html from demo2.zip in bug 1698288 , and the analysis in https://bugzilla.mozilla.org/show_bug.cgi?id=1698288#c15 .

Comment 12

[Rob Wu [:robwu]]

(In reply to Rob Wu [:robwu] from comment #4)

The extension protocol handler already enforces a policy that extension resources can be read iff declared in web_accessible_resources. moz-extension:-URLs have a unique UUID as the host name (which has pros and cons - bug 1405971), so websites cannot easily redirect to an extension resource. Two issues that I can think of are loading extension resources to bypass restrictions (e.g. CSP) or privacy (bug 1405971). These two issues do not require a redirect, so I don't think that there is a significant issue here.

... in fact, after writing all about this, I guess that a way to fix this bug is to just allow redirects to any moz-extension:-URL...

I'm going to implement this.

Comment 13

[Rob Wu [:robwu]]

Attachment: Bug 1694679 - Skip CORS for moz-extension:-URLs

moz-extension:-URLs cannot be loaded by default, unless an extension
explicitly lists the resource in web_accessible_resources. At that
point, a URL is considered world-readable, and the load should succeed
regardless of the requested CORS mode in the fetch/request.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6f298a911d9c
Skip CORS for moz-extension:-URLs r=ckerschb,mixedpuppy,necko-reviewers,dragana

Comment 15

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/6f298a911d9c

Comment 16

[Rob Wu [:robwu]]

Would be nice to mention this in the changelog of 89.

Comment 17

[Rob Wu [:robwu]]

Submitted pull request with documentation update at https://github.com/mdn/content/pull/5186