1432137 - Add test to verify insecure redirects to data: URIs are blocked for script modules

Status: RESOLVED FIXED

(Core :: DOM: Security, enhancement, P3)

Description

[Christoph Kerschbaumer [:ckerschb]]

This is a follow up Bug for from Bug 1428793 to add a mochitest that makes sure insecure redirects to data: URIs are blocked for script modules.

Comment 1

[Christoph Kerschbaumer [:ckerschb]]

Ben, I saw you reviewed Bug 1330657. It seems I can't find the bug to enable script modules in release. Is there already one? If so, could you please mark this bug blocking the 'enable script modules in release' bug? Thanks!

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

Jon, can you make this bug block shipping modules?  Thanks!

Comment 3

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1432137_verify_script_module_redirects_data_blocked.patch

Hey Jon, within this Bug I am trying to add a test to make sure script modules that are insecurely redirected to a data: URI are blocked. It seems my test for script modules is not working properly. Hence, I already asked that question within Bug 1428793 [see 1] but didn't get a clear answer.

Anyway, in the attached testcase I always receive an .onerror() event for testModuleScriptRedirectToData even when the redirect is not blocked. I have to admit that I don't fully understand how script modules work hence I was wondering if you could provide some guidance. I started by explicitly flipping the pref dom.moduleScripts.enabled to true, but not matter what I try I always receive an .onerror() event. Any help is highly appreciated. Thanks!

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1428793#c18

Comment 4

[Jon Coppeard (:jonco)]

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #3)
One thing I noticed is that MIME type specified won't work: modules require a JS MIME type (e.g. application/javascript).

That's probably not the problem here though.  I think the issue is modules are always fetched with CORS enabled and so will fail to load a data URI since that's not same-origin with the page loading it.

Comment 5

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1432137_verify_script_module_redirects_data_blocked.patch

Jon, thanks for your help. In fact the 'regular' loading without any redirect requires a type of 'application/javascript', without that the load fails. For verification purposes I changed the *.sjs to include:

>  if (query === "modulescript") {
>    response.setHeader("Content-Type", "application/javascript", false);
>    response.write("var a = 0;");
>    return;
>  }

which verifies that your assessment was correct.

Using a data: URI fails the load, but I think it makes sense to land that test anyway. Similar to what we do in the worker case above.

What do you think? Agreed?

Comment 6

[Jon Coppeard (:jonco)]

Comment on attachment 8944689 [details] [diff] [review]
bug_1432137_verify_script_module_redirects_data_blocked.patch

Review of attachment 8944689 [details] [diff] [review]:
-----------------------------------------------------------------

I agree we should add the test.

I think you should make the MIME type for the data URLs 'text/javascript' in all cases though.

Comment 7

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Jon Coppeard (:jonco) from comment #6)
> I think you should make the MIME type for the data URLs 'text/javascript' in
> all cases though.

Agreed - thanks!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a4be09ecf97a
Add test to verify insecure redirects to data: URIs are blocked for script modules. r=jonco

Comment 9

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/a4be09ecf97a