Closed Bug 1353468 Opened 7 years ago Closed 6 years ago

Allow WebExtensions to construct a Cu.Sandbox

Categories

(WebExtensions :: General, defect, P2)

52 Branch
defect

Tracking

(firefox57 affected)

RESOLVED DUPLICATE of bug 1437098
Tracking Status
firefox57 --- affected

People

(Reporter: robwu, Assigned: rpl)

References

(Blocks 1 open bug)

Details

(Whiteboard: [outreach][awe:{e4a8a97b-f2ed-450b-b12d-ee082ba24781}] triaged)

User script managers need to run untrusted code in an isolated context, which
1) protects the script from tampering by the page and
2) protects other scripts from malicious scripts.

I looked at Tampermonkey for Chrome (also a user script manager), and it runs all user scripts in the context of the page, while exposing semi-privileged methods such as GM_xmlhttpRequest to the script. Creating such a sandbox takes lots of efforts (not just development time, but also runtime) and is not guaranteed to be secure.

Since we have Cu.Sandbox in Firefox [1], we should expose this to WebExtensions to allow them to run untrusted scripts.

[1] https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Language_Bindings/Components.utils.Sandbox
adding details for wont fix
Flags: needinfo?(kmaglione+bmo)
At some point in the future there may be JS Realms, which are quite similar to sandboxes. So maybe a shim around sandboxes that models things after realm APIs could be used? Although one thing is missing from realms: control over xrays.

see https://github.com/tc39/proposal-realms and bug 962053
today greasemonkey uses two modes of sandbox operation

a) |@grant none| mode[0]. This is a sandbox with the same principal as the page content and with xrays off.
b) privileged API mode[1]. This is a sandbox with an extended principal that only contains the content principal and with xrays 
on.

And to actually use the sandbox it obviously needs a way to load scripts into it[2]. That's all fairly similar to what webextensions are doing but there also are some differences that make implementing userscripts on top of webextensions less secure than they are in GM. 


[0] https://github.com/greasemonkey/greasemonkey/blob/3.10/modules/sandbox.js#L32-L39
[1] https://github.com/greasemonkey/greasemonkey/blob/3.10/modules/sandbox.js#L50-L58
[2] https://github.com/greasemonkey/greasemonkey/blob/3.10/modules/sandbox.js#L190
Flags: needinfo?(sescalante)
Kris will add details about the needed api
Flags: needinfo?(sescalante)
Whiteboard: [outreach] triaged
Whiteboard: [outreach] triaged → [outreach][awe:{e4a8a97b-f2ed-450b-b12d-ee082ba24781}] triaged
Andy is going to follow up on this one when there is an opening
Flags: needinfo?(amckay)
Assignee: nobody → mixedpuppy
Flags: needinfo?(amckay)
FWIW I want this (for Greasemonkey) quite desperately.  I'd want to:

* Be able to construct a sandbox, like Cu.sandbox from XPCOM.
* Be able to exportFunction() to it.
** With some kind of message passing -- ideally, directly to my background script.
* Be able to control the principal -- sometimes pure page scope (effectively just an isolated namespace, in which I can run JS regardless of content security policy), sometimes wrapped/extended principal (of page) scope.

> I looked at Tampermonkey for Chrome (also a user script manager), and it runs
> all user scripts in the context of the page, while exposing semi-privileged
> methods such as GM_xmlhttpRequest to the script. 

My webext port of Greasemonkey is getting semi functional.  I'm discovering that (as best I can tell, I have two options):

1) Run as a "content script" -- but this leaks (at least) the <all_urls> privilege, which I must have to run the script, which gives cross-domain fetching to everything I run.
2) Run as a page script (window.eval() or <script> injection) -- but this means I'm vulnerable to blockage by content security policy, and I can't expose privileged methods to my script and not the whole page.

Both of which are crummy options.
See Also: → 1323433
I think creating a new API is a bad idea. WebExtensions are supposed to be optimized for security themselves, which means any way to run arbitrary user/remote code, should just be sandboxed by default: eval(), injecting <script>s, ...
They would still result in inferior performance. GM can load scripts straight from blob/file URIs, which means the memory can be shared and script caches can work. Using eval needs lots of string copying. And injecting API functions in eval means string processing or using |with| hackery which is also bad for performance.

I don't think eval is a good fit for such uses. Not without a Realm API + loaders, but those seem to be far off into the future.

One possible alternative, once we have declarative content script APIs, might be an additional flag that lets each declared content script run in a separate sandbox so that they don't interfere with each other.
(In reply to The 8472 from comment #8)
> They would still result in inferior performance. GM can load scripts
> straight from blob/file URIs, which means the memory can be shared and
> script caches can work. Using eval needs lots of string copying. And
> injecting API functions in eval means string processing or using |with|
> hackery which is also bad for performance.
> 
> I don't think eval is a good fit for such uses. Not without a Realm API +
> loaders, but those seem to be far off into the future.
> 
> One possible alternative, once we have declarative content script APIs,
> might be an additional flag that lets each declared content script run in a
> separate sandbox so that they don't interfere with each other.

My point here is that a separate API shouldn't exist for this, this should either be part of a content script API (by default or as an option if needed), but not a new API.
Well your specific suggestions of somehow making it work with eval or <script> seemed farfetched to me. So if you want to quash a feature request at least try to propose realistic alternatives.

If it's supposed to be part of the content script API then we'd still need several things

- declarative content scripts, bug 1323433
- reduced permissions for content scripts, bug 1391669
- an option to isolate multiple content scripts from each other. equivalent to new sandbox with xrays
- an option to turn off xray wrappers for isolated and permission-reduced content scripts. equivalent to new sandbox without xrays.

That seems like a lot of work. Just adding a Cu.Sandbox + script loader wrapper as low-level building blocks would seem a lot simpler and easier to implement. I think the latter could be done with a few LoCs in a single module. The above list takes more work and is less powerful.
<script> and eval also doesn't give you a mechanism to expose privileged APIs to the "sandboxed" code, without potentially exposing those APIs to webpage scripts. I think we should have a sandbox as an "unstable" API, because designing a good API for this might be hard. Also XPConnect sandboxes are kind of weird from a JS engine perspective, so I don't want to box us into a corner here.
Looking through the webextension internals I see that they don't use nsISubscriptLoader but |ChromeUtils.compileScript|. What are the tradeoffs between those?

> I think we should have a sandbox as an "unstable" API

Maybe provide it as a promise that only gets resolved after the content script relying on that API has been evaluated but before content parsing continues so that it can be used at |document_start|? That small bit of asynchronicity should force a coding style that's a bit less fragile than just using it without feature-testing it.
See Also: → 1332273
Priority: -- → P2
non-exhaustive list of security issues without sandboxes:

userscripts being eval'd in the content script scope have access to:

- content script APIs such as browser.storage of the userscript extension
- they share a global object
- they share xray wrappers
- they have access to <all_urls> XHR and fetch APIs even though userscripts do not always have that kind of permission
- they have access to the moz-extension://{uuid} principal of the extension, which they might be able to exploit further through <iframes>

Since userscripts are unreviewed  code installed from 3rd party sites and executed in a privileged context this pretty looks like a violation of the addon validation rules. The alternative approach is executing them in the page context and exposing privileged APIs (such as XHR) to that context. Which would also appear to be a violation of the rules.
So right now it's technically not possible to write a compliant addon. Short of attempting to implement a javascript sandbox.
See https://github.com/greasemonkey/greasemonkey/issues/2564

In addition to all the above: we'd really like error reporting to "work".  In the past I've tried to //#sourceMappingURL with tabs.executeScript() which failed to work.  If we get some kind of .exec() on the sandbox, we can just //#sourceURL, and the primary gain there would be getting logs/errors to actually show where they came from.  Also, all logs/errors should show up in the right tab's console.

Extra bonus points: the link should point to something that actually works.  In GM3, we pointed to the file on disk -- so we had the right name for free, and clicking the link worked.
(In reply to Anthony Lieuallen from comment #15)
> See https://github.com/greasemonkey/greasemonkey/issues/2564
> 
> In addition to all the above: we'd really like error reporting to "work". 
> In the past I've tried to //#sourceMappingURL with tabs.executeScript()
> which failed to work.  If we get some kind of .exec() on the sandbox, we can
> just //#sourceURL, and the primary gain there would be getting logs/errors
> to actually show where they came from.  Also, all logs/errors should show up
> in the right tab's console.
> 
> Extra bonus points: the link should point to something that actually works. 
> In GM3, we pointed to the file on disk -- so we had the right name for free,
> and clicking the link worked.

You should file a bug in the Developer Tools: Console category.
I don't understand comment #16 .  What bug should I file, what should it say?

The comments I made above apply specifically to the sort of control that would be available for executing code in the sandbox that doesn't exist except for this bug.  I'm saying: if/when we get some kind of sandbox, surely we'll get some matching sort of eval/exec (acting within that sandbox).  Please make sure it supports specifying a sourceURL.
https://groups.google.com/forum/#!topic/greasemonkey-users/wIVnuHSLBuU

Assuming this feature is added, how/can users debug the scripts run in such a sandbox?

Also: Is progress being made here?  Greasemonkey has several deficiencies which I'm hoping to fix by relying on this feature.
(In reply to Anthony Lieuallen from comment #18)
> Assuming this feature is added, how/can users debug the scripts run in such
> a sandbox?

Hopefully a webextension-safe wrapper around Cu.Sandbox and the script compiler/loader would contain all the necessary boilerplate to wire up debugging information, associating it with the right console, setting the right GC zone etc.
Greasemonkey is still (desperately) waiting for this.  Is there any progress or estimate to report?
Flags: needinfo?(amckay)
We wrote up a plan for this here: https://wiki.mozilla.org/WebExtensions/UserScripts
Flags: needinfo?(amckay)
Plan linked in comment #21 looks like it could be made to work.  Though I've got to guess at a few implied details.  It's unclear exactly what form `apiOptions` takes, and thus exactly what it does.  Where `metadata` goes exactly?  What `registerAPI` is (part of the page says not in content scripts, but then "example apiContentScript.js" uses it?)?
Oh, and: how/can I control whether there are xrays/wrappers between the script and the page?  One of Greasemonkey's biggest breakages now (legacy->webext feature set) is that it does not support a no-wrapper mode, which ideally would come with a distinct (global) namespace from the content page.
Hi Anthony, first of all, I'm sorry for the unclear details, we are still discussing
part of the API design and so the wiki page isn't fully finalized yet.

Having said that, your feedback in these comments is very useful!

I've just applied one more round of changes to the wikipage, based on the last discussions/brainstorming on the topic we had yesterday.

Follow some additional info related to your questions in comment 22 and comment 23:

(In reply to Anthony Lieuallen from comment #22)
> Plan linked in comment #21 looks like it could be made to work.  Though I've
> got to guess at a few implied details.  It's unclear exactly what form
> `apiOptions` takes, and thus exactly what it does.  

`apiOptions` was a "placeholder" for the options shared with the contentScripts.register API method
(and so it was basically a placeholder for the following ones: 
matches/excludeMatches, includeGlobs/excludeGlobs, runAt, matchAboutBlank, allFrames).

In the updated version of the wiki page we have now replaced `apiOptions` with the list of the actual options.

> Where `metadata` goes exactly?  

`metadata` is meant to be an object opaque to Firefox, it just needs to be "serializable",
so that it can be send across the processes (and possibly also to be pretty small).

The idea is that this object will be populated by the extension with the metadata related to the userScript,
so that it can be passed to the custom APIs implementations as an additional parameter, and then these
custom API methods implementation (provided by the extension) may use it to check the metadata of the caller userScript, 
e.g. Greasemonkey may use it to store the userScript name and/or the array of the granted APIs.

> What `registerAPI` is (part of the page says not in content scripts, but then "example apiContentScript.js" uses it?)?

This method (in the last round of updates on the wiki page I've renamed it to `setAPIScript`) is meant to allow the extension to specify the extension url for a content script to be executed automatically when one or more of the registered userScripts matched a webpage, so that it can provide the custom APIs to inject into the userScripts sandboxes.

(In reply to Anthony Lieuallen from comment #23)
> Oh, and: how/can I control whether there are xrays/wrappers between the
> script and the page?  One of Greasemonkey's biggest breakages now
> (legacy->webext feature set) is that it does not support a no-wrapper mode,
> which ideally would come with a distinct (global) namespace from the content
> page.

The idea is that userScripts.register would accept an additional parameter (e.g. one named sandboxOptions) to be used to customize the created sandbox (detailing the exact kinds of customization that we can allow on these sandboxes will need further evaluations, especially one from a security point of view, in the meantime we've explicitly included some notes about this in the last round of updates on the wiki page).
Assignee: mixedpuppy → lgreco
This sounds great.  Don't hesitate to ask me for any further detail that would help you make decisions/understand our needs.  I'd love to experiment with whatever like this is available ASAP.
Marking as a duplicate of the userScripts API tracking issue ('cause it is how we agreed that we are going to allow scripts that needs to access webpages to be executed in their own isolated sandbox)
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
Clearing an old needinfo.
Flags: needinfo?(kmaglione+bmo)
Does the userScripts API also cover the case where the developer wants to run code fetched from a background script, that is not specific to a web page? If not, is this something we will be able to support? I've come across this in the one or other add-on.
Flags: needinfo?(lgreco)
Hi Philipp,
sorry for the late response, I didn't notice this needinfo.

The userScripts API does not cover the use case of "running third party js code in a sandboxed environment" from a background script (or any other extension page), mostly because the userScripts API has been designed to take into account the oop architecture (the userScript will be registered from the extension process, but then it is going to run in the content process) and the particular "life cycle" of a "content script"/"userScript" (that is tied to the webpage that the userScript is hooked to).

I would not exclude that covering the "running third party js code in a sandboxed environment" use case may also be useful to some extension, we may need to detail the use cases that it should cover in a bit more detail, so that we can draft a "API design proposal" to discuss/evaluate.
Flags: needinfo?(lgreco)
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.