Closed Bug 1205886 Opened 6 years ago Closed 5 years ago

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


(WebExtensions :: Untriaged, defect, P1)



(firefox46 verified, firefox48 verified)

Tracking Status
firefox46 --- verified
firefox48 --- verified


(Reporter: wbamberg, Assigned: billm)


(Blocks 1 open bug)



(2 files)

Attached file 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, if you click the button, it injects a content script. The content script fetches content from and replaces the page with the fetched content.

The WebExtension has this in its manifest.json:

  "permissions": ["*", ""],

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:
Blocks: webext
Priority: -- → P1
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.
Assignee: nobody → kmaglione+bmo
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.
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 :-).
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.
Flags: blocking-webextensions+
Attached patch patchSplinter Review
I started working on that whiteListedHosts bug I mentioned in the review yesterday and it kinda snowballed into this.
Assignee: kmaglione+bmo → wmccloskey
Attachment #8695102 - Flags: review?(kmaglione+bmo)
Comment on attachment 8695102 [details] [diff] [review]

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[";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[";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.
Attachment #8695102 - Flags: review?(kmaglione+bmo) → review+
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.
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Duplicate of this bug: 1235543
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.
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.