1205886 - Cross-origin access for content scripts isn't working as expected

Status: VERIFIED FIXED

(WebExtensions :: Untriaged, defect, P1)

Description

[Will Bamberg [:wbamberg]]

Attachment: co.xpi

According to the Chrome docs, I think, content scripts should get the same cross-origin privileges that their parent extension has.

I've attached a WebExtension.

It adds a button. When you're on a page under https://developer.mozilla.org/, if you click the button, it injects a content script. The content script fetches content from https://example.com/ and replaces the page with the fetched content.

The WebExtension has this in its manifest.json:

  "permissions": ["https://developer.mozilla.org/*", "https://example.com/"],

With these permissions, I think it ought to work, and it does on Chrome. But on Firefox it doesn't, and I get a "Cross-Origin Request Blocked" error in the console:

Comment 1

[Kris Maglione [:kmag]]

This should probably apply to cross-compartment DOM access too. It looks like Chrome doesn't support that, though, so this is simpler than I thought.

It seems we already have the cross-origin security checks required for this (bug 1180921), so this is just a matter of giving content scripts an XMLHttpRequest that's bound to their principal, rather than the principal of the content document.

Provisionally taking this, since it looks pretty doable now.

Comment 2

[Kris Maglione [:kmag]]

Actually, I take some of that back. We currently run content scripts in a sandbox with the principal of the content window, which makes this a bit hairier.

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

We actually designed for this to work, but I forgot to implement one piece of it. We need to make sure that the expanded principal that the sandbox gets has a principal with the addonId OriginAttribute set. Bug 1182357 adds a facility to create such a principal. We just need to use it in ExtensionContent.jsm when creating our sandbox.

Still not sure this will all work as expected though :-).

Comment 4

[Kris Maglione [:kmag]]

Oh, good, that saves me the trouble of suggesting it. In that case, I guess someone should file a bug for finishing adding expanded principals to content scripts (I'm probably not the best person) that blocks this.

Once the scripts have the proper extension principal, we can just inject XMLHttpRequest bindings from a compartment bound to that principal without resorting to any major hacks.

Comment 5

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: patch

I started working on that whiteListedHosts bug I mentioned in the review yesterday and it kinda snowballed into this.

Comment 6

[Kris Maglione [:kmag]]

Comment on attachment 8695102 [details] [diff] [review]
patch

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

::: toolkit/components/extensions/ExtensionContent.jsm
@@ +222,2 @@
>      // Make sure we don't hand out the system principal by accident.
>      prin = Cc["@mozilla.org/nullprincipal;1"].createInstance(Ci.nsIPrincipal);

At some point, I think we're going to need to do a check for other kinds of privileged principals, too. Chrome doesn't let add-ons overlay other add-ons without special permissions (there are also privilege escalation issues there), for instance.

@@ +222,5 @@
>      // Make sure we don't hand out the system principal by accident.
>      prin = Cc["@mozilla.org/nullprincipal;1"].createInstance(Ci.nsIPrincipal);
> +  } else {
> +    let extensionPrincipal = ssm.createCodebasePrincipal(this.extension.baseURI, {addonId: extensionId});
> +    prin = ssm.createExpandedPrincipal([contentPrincipal, extensionPrincipal]);

Is there a reason we need to create the expanded principal explicitly rather than passing the array to |Cu.Sandbox|? The sandbox code also checks for system principals when creating expanded principals, so that might let us skip extracting the content principal and the security checks, too.

Comment 7

[Kris Maglione [:kmag]]

Actually, on second thought, can you add a test to check that scripts in the content page don't wind up with elevated privileges, and can't access the sandbox's XMLHttpRequest constructor if it somehow gets leaked to them?

I know it's probably a bit paranoid, but that would be a massive security hole if it ever happened.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4a967e08dff7

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4a967e08dff7

Comment 11

[Vasilica Tamas, QA [:vtamas]]

I was able to reproduce the initial issue on Firefox 43.0a1 (2015-08-22) under Windows 10 64-bit.

Verified fixed on Firefox 48.0a1 (2016-03-22) and Firefox 46.0b4 (20160322075646) using Windows 10 64-bit, Mac OS X 10.11 and Ubuntu 12.04 32-bit.