1128126 - (CVE-2015-0812) Addon permissions exposed to man-in-the-middle attacks

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Armin Ebert [:arminius]]

Attachment: test case which installs a theme on page load

Firefox exposes Addon management permissions to a man in the middle, possibly
due to broken Public Key Pinning.

Extended browser access is granted to some Mozilla-owned domains, such as addon
management for addons.mozilla.org, wich is defined by the default_permissions
file, as I understood it.

But unlike the "UITour" permissions granted for www.mozilla.org, this API is
not restricted to the https:// protocol.

This should not be a security risk per se, since addons.mozilla.org provides
HSTS headers and is included in the static HPKP pinning list anyways, which
should render any attempt to tamper with plain HTTP traffic impossible.

Nevertheless sub-subdomains do not seem to enforce SSL here.

So a MITM can spoof the DNS entry for http://evil.addons.mozilla.org/ and from
there still act with the same privileges as https://addons.mozilla.org/.


Reproduction:

  - Set up a local web server together with a host entry like:
    127.0.0.1   evil.addons.mozilla.org

  - Perform operations, which require the access level of the AMO domain.
  - You can load the attached test case as a proof of concept, which installs a
    harmless theme.

  Observation:
  - http://localhost/install_theme.html
    -> Warning message: "This site (localhost) attempted to install a theme."

  - http://evil.addons.mozilla.org/install_theme.html
    -> The theme will be installed on page load without any user confirmation.

Comment 1

[:Gijs (he/him)]

Blair, is there any reason not to force the add-on manager itself (XPIProvider.jsm and wherever the LWT isInstallAllowed method is hiding) to do checks for https, considering that bug 891281 means the permissions manager doesn't?

Comment 2

[Blair McBride [:Unfocused] (UNAVAILABLE)]

AFAIK, the only reason not to enforce that are historical - changing it may break existing legitimate 3rd party install sites. And specialized cases like enterprise environments, I guess - but I don't have any data to back that up (and we can work around that anyway).

We do perform a reasonably strict HTTPS check in the install phase, but we allow a bypass using a hash check. That's designed for a MitM of the actual install file, not the install request. If a MitM can control the install request site, then it would likely be pointing at a file on a site it controls anyway - so our checks in the install phase won't help.

I think the benefits outweigh the costs of what we may break.

Saying that, needinfo on Dave in case he has more historical context around this. In the mean time, I'll work up a patch.

Comment 3

[Dave Townsend [:mossop]]

I can't think of any reasons.

Comment 4

[Daniel Veditz [:dveditz]]

I assume this lack of confirmation is just for themes, and not full addon installs?

Comment 5

[:Gijs (he/him)]

(In reply to Daniel Veditz [:dveditz] from comment #4)
> I assume this lack of confirmation is just for themes, and not full addon
> installs?

Yes, and only for lightweight themes.

Comment 6

[Armin Ebert [:arminius]]

(In reply to :Gijs Kruitbosch from comment #5)
> (In reply to Daniel Veditz [:dveditz] from comment #4)
> > I assume this lack of confirmation is just for themes, and not full addon
> > installs?
> 
> Yes, and only for lightweight themes.

However, the statement that "the full set of addon-related permissions from addons.mozilla.org gets exposed" still holds true.

The reason why the confirmation for regular addons cannot be bypassed is that even AMO does not have privileges to install without showing the approval dialog.

For regular addons the behavior still differs slightly, since AMO (or the spoofed domain) can evoke the installation promt instantly while an unprivileged site will just display that the attempt to install an addon has been blocked.

My point is that the issue is not lightweight-theme-specific but relates to all permissions of addons.mozilla.org.

Comment 7

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Update: Have patch, need to unbreak a bunch of tests.


(In reply to Armin Razmdjou from comment #6)
> My point is that the issue is not lightweight-theme-specific but relates to
> all permissions of addons.mozilla.org.

Yep.

Comment 8

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v1

Comment 9

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Regarding QE verification of this: The tests cover that we block this correctly, but I think it's worth ensuring it doesn't break real-world cases. In the case there is any instance where this breaks on a Mozilla web property (I can't think of any), we would need to get that switched over to only supporting HTTPS.

Comment 10

[:Gijs (he/him)]

Comment on attachment 8561128 [details] [diff] [review]
Patch v1

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

(Apologies for the driveby... )

::: browser/base/content/browser-addons.js
@@ +370,5 @@
>      var pm = Services.perms;
>  
>      var uri = node.ownerDocument.documentURIObject;
> +
> +    if (!uri.schemeIs("https"))

Why only https here, but also file+chrome in XPIProvider.jsm? How does this work with the "builtin" themes provided in about:customizing? (we... may or may not have tests for that. I forget.)

Also, you added a pref but don't seem to check it here... just because the tests for lwts don't need it, and the pref is really only there because tests?

Almost wonder if it should be a private bool that you set with a backstagepass to the XPIProvider.jsm code, so that we discourage add-ons / customized installs / malware from triggering that pref... Wouldn't want to pretend that making us intentionally less secure is supported (see also: https://twitter.com/dveditz/status/563774923697422339 )

Comment 11

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to :Gijs Kruitbosch from comment #10)
> Why only https here, but also file+chrome in XPIProvider.jsm?

Yes, that's intentional - we have existing bugs where non-web protocols don't currently work for lwthemes. I'm not fixing those here, and I'd rather it at least be consistent for lwthemes.


> Also, you added a pref but don't seem to check it here... just because the
> tests for lwts don't need it, and the pref is really only there because
> tests?

For XPI installs, we need to support that pref (extensions.install.requireSecureOrigin) for enterprise environments. I don't see a reason to support it for lwthemes.

Comment 12

[Dave Townsend [:mossop]]

Comment on attachment 8561128 [details] [diff] [review]
Patch v1

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

::: toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ +3741,5 @@
>        return false;
>  
> +    let requireSecureOrigin = Preferences.get(PREF_INSTALL_REQUIRESECUREORIGIN, true);
> +    let safeSchemes = ["https", "chrome", "file"];
> +    if (requireSecureOrigin && !safeSchemes.includes(aUri.scheme))

It's not clear that Array.includes is going to ride the trains yet (bug 1070767), please use indexOf or find instead.

Comment 13

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to Dave Townsend [:mossop] from comment #12)
> It's not clear that Array.includes is going to ride the trains yet (bug
> 1070767), please use indexOf or find instead.

Oh FFS...

Thanks for the heads up.

Comment 14

[Blair McBride [:Unfocused] (UNAVAILABLE)]

https://hg.mozilla.org/integration/fx-team/rev/d86ac92e8a41

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d86ac92e8a41

Comment 16

[Daniel Veditz [:dveditz]]

assigning a security severity of sec-low because there's no permanent damage that can be done here. Lightweight themes are easily undone, although having one thrust on you would be surprising and possibly offensive. Non-AMO sites can already trigger installs, they just get an "allow" infobar before the main confirmation dialog, so this bug allows a MITM to save a click. Would potentially allow for a better spoof of AMO if you had a big green install button that actually behaved like the real AMO instead of having an infobar first, although if you successfully fool people that they're really on AMO they're likely to do whatever the site says to do. OK, given the seriousness of spoofing AMO I guess I can talk myself into sec-moderate.

Comment 17

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Daniel, I'm not familiar with policy around uplifting for the various sec- levels. Should this get uplifted?

Comment 18

[Daniel Veditz [:dveditz]]

We should definitely land this in Aurora. It would be nice to get this small (~7 line) fix into Beta but we're running up against the last few betas so it's OK if a sec-moderate misses that train if lmandel doesn't want to take it. I'd go ahead an request approval for both and see what he says.

We don't usually take moderates on the ESR, but we can when they're safe enough and a bad enough risk. This one is certainly safe enough but it's not that worrying a bug for an enterprise environment.

Comment 19

[Sylvestre Ledru [:Sylvestre]]

If it is safe, please nominate for uplift to beta? (and aurora anyway).
Thanks

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I do not think that this bug is fixed! It works perfect when a user clicks on an XPI or the install trigger is used but not when you directly load an XPI URL like:

http://mozqa.com/data/firefox/addons/extensions/icons.xpi

There is no notification shown at all. Blair, can you confirm that this is still broken? That is at least what our Mozmill tests have discovered!

Comment 21

[Dave Townsend [:mossop]]

(In reply to Henrik Skupin (:whimboo) from comment #20)
> I do not think that this bug is fixed! It works perfect when a user clicks
> on an XPI or the install trigger is used but not when you directly load an
> XPI URL like:
> 
> http://mozqa.com/data/firefox/addons/extensions/icons.xpi
> 
> There is no notification shown at all. Blair, can you confirm that this is
> still broken? That is at least what our Mozmill tests have discovered!

That's intentional, directly entering a url into the address bar has always bypassed the site install whitelist.

Comment 22

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Thanks Dave.

Comment 23

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to Daniel Veditz [:dveditz] from comment #18)
> We don't usually take moderates on the ESR, but we can when they're safe
> enough and a bad enough risk. This one is certainly safe enough but it's not
> that worrying a bug for an enterprise environment.

I consider this higher risk for ESR, because the change is much more likely to affect enterprise/corporate environments. I can almost guarantee various companies will be affected by this, so not a great thing to have in a point-release.

And given the timeframe, I don't feel comfortable with Beta. The code itself is low risk, but it's effects aren't.

Comment 24

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 8561128 [details] [diff] [review]
Patch v1

Approval Request Comment
[Feature/regressing bug #]: Add-on install from webpage
[User impact if declined]: Potential for MiTM attack, installing add-ons from an unsafe origin. However, there's still confirmations to get past.
[Describe test coverage new/current, TreeHerder]: Unit tests and QE
[Risks and why]: Low code risk, but will affect intentional cases of installing from HTTP. These do exist in the wild, sadly.
[String/UUID change made/needed]: None

Comment 25

[Blair McBride [:Unfocused] (UNAVAILABLE)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a57074c09df5

Comment 26

[Sylvestre Ledru [:Sylvestre]]

Comment #23 says that we should not take it in 36.

Comment 27

[Daniel Veditz [:dveditz]]

Given the limits on the potential damage it doesn't fit our normal bounty guidelines but we'd like to offer a partial bounty as a thank you for reporting this.

Comment 28

[Armin Ebert [:arminius]]

Thanks a lot!

I agree that the general permission leakage from the AMO domain does not imply a high security impact here, since the current permission set is rather limited.

Only for the sake of documentation I want to add the scenario, where an invisible theme (maybe named "Default") with an attacker-controlled "updateURL" is forced on the browser, acting like an evercookie that occasionally reveals the user's IP during an update request. Imagine this being injected into an anonymous blogger's browser, who unwittingly auto-updates the theme while having his private proxy turned off - and will likely never notice the presence of this hidden beacon.

Comment 29

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to Armin Razmdjou from comment #28)
> Only for the sake of documentation I want to add the scenario, where an
> invisible theme (maybe named "Default") with an attacker-controlled
> "updateURL" is forced on the browser, acting like an evercookie that
> occasionally reveals the user's IP during an update request. Imagine this
> being injected into an anonymous blogger's browser, who unwittingly
> auto-updates the theme while having his private proxy turned off - and will
> likely never notice the presence of this hidden beacon.

This is unrelated, right? We already require the updateURL to be over HTTPS - such that the default configuration doesn't even allow an add-on to be enabled unless updateURL is HTTPS.

With any add-on installed from AMO, all updates go through AMO. Any add-ons not installed from AMO can use this as a phone-home - admittedly we don't expose that very well. Perhaps a new bug worth exploring?

Comment 30

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Daniel: Can we safely inform add-on authors/publishers of this new requirement? We've just had bug 1138138 filed, due to confusion caused by this change. Typically jorgev posts on the AMO blog about changes that affect add-ons like this.

Comment 31

[Dave Townsend [:mossop]]

(In reply to Blair McBride [:Unfocused] (I don't read bugmail - needinfo? me!) from comment #30)
> Daniel: Can we safely inform add-on authors/publishers of this new
> requirement? We've just had bug 1138138 filed, due to confusion caused by
> this change. Typically jorgev posts on the AMO blog about changes that
> affect add-ons like this.

Apparently I can't read because I didn't think about the third party implications here. I don't know if forcing https for other sites is the right thing to do, especially since we might be dropping the whitelist in the future after add-on signing is a requirement.

Maybe we should only enforce https for the sites listed in the default prefs?

Comment 32

[Daniel Veditz [:dveditz]]

Both the IETF and W3C have published recommendations against using insecure http for things (particularly new things where we don't have to worry about compatibility with existing content). I feel fairly confident we would have made this change eventually without this particular example of how it can be abused, it just would have taken longer. Even for capabilities that we're likely to continue to allow for http sites such as geolocation and camera access we're likely to require https sites to save the permission.

Comment 33

[Benjamin Kerensa [:bkerensa]]

Blair, per comment 23 will we need this backported for ESR?

Comment 34

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to Benjamin Kerensa [:bkerensa] from comment #33)
> Blair, per comment 23 will we need this backported for ESR?

No, I don't consider this safe for ESR-31. And AFAIK, this should automatically make it into ESR-38.

Comment 35

[Blair McBride [:Unfocused] (UNAVAILABLE)]

I agree with Daniel in comment 32 about this. I don't think signing is enough to not enforce HTTPS everywhere.

Daniel: Could you comment in respect to comment 30?

Comment 36

[Daniel Veditz [:dveditz]]

Yes, we should mention this requirement in the release notes and on the add-ons blog. We should also document (and mention in the blog, but not the release notes) the availability of the developer pref "extensions.install.requireSecureOrigin".

Comment 37

[Jorge Villalobos [:jorgev] (he/him)]

So, if I understand correctly, on Firefox 37 and above it will only possible to install add-ons from HTTPS URLs, except when the URL is entered directly in the URL bar or "extensions.install.requireSecureOrigin" is set to false. And once the add-on is installed, non-HTTPS updates are still supported. Is this correct?

Comment 38

[[:Cww]]

I got CC'd on this (probably because I'm supposed to keep an eye out for feedback about it in 37). The effect is that users won't be able to install XPIs over HTTP.

This is going to be a low volume action (installing addons from anything is already a low volume action) so it's going to be hard to tease out of feedback. Are there any popular add-ons/theme sites we know where this is the main path of installation that I might key on to look for?

Comment 39

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to Jorge Villalobos [:jorgev] from comment #37)
> So, if I understand correctly, on Firefox 37 and above it will only possible
> to install add-ons from HTTPS URLs, except when the URL is entered directly
> in the URL bar or "extensions.install.requireSecureOrigin" is set to false.

Correct.

> And once the add-on is installed, non-HTTPS updates are still supported.

Updates aren't changed, nor are requirements for the location of the actual file - only the requirements for the page starting the install. 

The update manifest must be via HTTPS - this is an existing requirement (except if extensions.checkUpdateSecurity=false). But the actual file it points to can be via HTTP, assuming the update manifest specifies a secure file hash (ditto with where the file is hosted for an initial install - need a hash if it points to a HTTP URL).

Comment 40

[Dave Townsend [:mossop]]

(In reply to Blair McBride [:Unfocused] (I don't read bugmail - needinfo? me!) from comment #39)
> (In reply to Jorge Villalobos [:jorgev] from comment #37)
> > So, if I understand correctly, on Firefox 37 and above it will only possible
> > to install add-ons from HTTPS URLs, except when the URL is entered directly
> > in the URL bar or "extensions.install.requireSecureOrigin" is set to false.
> 
> Correct.

This is not correct. The change has made it impossible to whitelist installation from HTTP sites. You can still install add-ons from HTTP sites, you just have to click the additional "allow" doorhanger. As always with the whitelist only the url of the site triggering the install matters, the url of the XPI can be anything.

Comment 41

[Jorge Villalobos [:jorgev] (he/him)]

Ah, that's a much smaller change then. Is there any concern with announcing it now, Dan?

Comment 42

[Daniel Veditz [:dveditz]]

Nope, no concern: Announce away.

Comment 43

[Jorge Villalobos [:jorgev] (he/him)]

This isn't going into ESR 31, right?

Comment 44

[Al Billings [:abillings - ex-MoCo]]

(In reply to Jorge Villalobos [:jorgev] from comment #43)
> This isn't going into ESR 31, right?

As a sec-moderate, it wouldn't without a good reason. I'm not sure why bkerensa marked it tracking for ESR.

Comment 45

[Al Billings [:abillings - ex-MoCo]]

Comment 34 says why we're not taking this in ESR31.

Comment 46

[Christiane Ruetten [INACTIVE]]

Would a variant of this perhaps affect b2g, too?

Comment 47

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

(In reply to Christiane Ruetten [:cr] from comment #46)
> Would a variant of this perhaps affect b2g, too?

No. Addons on Firefox OS are just apps, and are unrelated to addons on the desktop. See this link for details: https://developer.mozilla.org/en-US/Firefox_OS/Add-ons