Bug 1226423 (CVE-2015-7223)

Privilege escalation vulnerabilities in WebExtension APIs

RESOLVED FIXED

Status

--
critical
RESOLVED FIXED
3 years ago
4 months ago

People

(Reporter: kmag, Assigned: kmag)

Tracking

({sec-critical})

unspecified
sec-critical

Firefox Tracking Flags

(firefox42 wontfix, firefox43+ fixed, firefox44+ fixed, firefox45+ fixed, firefox-esr38 unaffected)

Details

(Whiteboard: [adv-main43+])

Attachments

(5 attachments, 1 obsolete attachment)

(Assignee)

Description

3 years ago
Created attachment 8689803 [details]
Testcase add-on

We don't currently check that a document belongs to our extension before injecting APIs into it. In the case of background pages, we also continue injecting APIs into new window globals even after the first load. This means that if a background page navigates to a remote web page, that page has the full privileges of the extension.

The attached testcase has a background page which navigates to https://people.mozilla.org/~kmaglione/webext-privilege-escalation.html, which contains this code:

    dump(`WEBEXT-PRIVILEGE-ESCALATION.HTML\n`);
    dump(`WEBEXT-PRIVILEGE-ESCALATION.HTML: window.browser: ${window.browser && Object.keys(window.browser).sort()}\n`);
    browser.tabs.query({}, tabs => {
      dump(`WEBEXT-PRIVILEGE-ESCALATION.HTML: Tab urls: ${Array.from(tabs, t => t.url).join(" ")}\n`);
    });

And results in this console output:

WEBEXT-PRIVILEGE-ESCALATION.HTML
WEBEXT-PRIVILEGE-ESCALATION.HTML: window.browser: alarms,browserAction,extension,i18n,pageAction,runtime,tabs,test,windows
WEBEXT-PRIVILEGE-ESCALATION.HTML: Tab urls: about:home about:addons
(Assignee)

Comment 1

3 years ago
Hm. Actually, this behavior seems to apply to remote URLs loaded into popups, too, which means that this definitely happens in the wild. Bumping to critical.
Severity: major → critical
Keywords: sec-high → sec-critical
I guess we need to move the addonId check above the docshell comparison in GlobalManager.observe. I think that should fix it.
(Assignee)

Updated

3 years ago
Assignee: nobody → kmaglione+bmo

Comment 3

3 years ago
Should we request an uplift on this (once its done) so we can have it in 43?
(Assignee)

Comment 4

3 years ago
I was going to request uplift all the way to release, if it's affected.

At the moment, I just have to finish writing the panel tests. Unfortunately, those are probably going to have to be different for every version we uplift to.
Kris, comment 1 sounds a little scary now that I re-read it. Do you mean browser action and page action popups or just ordinary popups opened by web pages?
(Assignee)

Comment 6

3 years ago
I mean browser action popups. It's not nearly as bad as it would be for ordinary web pages, but it's still pretty bad given how often those popups are used to load ordinary web content.
(Assignee)

Comment 7

3 years ago
Created attachment 8690289 [details] [diff] [review]
Don't inject WebExtension APIs into documents without WebExtension principals
Attachment #8690289 - Flags: review?(wmccloskey)
(Assignee)

Comment 8

3 years ago
Created attachment 8690290 [details] [diff] [review]
[webext] Add tests for API injection into background pages
Attachment #8690290 - Flags: review?(wmccloskey)
(Assignee)

Comment 9

3 years ago
Created attachment 8690292 [details] [diff] [review]
[wbext] Add tests for panel popup API injection
Attachment #8690292 - Flags: review?(wmccloskey)
(Assignee)

Comment 10

3 years ago
Comment on attachment 8690290 [details] [diff] [review]
[webext] Add tests for API injection into background pages

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

::: toolkit/components/extensions/test/mochitest/file_privilege_escalation.html
@@ +5,5 @@
> +<meta charset="utf-8">
> +</head>
> +<body>
> +  <script type="application/javascript">
> +    throw new Error(`WebExt Privilege Escalation: typeof(browser) = ${typeof(browser)}`);

This is definitely a hack, but I couldn't think of a good e10s-compatible way of checking this that wouldn't have required a lot of new code.
Attachment #8690289 - Flags: review?(wmccloskey) → review+
Attachment #8690290 - Flags: review?(wmccloskey) → review+
Comment on attachment 8690292 [details] [diff] [review]
[wbext] Add tests for panel popup API injection

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

::: browser/components/extensions/test/browser/browser_ext_popup_api_injection.js
@@ +16,5 @@
> +    },
> +
> +    files: {
> +      "popup-a.html": String.raw`<html><head><meta charset="utf-8"><script type="application/javascript">
> +        browser.runtime.sendMessage("from-popup-a");

You should be able to use browser.test from this context without a problem.

@@ +47,5 @@
> +  });
> +
> +  let browserActionId = makeWidgetId(extension.id) + "-browser-action";
> +  let pageActionId = makeWidgetId(extension.id) + "-page-action";
> +  let panelId = makeWidgetId(extension.id) + "-panel";

This is unused.
Attachment #8690292 - Flags: review?(wmccloskey) → review+
(Assignee)

Comment 12

3 years ago
Comment on attachment 8690289 [details] [diff] [review]
Don't inject WebExtension APIs into documents without WebExtension principals

Approval Request Comment
[Feature/regressing bug #]: Bug 1226423
[User impact if declined]:

This is a privilege escalation vulnerability which allows code web content to
execute code with the privileges of a particular WebExtension. Depending on
the privileges of the extension which causes the leak, this could result in
minor personal information leaks, or full-scale cross-site-scripting,
including theft of all browser cookies.

The potential for damage is limited by the fact that it requires an extension
to intentionally load a remote web page either as a background script, or into
a panel opened by a toolbar button. The former is unlikely. The latter is
common in Add-on SDK add-ons, and has already been seen in WebExtension
examples, but is not trivially possible in Chrome extensions.

Given that this is a security bug, and landing a fix on mozilla-central will
make its existence public, I'm requesting landing on all branches at once.

[Describe test coverage new/current, TreeHerder]:

There are tests attached to this bug which verify that the behavior is fixed.
Tests for code that it's likely to affect is moderate, but not exhaustive.
However, even tests much more basic than those we have should be sufficient to
catch any disruption from this change.

The tests for this fix will most likely not be upliftable to Release or Beta.

[Risks and why]:

Low. The change is extremely small, and should not have any impact outside of
the WebExtensions tree. The support for this feature on Release and Beta is
limited, and not production quality, so the potential for noticeable
disruption is very small.

[String/UUID change made/needed]:

None.
Attachment #8690289 - Flags: approval-mozilla-release?
Attachment #8690289 - Flags: approval-mozilla-beta?
Attachment #8690289 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 13

3 years ago
Created attachment 8690345 [details] [diff] [review]
[wbext] Add tests for panel popup API injection
(Assignee)

Updated

3 years ago
Attachment #8690292 - Attachment is obsolete: true
(Assignee)

Comment 14

3 years ago
Comment on attachment 8690289 [details] [diff] [review]
Don't inject WebExtension APIs into documents without WebExtension principals

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

Very easily, in cases where an affected extension is installed, or
can be installed.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

Yes.

Which older supported branches are affected by this flaw?

Release 42 and later.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

The fix should apply cleanly to all branches. The tests can be
backported, but doing so would not be trivial.

How likely is this patch to cause regressions; how much testing does it need?

Very low. The change is very small, and only affects WebExtension code.
The related code on Relase and Beta branches is essentially in a
pre-release state, so any visible regressions are unlikely to be
noticed.
Attachment #8690289 - Flags: sec-approval?
status-firefox42: --- → wontfix
status-firefox43: --- → affected
status-firefox44: --- → affected
status-firefox45: --- → affected
tracking-firefox43: --- → +
tracking-firefox44: --- → +
tracking-firefox45: --- → +
status-firefox-esr38: --- → unaffected
Comment on attachment 8690289 [details] [diff] [review]
Don't inject WebExtension APIs into documents without WebExtension principals

sec-approval+ for trunk and I'm giving Aurora and Beta approval.
This isn't going to get "release" approval as we aren't doing a chemspill for this.
Attachment #8690289 - Flags: sec-approval?
Attachment #8690289 - Flags: sec-approval+
Attachment #8690289 - Flags: approval-mozilla-release?
Attachment #8690289 - Flags: approval-mozilla-release-
Attachment #8690289 - Flags: approval-mozilla-beta?
Attachment #8690289 - Flags: approval-mozilla-beta+
Attachment #8690289 - Flags: approval-mozilla-aurora?
Attachment #8690289 - Flags: approval-mozilla-aurora+
Any tests that expose this issue cannot be checked in until we ship a release with this fix (plus a few weeks at least).
this still need to land on m-c
(Assignee)

Comment 18

3 years ago
https://hg.mozilla.org/integration/fx-team/rev/2948ba54950b65cadcacf16a75828e56e164998d
Bug 1226423: Don't inject WebExtension APIs into documents without WebExtension principals. r=billm

https://hg.mozilla.org/integration/fx-team/rev/f127897cdf0193ac09fa96f6f5bec5d2ae2d2331
Bug 1226423: [webext] Add tests for API injection into background pages. r=billm

https://hg.mozilla.org/integration/fx-team/rev/65cd2c7696e438c74bfbb0ad40e18f411dbead38
Bug 1226423: [wbext] Add tests for panel popup API injection. r=billm
(Assignee)

Comment 19

3 years ago
Er, it looks like I accidentally landed the tests with the fix. I guess it's too late to do anything about that now.
This appears to have caused an intermittent failure on aurora: https://treeherder.mozilla.org/logviewer.html#?job_id=1524643&repo=mozilla-aurora

Any ideas for a fix so I don't have to back it all out?
Flags: needinfo?(kmaglione+bmo)
(Assignee)

Comment 22

3 years ago
I'd be OK with backing out just the test commits (since I shouldn't have landed them in the first place...), but uplifting bug 1224893 should also fix the intermittents.
Flags: needinfo?(kmaglione+bmo)
A note to say the test fix is on aurora now, but we still need to back the patch on this bug out of beta.

Updated

3 years ago
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
I take it back, we never landed it on beta.  So, looks like we should land https://bugzilla.mozilla.org/attachment.cgi?id=8690289 on beta without any of the tests.
(In reply to Kris Maglione [:kmag] from comment #22)
> I'd be OK with backing out just the test commits (since I shouldn't have
> landed them in the first place...), but uplifting bug 1224893 should also
> fix the intermittents.

backed this out in https://treeherder.mozilla.org/#/jobs?repo=mozilla-aurora&revision=f8573ae21af8 but the patch is still in of course
Hi Kris, the fix doesn't apply to beta cleanly:

grafting 316380:af47051962f8 "Bug 1226423 - Don't inject WebExtension APIs into documents without WebExtension principals. r=billm, a=al"
merging toolkit/components/extensions/Extension.jsm
warning: conflicts during merge.
merging toolkit/components/extensions/Extension.jsm incomplete! (edit conflicts, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use hg resolve and hg graft --continue)

could you take a look, thanks!
Flags: needinfo?(kmaglione+bmo)
(Assignee)

Comment 27

3 years ago
Created attachment 8692227 [details] [diff] [review]
Patch grafted to mozilla-beta.
Flags: needinfo?(kmaglione+bmo)

Updated

3 years ago
Iteration: --- → 45.2 - Nov 30
Does this still need to land in beta? Thanks!
Flags: needinfo?(cbook)
https://hg.mozilla.org/releases/mozilla-beta/rev/47075790a159
status-firefox43: affected → fixed
Flags: needinfo?(cbook)
Group: toolkit-core-security → core-security-release

Comment 30

3 years ago
Reproduced with Nightly from 2015-11-21 under Ubuntu 12.04 32-bit - with the attached testcase, the following lines are displayed in Terminal:
> WEBEXT-PRIVILEGE-ESCALATION.HTML
> WEBEXT-PRIVILEGE-ESCALATION.HTML: window.browser: alarms,browserAction,extension,i18n,pageAction,runtime,tabs,test,windows
> WEBEXT-PRIVILEGE-ESCALATION.HTML: Tab urls: about:config https://bugzilla.mozilla.org/attachment.cgi?id=8689803

Encountered the following results with 43.0b9 (Build ID: 20151203163240), across platforms [1]:
1. Via Terminal I get: 
> WEBEXT-PRIVILEGE-ESCALATION.HTML
> WEBEXT-PRIVILEGE-ESCALATION.HTML: window.browser: undefined
> JavaScript error: https://people.mozilla.org/~kmaglione/webext-privilege-escalation.html, line 13: ReferenceError: browser is not defined
2. Via Browser Console:
> ReferenceError: browser is not defined thrown by webext-privilege-escalation.html:13:5
> Security Error: Content at https://people.mozilla.org/~kmaglione/webext-privilege-escalation.html may not load or link to moz-extension://dc6ddf17-a0b6-4055-a138-56392371061e/background.js.

At a first glance, I would call this verified - but, Kris, could you please confirm that the above results are the expected ones? Thanks in advance!

[1] Windows 7 64-bit, Mac OS X 10.11 and Ubuntu 14.04 32-bit
Flags: needinfo?(kmaglione+bmo)
Whiteboard: [adv-main43+]
Alias: CVE-2015-7223
(Assignee)

Updated

3 years ago
Flags: needinfo?(kmaglione+bmo)
Group: core-security-release

Updated

4 months ago
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.