Bug 1128126 (CVE-2015-0812)

Addon permissions exposed to man-in-the-middle attacks

RESOLVED FIXED in Firefox 37

Status

()

defect
RESOLVED FIXED
5 years ago
2 years ago

People

(Reporter: armin, Assigned: Unfocused)

Tracking

({dev-doc-needed, relnote, sec-moderate})

Trunk
mozilla38
Points:
3
Dependency tree / graph
Bug Flags:
firefox-backlog +
sec-bounty +
qe-verify +

Firefox Tracking Flags

(firefox36+ wontfix, firefox37+ fixed, firefox38+ fixed, firefox-esr31 wontfix, b2g-v2.2 unaffected)

Details

(Whiteboard: [adv-main37+])

Attachments

(2 attachments)

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.
Severity: major → normal
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?
Component: Security → Add-ons Manager
Flags: needinfo?(bmcbride)
Product: Firefox → Toolkit
Version: unspecified → Trunk
Flags: sec-bounty?
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.
Assignee: nobody → bmcbride
Status: UNCONFIRMED → ASSIGNED
Points: --- → 3
Ever confirmed: true
Flags: needinfo?(dtownsend)
Flags: needinfo?(bmcbride)
Flags: firefox-backlog+
OS: Linux → All
Hardware: x86_64 → All
I can't think of any reasons.
Flags: needinfo?(dtownsend)
Iteration: --- → 38.2 - 9 Feb
Flags: qe-verify?
I assume this lack of confirmation is just for themes, and not full addon installs?
Flags: needinfo?(dtownsend)
(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.
Flags: needinfo?(dtownsend)
(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.
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.
Posted patch Patch v1Splinter Review
Attachment #8561128 - Flags: review?(dtownsend)
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.
Flags: qe-verify? → qe-verify+
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 )
Attachment #8561128 - Flags: feedback+
(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 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.
Attachment #8561128 - Flags: review?(dtownsend) → review+
(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.
Iteration: 38.2 - 9 Feb → 38.3 - 23 Feb
https://hg.mozilla.org/mozilla-central/rev/d86ac92e8a41
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
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.
Keywords: sec-moderate
Daniel, I'm not familiar with policy around uplifting for the various sec- levels. Should this get uplifted?
Flags: needinfo?(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.
If it is safe, please nominate for uplift to beta? (and aurora anyway).
Thanks
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!
Flags: needinfo?(bmcbride)
(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.
(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.
Flags: needinfo?(bmcbride)
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
Attachment #8561128 - Flags: approval-mozilla-aurora?
Attachment #8561128 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment #23 says that we should not take it in 36.
Flags: sec-bounty? → sec-bounty+
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.
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.
Depends on: 1138138
(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?
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.
Flags: needinfo?(dveditz)
(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?
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.
Flags: needinfo?(dveditz)
Blair, per comment 23 will we need this backported for ESR?
Flags: needinfo?(bmcbride)
(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.
Flags: needinfo?(bmcbride)
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?
Flags: needinfo?(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".
Flags: needinfo?(dveditz)
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?
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?
(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).
(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.
Ah, that's a much smaller change then. Is there any concern with announcing it now, Dan?
Flags: needinfo?(dveditz)
Nope, no concern: Announce away.
Flags: needinfo?(dveditz)
This isn't going into ESR 31, right?
Flags: needinfo?(bkerensa)
(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.
Flags: needinfo?(bkerensa)
Whiteboard: [adv-main37+]
Alias: CVE-2015-0812
Comment 34 says why we're not taking this in ESR31.
Would a variant of this perhaps affect b2g, too?
status-b2g-v2.2: --- → ?
Flags: needinfo?(ptheriault)
(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
Flags: needinfo?(ptheriault)
Group: core-security → core-security-release
See Also: → CVE-2016-1948
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.