1470310 - User Scripts API Sec Review

Status: RESOLVED FIXED

(Firefox Graveyard :: Security: Review Requests, enhancement)

Description

[Paul Theriault [:pauljt] (no longer reading bugmail)]

We need to review a new API called the User Scripts API. See a description of this API here: https://wiki.mozilla.org/WebExtensions/UserScripts

Implementation is in bug 1332273

Off the top of my head, the main risk I can think of is that we accidentally grant privileges to UserScripts which are meant for some other context (like if we accidentally grant access to Web Extension API). 

That said, as far as I can see, extensions can grant access to any of the Web Extension APIs simply by proxying calls. As far as I can see[1] there hasn't been any consideration yet given to the security implications here for this API. So we should probably do that to...


[1] https://wiki.mozilla.org/WebExtensions/UserScripts#Concerns

Comment 1

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Taking this back and it doesn't need to be confidential. Also I see that two key bugs have landed so we can maybe test this now. 

Luca, once 1437861, 1437864 l and, are we good to do a security test of this? Or are there more pieces that need land?

Comment 2

[Luca Greco [:rpl] [:luca] [:lgreco]]

Yes, once Bug 1437861 and Bug 1437864 land, the userScripts API is good for its initial security testing.
Both are already reviewed, but I'm going to land them on 64 (as soon as 63 goes to beta).

There is also the patch from Bug 1470466 (which is also already reviewed), but it only changes the telemetry histogram, and so it shouldn't require an additional security testing on its own.

Of the dependencies bugs already filed, Bug 1437867 is another one that is going to require an additional security assessment and security test (but it doesn't have a patch yet, it is basically still open for discussion, especially in terms of which kind of sandbox configurations we can allow to the extensions, Bug 1437867 comment 1 contains some additional details about the kind of sandbox configuration options that Greasemonkey is going to need).

Comment 3

[Paul Theriault [:pauljt] (no longer reading bugmail)]

While we are testing this, we should also test the current state of CSP. See comment https://bugzilla.mozilla.org/show_bug.cgi?id=1438177#c2

Comment 4

[Luca Greco [:rpl] [:luca] [:lgreco]]

Hi Freddy,
is the Risk assessment for the userScript APIs (in the form they landed in 64) completed?

Some of the new userScripts issue that we are working on as part of Nightly 65 (e.g. Bug 1498739) are likely going to need an additional explicit review from a security point of view, but we are planning to flag them for sec-review individually, and so I'm wondering if this bug can be closed.

Comment 5

[Frederik Braun [:freddy]]

Yes, the testing is done.
Let's file follow-ups review bugs for the upcoming work (or just use the sec-review? / feedback? flags for minor things)