1371551 - Relative imports in WebExtensions use wrong url and cause security error

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: Testcase - v1

In a WebExtension, I have a background page that loads a <script type="module">. This works fine (pref is enabled). Now when importing a module in that script using:

import { check } from "./imported.js"

I get a security error because it is trying to load from a file:// url instead of a moz-extension:// url.

Security Error: Content at moz-extension://baf998c3-e9d6-a242-bd5c-afeac3192105/background/background.html may not load or link to file:///Users/kewisch/mozilla/extensions/test-es6-module/background/imported.js.

See attached extension for a test case.

Comment 1

[Tom S [:evilpie]]

Attachment: Use channel originalURI instead of URI

I am not totally sure if this correct, but it works. Instead of using file: we want to use the original moz-extension: protocol.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Could you explain the patch?

Comment 3

[Tom S [:evilpie]]

I don't have any great explanation, but basically see https://searchfox.org/mozilla-central/source/netwerk/base/nsIChannel.idl#36.

> This is used in
>     * the case of a redirect or URI "resolution" (e.g. resolving a
>     * resource: URI to a file: URI) so that the original pre-redirect
>     * URI can still be obtained.  This is never null.  Attempts to
>     * set it to null must throw.

In this case nsIChannel.URI is "file:///Users/kewisch/mozilla/extensions/test-es6-module/background/imported.js", because that is how we resolved "moz-extension://baf998c3-e9d6-a242-bd5c-afeac3192105/background/background.html". Extension however may not load file:// URIs directly, and must go through moz-extension://. originalURI is still ""moz-extension://baf998c3-e9d6-a242-bd5c-afeac3192105/background/background.html", i.e. the URI before we resolved it to actual "file" URI where the extensions' file is stored.

Comment 4

[Tom S [:evilpie]]

I am not sure if the "redirect" case would be an issue here.

Comment 5

[Tom S [:evilpie]]

Maybe the safest option would be so something like:

if (!originalURI.SchemeIs("moz-extension"))
  channel->GetURI(getter_AddRefs(request->mBaseURL));

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8937437 [details] [diff] [review]
Use channel originalURI instead of URI

I think jonco should review this first.
Could someone point to the relevant spec defining what the base uri should be?
Is channel's uri the right, or should it be script element's document's base uri or something?

Comment 7

[Jon Coppeard (:jonco)]

Comment on attachment 8937437 [details] [diff] [review]
Use channel originalURI instead of URI

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

The base URL is set to the response URL, so it's like this intentionally.  The relevant part of the spec is:

https://html.spec.whatwg.org/#fetching-scripts:concept-script-base-url

Comment 8

[Tom S [:evilpie]]

Attachment: Use channel originalURI instead of URI only for webextensions

Thanks for linking the relevant spec. So what about this more targeted fix for WebExtensions, I don't think that should break anything else?

Comment 9

[Jon Coppeard (:jonco)]

Comment on attachment 8937757 [details] [diff] [review]
Use channel originalURI instead of URI only for webextensions

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

Fine by me.

Comment 11

[Andrea Marchesini [:baku]]

Comment on attachment 8937757 [details] [diff] [review]
Use channel originalURI instead of URI only for webextensions

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

::: dom/script/ScriptLoader.cpp
@@ +3048,5 @@
>        return NS_ERROR_FAILURE;
>      }
>  
> +    nsCOMPtr<nsIURI> uri;
> +    channel->GetOriginalURI(getter_AddRefs(uri));

in theory, this can return an error. just do:

rv = channel->GetOriginalURI(getter_AddRefs(uri));
NS_ENSURE_SUCCESS(rv, rv);

@@ +3052,5 @@
> +    channel->GetOriginalURI(getter_AddRefs(uri));
> +
> +    // Fixup moz-extension URIs, because the channel URI points to file:,
> +    // which won't be allowed to load.
> +    bool isWebExt;

isWebExt = false;

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/73c3034830d9
Make ES6 modules work for webextension URLs. r=baku

Comment 13

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/73c3034830d9