Closed Bug 1483602 (CVE-2018-12396) Opened 2 years ago Closed 2 years ago

Extensions can run content scripts anywhere when the document navigates during content script execution

Categories

(WebExtensions :: General, defect, P1)

defect

Tracking

(firefox-esr52 unaffected, firefox-esr6063+ verified, firefox61 wontfix, firefox62 wontfix, firefox63 verified)

VERIFIED FIXED
mozilla63
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 63+ verified
firefox61 --- wontfix
firefox62 --- wontfix
firefox63 --- verified

People

(Reporter: robwu, Assigned: robwu)

Details

(Keywords: regression, sec-moderate, Whiteboard: [adv-main63+][adv-esr60.3+])

Attachments

(3 files)

There are multiple TOCOU bugs in extension the scheduler of extension content scripts that allow extensions to run content scripts in principals that it should not have access to.


TOCTOU bug 1:
When an extension is just installed, or when a content process starts up,
injectExtensionScripts is called to run content scripts in existing pages.
https://searchfox.org/mozilla-central/rev/2466b82b/toolkit/components/extensions/extension-process-script.js#202-221

It first checks matchesWindow and then runs scripts in three batches, document_start, document_end and document_idle. There is no guarantee that the inner window has not changed.

TOCTOU bug 2:
When a new document is loaded, ExtensionPolicyService::CheckContentScripts checks whether a script should run via script->Matches(aDocInfo), and then loads a script via loadContentScript:
https://searchfox.org/mozilla-central/rev/2466b82b/toolkit/components/extensions/ExtensionPolicyService.cpp#338-354
https://searchfox.org/mozilla-central/rev/2466b82b/toolkit/components/extensions/extension-process-script.js#499-503

The problem is that aDocInfo has a fixed value, while the document is not guaranteed to have a constant value.

TOCTOU bug 3:
When a dynamic content script is executed (via "Extension:Execute" message), there is no check that the extension is still valid (i.e. not disabled or reloaded). Consequently, it is possible for a dynamic content script to execute after having uninstalled an add-on.
https://searchfox.org/mozilla-central/rev/2466b82b/toolkit/components/extensions/ExtensionContent.jsm#865-876

In short:
- TOCTOU bug 1 and 2 allows extensions to escalate privileges.
- TOCTOU 3 allows extensions to run code after an extension is disabled (any of these bug do).


Bug 1 was introduced in Firefox 57 by https://hg.mozilla.org/mozilla-central/rev/ce17d1d232f1
Bug 2 was introduced in Firefox 55 by https://hg.mozilla.org/mozilla-central/rev/32a3b7c39207
ESR 52 is not affected (tested with 52.9), ESR 60 and current release branches are affected.
Attached file toctou-scripts.zip
STR:
1. Load attached extension at about:debugging.
2. Visit example.com
3. A dialog appears. If the dialog is not automatically closed after the navigation completes, close the dialog.
4. Wait until the new navigation (addons.mozilla.org) has finished.

Expected:
= No additional dialogs.

Actual:
- Dialog with: "Managed to run script at https://addons.mozilla.org/en-US/firefox/


If you don't see the actual result, try navigating back to example.com (and refresh the page).
The first paragraph of the bug looks unintelligible. It should read:

> There are multiple time of check to time to use (TOCTOU) bugs in the scheduler of extension content scripts that allow an extension to run content scripts in principals that it should not have access to.
Summary: TOCTOU enables extensions to run content scripts anywhere → Extensions can run content scripts anywhere when the document navigates during content script execution
I'm going to address the third bug as part of bug 1346941, because it has no significant security impact* and the relevant code needs to be refactored anyway.

* The third part of the bug is a race condition that allows extensions to run content scripts after an extension has shut down, but only in pages where extensions already had the permission to do so. Extensions already have this capability (they can simply inject a script in a web page).
The relevant code is being refactored by bug 1484373.
This has been fixed on trunk by bug 1484373:
- "TOCTOU bug 1" fixed by https://hg.mozilla.org/mozilla-central/rev/a7848dcb339e
- "TOCTOU bug 2" fixed by https://hg.mozilla.org/mozilla-central/rev/eef3785a6b44
  (this second fix still leaves open one (non-security?) bug though: The second loop uses aDocInfo.GetWindow() again even though the inner window might have changed.).
Verified fixed in Firefox 63, buildID 20180827220123 .

I want to uplift a fix to Beta and ESR because the bug is easy to exploit and allows extensions to escalate privileges (and even run code on AMO).

The changes from bug 1484373 are however too large for uplifting, so try a minimal fix such as comment 4.
Note that we're already build Fx62 release candidates, so this is going to have to ship with Fx63 and ESR 60.3 in the next cycle.
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Group: toolkit-core-security → core-security-release
Verified as fixed in Firefox 63.0b3,Build ID 20180904170936, using Windows10x64 and macOS 10.13.6.
Status: RESOLVED → VERIFIED
What needs to be done for ESR 60.3 here?
Flags: needinfo?(rob)
I've rebased https://phabricator.services.mozilla.com/D3772 to the ESR60 branch and requested review again.
Flags: needinfo?(rob)
QA Contact: ddurst
esr60 will need manual qa using the extension.  I've verified the fix with the extension on esr60.  The test patch will only apply and work on nightly.
Comment on attachment 9002424 [details]
Bug 1483602 - Skip unnecessary content scripts

r=mixedpuppy at https://phabricator.services.mozilla.com/D3772 , but this r+ is somehow not mirrored here.

[ESR Uplift Approval Request]

If this is not a sec:{high,crit} bug, please state case for ESR consideration: Privilege escalation in extensions; extensions can run code on any website.

User impact if declined: Extensions with minimal permissions can run JavaScript code on any website, even privileged websites such as addons.mozilla.org .

Fix Landed on Version: 63

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): The cpp changes from the patch have already landed in 63 in bug 1484373. The JS-specific part has not, but its correctness was verified using the automated test in 63.

String or UUID changes made by this patch: none
Attachment #9002424 - Flags: approval-mozilla-esr60?
Comment on attachment 9002424 [details]
Bug 1483602 - Skip unnecessary content scripts

Thanks for the rebase and testing. Approved for ESR 60.3.
Attachment #9002424 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
https://hg.mozilla.org/releases/mozilla-esr60/rev/f27145bd5502

Rob, how/where are you planning to land the tests?
Flags: qe-verify+
Flags: needinfo?(rob)
Flags: in-testsuite?
After a few releases (if at all). I don't want to put users who haven't updated yet at risk, and the tests show that a nested event loop is all that it takes to exploit this vulnerability.
Flags: needinfo?(rob)
Verified as fixed using macOS with FF 60.2.3esr(buildid 20181012094822) and Win10x64 with FF 60.2.3esr(buildid  	20181012094822)
Flags: qe-verify+
Whiteboard: [adv-main63+][adv-esr60.3+]
Alias: CVE-2018-12396

Rob, the test patch here never progressed. Any status on this?

Flags: needinfo?(rob)

I planned to update the comments, rebase it and land it when this bug becomes public.

Flags: needinfo?(rob)

It's a year since this landed, what is the process to close this out?

Flags: needinfo?(dveditz)
Group: core-security-release
Flags: needinfo?(dveditz)

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Keywords: regression
You need to log in before you can comment on or make changes to this bug.