This is from bug 949706 - Actually I'm not sure about this bug, the alert is blocked on Chrome, is there something missing? PoC: http://vulnerabledoma.in/fx_csp_bypass_with_importScripts CSP does not block unpermitted resource.

This is blocked on Chrome because they don't allow a redirect to a data: url (I thought we blocked it too!). Nothing to do with CSP (which doesn't mean the CSP behavior isn't wrong, just that it shouldn't come into play here). Workers don't inherit the loading page's CSP (most of it doesn't even apply to workers), workers need to be served with their own CSP. https://w3c.github.io/webappsec-csp/2/#processing-model-workers (actually workers loaded from blob: or data: do inherit, but a worker with a real URL from a real server needs to be sent with a header to get a policy)

As discussed, I'll fix that, clearing ni? for me.

Honza, we would like to block insecure redirects to data: URIs. At this point I am not entirely sure whether to block redirects to data: for all subresource loads or just for scripts. Anyway, where in Necko code would be the best place to block such redirects?

This is where we enforce nsILoadInfo::SEC_DONT_FOLLOW_REDIRECTS: https://searchfox.org/mozilla-central/rev/48cbb200aa027a0a379b6004b6196a167344b865/netwerk/base/nsAsyncRedirectVerifyHelper.cpp#84 Perhaps somewhere around there.

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #3) > Honza, we would like to block insecure redirects to data: URIs. At this > point I am not entirely sure whether to block redirects to data: for all > subresource loads or just for scripts. Anyway, where in Necko code would be > the best place to block such redirects? You should probably do this as part of the CSP checking, likely somewhere around/after [1] (called from [2]). If not suitable, then [3] could be used to hard-block, but I would not personally suggest that. CSP is a better place. [1] https://searchfox.org/mozilla-central/rev/48cbb200aa027a0a379b6004b6196a167344b865/dom/security/nsCSPContext.cpp#179 [2] https://searchfox.org/mozilla-central/rev/48cbb200aa027a0a379b6004b6196a167344b865/dom/security/nsCSPService.cpp#246 [3] https://searchfox.org/mozilla-central/rev/48cbb200aa027a0a379b6004b6196a167344b865/netwerk/protocol/http/HttpBaseChannel.cpp#3399

(In reply to Ben Kelly [:bkelly] from comment #4) > This is where we enforce nsILoadInfo::SEC_DONT_FOLLOW_REDIRECTS: > > https://searchfox.org/mozilla-central/rev/ > 48cbb200aa027a0a379b6004b6196a167344b865/netwerk/base/ > nsAsyncRedirectVerifyHelper.cpp#84 > > Perhaps somewhere around there. Could stand as an option between [1] and [3] from the previous comment, yes.

Thanks Ben and Honza. Looking a little closer maybe the right place in the ContentSecurityManager where we perform security checks for redirects [1]. [1] https://dxr.mozilla.org/mozilla-central/source/dom/security/nsContentSecurityManager.cpp#584

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #7) > Thanks Ben and Honza. Looking a little closer maybe the right place in the > ContentSecurityManager where we perform security checks for redirects [1]. > > [1] > https://dxr.mozilla.org/mozilla-central/source/dom/security/ > nsContentSecurityManager.cpp#584 Yes, this looks like the right place!

Hey smaug, this patch blocks insecure redirects to data URIs for scripts. Chrome is doing this and we should do the same in my opinion. Not sure about other types of subresource loads. Happy to block more but I think only script is really worth blocking. The STR from comment 0 [1] now work correctly and log a message to the console that the load was blocked. [1] http://vulnerabledoma.in/fx_csp_bypass_with_importScripts

And the automated test for that new behavior.

This blocks for worker scripts, importScripts, and modules, right?

(In reply to Ben Kelly [:bkelly] from comment #11) > This blocks for worker scripts, importScripts, and modules, right? It blocks all redirects to a data: URI for loads that are loaded using the TYPE_SCRIPT, which includes worker scripts as well as importscripts. I don't know what you mean by modules though.

I meant `<script module>`. I guess that uses TYPE_SCRIPT as well?

(In reply to Ben Kelly [:bkelly] from comment #13) > I meant `<script module>`. I guess that uses TYPE_SCRIPT as well? Oh, I see. Yes it does.

Comment on attachment 8943649 [details] [diff] [review] bug_1428793_test_insec_redirects_to_data.patch do you need to escape the url here? And, actually, do you need to unescape in c++ code? Could you test also what happens when non-escaped data url is used?

Comment on attachment 8943649 [details] [diff] [review] bug_1428793_test_insec_redirects_to_data.patch I guess this should be r-, since we do need to test also workers and modules and such. And why the data url is "<script>alert('this alert should be blocked');</script>"; Shouldn't it be just alert('this alert should be blocked');

(In reply to Olli Pettay [:smaug] from comment #15) > do you need to escape the url here? > And, actually, do you need to unescape in c++ code? I guess so, if we don't unescape the URL then the error log looks like this Security Error: Content at http://mochi.test:8888/tests/dom/security/test/general/test_block_subresource_redir_to_data.html may not load data from data:text/html,onmessage%20%3D%20function%28event%29%20%7B%20postMessage%28%27worker-loaded%27%29%3B%20%7D which seems hard to decipher. I would rather keep the unescaping.

Hey Ben, I am about to extend the test to not only handle regular script, but also workers and module scripts. If I apply the patch with the code changes within the contentSecurityManager to stop redirects to data: URIs then I get three tests that pass. If I do the opposite and just run the test without the actual code changes applied, then I observe the following: * regular script tests loads (as expected) * worker script gets blocked, because workers need to be same origin (exepcted, but not sure how much sense it makes to keep that subtest within that file) * script modules gets blocked with the following error in the console: Module source URI is not allowed in this document: “http://mochi.test:8888/tests/dom/security/test/general/file_block_subresource_redir_to_data.sjs?modulescript”. I tried many things, but I can't get even a regular module script to load; not even without any redirect testing. Am I missing something? Asked differently, can you guide me to a test that should work?

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #18) > * worker script gets blocked, because workers need to be same origin > (exepcted, but not sure how much sense it makes to keep that subtest within > that file) Interesting. We do allow data URL workers with an opaque origin. Its interesting that the same-origin redirect check gets to it first. In theory we could let redirect to data URL from secure origins work in worker scripts. I'm not sure sure its worth the effort, though. I'm happy to leave the test for now, though. We want something that verifies its blocked, regardless of the reason. > * script modules gets blocked with the following error in the console: > > Module source URI is not allowed in this document: > “http://mochi.test:8888/tests/dom/security/test/general/ > file_block_subresource_redir_to_data.sjs?modulescript”. > > I tried many things, but I can't get even a regular module script to load; > not even without any redirect testing. Am I missing something? Asked > differently, can you guide me to a test that should work? Probably want to ask Jon about testing modules.

Olli, would you be fine with me landing the change only using the testcase for regular script and workers and file a follow up for script modules? Given the merge date I would like to get that landed?

Sounds ok

But try to find the bug to enable modules in release builds and file a bug to add the test and make that to block the enabling bug.

Olli, I filed Bug 1432137 for adding the script module test. Could you please green light this test? Thanks!

Comment on attachment 8944385 [details] [diff] [review] bug_1428793_test_insec_redirects_to_data.patch Review of attachment 8944385 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/security/test/general/test_block_subresource_redir_to_data.html @@ +31,5 @@ > +testScriptRedirectToData.onload = function() { > + ok(false, "script that redirects to data: URI should not load"); > + checkFinish(); > +} > +testScriptRedirectToData.src = "file_block_subresource_redir_to_data.sjs?script"; I'm confused by this. Does setting the 'src' attribute for a script that has already been processed make it get loaded again? Why does that happen? Note modules are enabled by default in nightly only at the moment, otherwise you'll need to set dom.moduleScripts.enabled.

That src attribute setting works because of https://searchfox.org/mozilla-central/rev/b7e3ec2468d42fa59d86c03ec7afeb209813f1d4/dom/script/ScriptElement.cpp#125 See HasScriptContent() call.

Pushed by mozilla@christophkerschbaumer.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/bb6f3f1ffb41 Block insecure redirects to data: URIs. r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/ccef1ce1cb68 Test block insecure redirects to data: URIs. r=smaug

https://hg.mozilla.org/mozilla-central/rev/bb6f3f1ffb41 https://hg.mozilla.org/mozilla-central/rev/ccef1ce1cb68

(In reply to Olli Pettay [:smaug] from comment #25) Thanks.

I am pretty sure the change here is what breaks uBlock Origin's ability to redirect blocked network requests to local neutered scripts (through data: URIs). This is a big deal, as redirection is used by uBlock Origin for key core features such as: - Preventing site breakages as a result of blocking scripts - Foiling anti-blocker mechanisms Consider this issue: https://github.com/uBlockOrigin/uAssets/issues/1370 I get this at the browser console: > Redirecting to insecure data: URI not allowed > (Blocked loading of: “data:application/javascript;base64,KGZ1bmN0aW9uKCk...”) uBO's anti-blocker abilities no longer work with Nightly. It still work fine for FF58, Chromium 63, Chrome 65. I would appreciate insights about I could restore these uBO core features in Nightly given the changes here.

Hm, interesting. I am wondering what kind of distinction Chrome makes when blocking redirects as insecure and when not, given that Chrome also blocks the POC from comment 0: http://vulnerabledoma.in/fx_csp_bypass_with_importScripts I guess that needs some further debugging. I'll take a look and post an update here!

Chromium will also block uBO's redirects to data: URIs *if* a site declares a CSP header which prevent the use of data: URIs. This is actually an open issue on Chromium's issue tracker[1]. However the site provided as having the issue[2] does not declare any CSP header, so it appears the change here affects all redirections to data: URIs, even without a CSP directive forbidding data: URIs. Consider this very simple test case, in which uBO blocks and redirects the Google Analytics script: http://motherfuckingwebsite.com/ It does not work with Nightly, even though the site does not declare any CSP header. *** [1] https://bugs.chromium.org/p/chromium/issues/detail?id=749236 [2] https://github.com/uBlockOrigin/uAssets/issues/1370

(In reply to rhill@raymondhill.net from comment #31) > Chromium will also block uBO's redirects to data: URIs *if* a site declares > a CSP header which prevent the use of data: URIs. Really, that's not what I am observing when visiting http://vulnerabledoma.in/fx_csp_bypass_with_importScripts. I gotta setup some more test sites but it seems that they are blocking insecure redirects to data: URIs regardless of the CSP.

(In reply to rhill@raymondhill.net from comment #31) > Chromium will also block uBO's redirects to data: URIs *if* a site declares > a CSP header which prevent the use of data: URIs. This is actually an open > issue on Chromium's issue tracker[1]. Where do your scripts come from? If they're fixed could you use moz-extension: urls and bundle them? That would solve your Firefox issue _and_ your Chrome issue. I bet they come from dynamic filter lists though, so maybe not possible. Turn the data into blob: urls? That would evade the Firefox block. Might even work around your Chrome issue because I think they track blob: origin and might give a free-pass to ones they know were created in an extension context.

We should file a _different_ bug "uBlock Origin neutered scripts broken by preventing redirects to data: urls" or something rather than carry on this conversation here.

Please note that redirecting to a blob URI might still be blocked by CSP because blobs needs to be explicitly whitelisted within CSP.

Raymond, what do you think about comment 33? Could that work? FWIW, we are about to file a new bug to make sure uBlock Origin continues to work within Firefox.

Filed bug 1434357 for this breakage.

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #36) > Raymond, what do you think about comment 33? Could that work? > FWIW, we are about to file a new bug to make sure uBlock Origin continues to > work within Firefox. Clearing ni? on this bug, since the conversation continues within bug 1434357.