Closed Bug 1226423 (CVE-2015-7223) Opened 9 years ago Closed 9 years ago

Privilege escalation vulnerabilities in WebExtension APIs

Categories

(WebExtensions :: Untriaged, defect)

defect
Not set
critical

Tracking

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

RESOLVED FIXED
Iteration:
45.2 - Nov 30
Tracking Status
firefox42 --- wontfix
firefox43 + fixed
firefox44 + fixed
firefox45 + fixed
firefox-esr38 --- unaffected

People

(Reporter: kmag, Assigned: kmag)

Details

(Keywords: sec-critical, Whiteboard: [adv-main43+])

Attachments

(5 files, 1 obsolete file)

Attached file 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
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-highsec-critical
I guess we need to move the addonId check above the docshell comparison in GlobalManager.observe. I think that should fix it.
Assignee: nobody → kmaglione+bmo
Should we request an uplift on this (once its done) so we can have it in 43?
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?
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.
Attachment #8690292 - Flags: review?(wmccloskey)
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+
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?
Attachment #8690292 - Attachment is obsolete: true
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?
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
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
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)
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.
Status: NEW → RESOLVED
Closed: 9 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)
Flags: needinfo?(kmaglione+bmo)
Iteration: --- → 45.2 - Nov 30
Does this still need to land in beta? Thanks!
Flags: needinfo?(cbook)
Group: toolkit-core-security → core-security-release
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
Flags: needinfo?(kmaglione+bmo)
Group: core-security-release
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: