1361953 - Consider preventing Web Extensions interacting with privileged Firefox web URLs

Status: RESOLVED FIXED

(WebExtensions :: General, enhancement, P3)

Description

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

We have a number of URLs that are blessed with special APIs, often provided via webChannel. The pref webchannel.allowObject.urlWhitelist lists a few of these:

https://accounts.firefox.com
https://content.cdn.mozilla.net 
https://input.mozilla.org 
https://support.mozilla.org 
https://install.mozilla.org

A web extension could interact with these and potentially cause issues for the user. Accounts is probably the biggest risk. I haven't looked into the details of what psuedo-APIs are exposed, but it might be simplest to just restrict access.

Of course we may actually need Web Extensions to have access (password managers need to be able to remember your FxA password) so maybe its a case of just requiring an extra permission for privilegedURLs. 

At this stage im not sure if its something that is worth the effort, but given that we block web ext from interacting with other privilege pages (chrome: about:) then it makes sense to me to take a look at this.

(this is part of current web extensions security review, marking confidential until im sure it not an issue)

Comment 1

[Frederik Braun [:freddy]]

Besides the webchannel.allowObject.urlWhitelist, we should probably also check https://dxr.mozilla.org/mozilla-central/source/browser/app/permissions.

I suppose there must be something in PermissionManager that we can use to query for permissions and disallow webextension access based on this.

Comment 2

[Andy McKay]

As you mentioned, I think should be discussed on a case by case basis. For example, we block content scripts on https://addons.mozilla.org and https://testpilot.firefox.com to prevent access to mozAddonsManager but the rest eg: interacting with tabs, webRequest and so on is allowed.

It also seems odd to have input.mozilla.org in there since its especially dead now.

Comment 3

[Andy McKay]

I think install.mozilla.org, input.mozilla.org and support.mozilla.org (soon moving to Lithium) should be removed from this list. Ryan, do you know if FxA uses this?

Comment 4

[Ryan Kelly [:rfkelly]]

Yes, FxA still uses this privileged communication channel and will continue to do so for the forseeable future.

Comment 5

[Andy McKay]

Pinging Giorgos to check on SUMO. Is webchannel something you still need? I couldn't find a reference to it in kitsune.

Comment 6

[Andy McKay]

After reading through some bugs and the comment on that pref, it sounds like the list of URLs is to allow the sending of objects, instead of just strings through webChannels. 

Relating to that preference, I also chased down content.cdn.mozilla.net and that is related to the now demised content services team. So that domain can also be removed most likely. 

Does that mean webChannels everywhere else with strings are ok with WebExtensions?

Comment 7

[Ryan Kelly [:rfkelly]]

> Does that mean webChannels everywhere else with strings are ok with WebExtensions?

I don't think so; the string-vs-object distinction is only about guarding against certain classes of bug where we might leak information across the barrier between chrome and web content, but even string-valued webchannel messages can trigger privileged functionality that we might want to lock down from webextensions.

Comment 8

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

(In reply to Andy McKay [:andym] from comment #2)
> As you mentioned, I think should be discussed on a case by case basis. For
> example, we block content scripts on https://addons.mozilla.org and
> https://testpilot.firefox.com to prevent access to mozAddonsManager but the
> rest eg: interacting with tabs, webRequest and so on is allowed.

Do you know why we chose to block content scripts here? Was it just an vague attempt to stop extensions that do this ? (since its obviously by-passable with the things you mention?) Or were the other vectors (tabs.executeScript, webRequest etc) just not really considered?

Comment 9

[Giorgos Logiotatidis [:giorgos]]

Kitsune uses webchannel for the remote-troubleshooting functionality. [0] That said we're going to switch from Kitsune to Lithium again at some point. Lithium does not implement any of that functionality but maybe the team plans to do so in the future. I'm deferring to Patrick:

 - Patrick do you plan to add remote-troubleshooting functionality in Lithium?


[0] https://github.com/mozilla/kitsune/blob/ae1f5ff5b160d460f7c45005c1f6ec50b18ad241/kitsune/sumo/static/sumo/js/remote.js

Comment 10

[Andrew Swan [:aswan]]

(In reply to Paul Theriault [:pauljt] from comment #8)
> (In reply to Andy McKay [:andym] from comment #2)
> > As you mentioned, I think should be discussed on a case by case basis. For
> > example, we block content scripts on https://addons.mozilla.org and
> > https://testpilot.firefox.com to prevent access to mozAddonsManager but the
> > rest eg: interacting with tabs, webRequest and so on is allowed.
> 
> Do you know why we chose to block content scripts here? Was it just an vague
> attempt to stop extensions that do this ? (since its obviously by-passable
> with the things you mention?) Or were the other vectors (tabs.executeScript,
> webRequest etc) just not really considered?

I wouldn't describe it as vague, the intention is to prevent extensions from getting backdoor access to mozAddonManager.
Blocking content scripts includes declarative scripts as well as tabs.executeScript()
And extensions can use webRequest to monitor requests to those pages but they may not modify the requests (see bug 1308688)

Comment 11

[Andy McKay]

See also bug 1295324.

Comment 12

[Patrick McClard;pmcclard]

(In reply to Giorgos Logiotatidis [:giorgos] from comment #9)
> Kitsune uses webchannel for the remote-troubleshooting functionality. [0]
> That said we're going to switch from Kitsune to Lithium again at some point.
> Lithium does not implement any of that functionality but maybe the team
> plans to do so in the future. I'm deferring to Patrick:
> 
>  - Patrick do you plan to add remote-troubleshooting functionality in
> Lithium?
> 
> 
> [0]
> https://github.com/mozilla/kitsune/blob/
> ae1f5ff5b160d460f7c45005c1f6ec50b18ad241/kitsune/sumo/static/sumo/js/remote.
> js

I looked at the code, but not sure what on Kitsune it was kicking off. Is this what triggered a Firefox Refresh?

I'd like to replicate that functionality on Lithium, but if this is too much of a security risk we can fallback to our documentation.

Comment 13

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

(In reply to Andrew Swan [:aswan] from comment #10)
> (In reply to Paul Theriault [:pauljt] from comment #8)
> > (In reply to Andy McKay [:andym] from comment #2)
> > > As you mentioned, I think should be discussed on a case by case basis. For
> > > example, we block content scripts on https://addons.mozilla.org and
> > > https://testpilot.firefox.com to prevent access to mozAddonsManager but the
> > > rest eg: interacting with tabs, webRequest and so on is allowed.
> > 
> > Do you know why we chose to block content scripts here? Was it just an vague
> > attempt to stop extensions that do this ? (since its obviously by-passable
> > with the things you mention?) Or were the other vectors (tabs.executeScript,
> > webRequest etc) just not really considered?
> 
> I wouldn't describe it as vague, the intention is to prevent extensions from
> getting backdoor access to mozAddonManager.
> Blocking content scripts includes declarative scripts as well as
> tabs.executeScript()
> And extensions can use webRequest to monitor requests to those pages but
> they may not modify the requests (see bug 1308688)

Ok that makes more sense - Andy's comment made it seem we only block content scripts but not the other methods. So if Im following the comments above correctly, it sounds like we prevent scripts being injected if the page has a window.navigator.mozAddonManager property defined [1], but we don't currently have any restrictions on a per-host basis. 

And we also prevent "blocking" webrequests to a blacklist of domains, specified in code [1].

I think we need to do _something_ about account.mozilla.com, and support.mozilla.com (depending on outcome of discussion above). 

Any thoughts on this? I had thought limit host permissions might be the obvious approach, but interested to hear thoughts on that?


[1] http://searchfox.org/mozilla-central/source/toolkit/components/extensions/extension-process-script.js#159
[2] http://searchfox.org/mozilla-central/source/toolkit/mozapps/extensions/AddonManagerWebAPI.cpp#23 


[1] http://searchfox.org/mozilla-central/source/toolkit/components/extensions/extension-process-script.js#159

Comment 14

[Giorgos Logiotatidis [:giorgos]]

(In reply to Giorgos Logiotatidis [:giorgos] from comment #9)
> Kitsune uses webchannel for the remote-troubleshooting functionality. [0]
> That said we're going to switch from Kitsune to Lithium again at some point.
> Lithium does not implement any of that functionality but maybe the team
> plans to do so in the future. I'm deferring to Patrick:
> 
>  - Patrick do you plan to add remote-troubleshooting functionality in
> Lithium?
> 

remote-troubleshooting is used to collect client data from about:support when you're asking questions in Kitsune. 

Patrick, if that's something you want to replicate in Lithium we need to keep support.mozilla.org whitelisted.

Comment 15

[Patrick McClard;pmcclard]

It is a feature that our contributors are requesting we replicate on Lithium. We should keep it whitelisted.

Comment 16

[Frederik Braun [:freddy]]

is this fixed by bug 1295324?
I see the latest comments get a bit side-tracked by discussing the list of privileged mozilla domains in itself.

Comment 17

[:Gijs (he/him)]

(In reply to Frederik Braun [:freddyb] from comment #16)
> is this fixed by bug 1295324?
> I see the latest comments get a bit side-tracked by discussing the list of
> privileged mozilla domains in itself.

For one, some of the checks seem to have been moved/replaced ( in https://hg.mozilla.org/mozilla-central/rev/1267d47eca93 ) and for another, I wonder if we need to do the same thing for other domains (mozAddonManager isn't the only relevant thing here that has added APIs - UITour and the webchannel stuff from comment #0 all seem relevant, too). Shane reviewed the changeset I referenced, so perhaps he knows where these checks live and if the list of things they block on is exhaustive?

Comment 18

[Andrew Swan [:aswan]]

Answerinf for Shane:
Stuff moved around but we still just explicitly test for the domains where mozAddonManager is injected:
http://searchfox.org/mozilla-central/rev/298033405057ca7aa5099153797467eceeaa08b5/toolkit/components/extensions/WebExtensionPolicy.cpp#380-382

Comment 19

[:Gijs (he/him)]

(In reply to Andrew Swan [:aswan] from comment #18)
> Answerinf for Shane:
> Stuff moved around but we still just explicitly test for the domains where
> mozAddonManager is injected:
> http://searchfox.org/mozilla-central/rev/
> 298033405057ca7aa5099153797467eceeaa08b5/toolkit/components/extensions/
> WebExtensionPolicy.cpp#380-382

OK. If that's the only thing, I don't think that is sufficient for the sites from comment #0, among other things.

Comment 20

[Shane Caraveo (:mixedpuppy)]

If we need something more exhaustive, then we'll need to add that, probably in WebExtensionContentScript::Matches.  It would be nice if there were a centralized api for this, a flag on the document, or something.  Discovery of what sites get extra APIs (and how those are handled) feels a little ad hoc.

Comment 21

[Andrew Swan [:aswan]]

(In reply to Shane Caraveo (:mixedpuppy) from comment #20)
> If we need something more exhaustive, then we'll need to add that, probably
> in WebExtensionContentScript::Matches.  It would be nice if there were a
> centralized api for this, a flag on the document, or something.  Discovery
> of what sites get extra APIs (and how those are handled) feels a little ad
> hoc.

How centralized are you imagining this?  mozAddonManager, webchannels, and UITour each use different mechanisms but each of those individual methods are reasonably well abstracted (mozAddonManager is the code linked in comment 18, webchannels uses the pref mentioned in comment 0, and UITour uses nsIPermissionManager::testPermission)

Comment 22

[Ryan Kelly [:rfkelly]]

> webchannels uses the pref mentioned in comment 0

Note that IIRC "webchannel.allowObject.urlWhitelist" is not a comprehensive list of all sites that can send webchannel messages to the browser; it's just a list of sites that are allowed to send non-string values across that interface, for backwards-compat reasons.

Comment 23

[Kris Maglione [:kmag]]

These sites are all currently in the restricted domain list added in bug 1415644. We should probably automatically restrict sites based on the availability of this API, but we have other priorities at the moment.

Comment 24

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

I agree - lets call this fixed, since 1415644 was basically my original intention when filing this bug. I'll file a follow-up to sync the availability of WebChannel to the extensions.webextensions.restrictedDomains list.