Closed
Bug 1452037
(CVE-2019-11738)
Opened 7 years ago
Closed 5 years ago
script-src 'sha256-47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU=' security policy will whitelist all javascript: URIs
Categories
(Core :: DOM: Security, defect, P2)
Core
DOM: Security
Tracking
()
People
(Reporter: jwkbugzilla, Assigned: sstreich)
References
Details
(Keywords: reporter-external, sec-low, Whiteboard: [domsecurity-active][post-critsmash-triage][adv-main69+][adv-esr68.1+])
Attachments
(2 files)
246 bytes,
text/html
|
Details | |
47 bytes,
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta+
RyanVM
:
approval-mozilla-esr68+
|
Details | Review |
Note: This is likely related to bug 1314567 which I cannot access.
As implemented right now, execution of any javascript: URIs can be allowed using a hash-based source that takes the empty string as input. For example, script-src 'sha256-47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU=' is supposed to allow empty <script> tags only but will enable arbitrary javascript: URIs as well. So specifying a source like this weakens CSP protection considerably which is not obvious.
You can reproduce by opening the attached proof of concept page and clicking the link. In Chrome 65 this doesn't do anything, the javascript: URI is blocked by CSP. In Firefox 59.0.2 and Firefox 61.0a1 (2018-04-04) nightly on the other hand the code executes and shows a message saying "clicked."
IMHO the issue is the code here:
> https://dxr.mozilla.org/mozilla-central/rev/2f5ffe4fa2153a798ed8b310a597ea92abd1b868/dom/security/nsCSPContext.cpp#582
If aElementOrContent is neither a string nor an element (nullptr in this case) this code will leave content variable uninitialized (empty) but perform matching hash sources nevertheless.
I can see two ways how this can have a security impact:
1) A website might specify this script source assuming that it is innocuous (an empty script cannot do any harm). Not sure whether any real-world websites actually do this.
2) WebExtensions aren't allowed to specify unsafe-inline in their CSP for security reasons, meaning in particular that they aren't allowed to use javascript: URIs. This issue allows weakening the CSP protection nevertheless.
Updated•7 years ago
|
Flags: sec-bounty?
Comment 1•7 years ago
|
||
I'm not that concerned about the first scenario, but bypassing the WebExtensions CSP is bad :)
CCing ckerschbaumer for this CSP bypass.
Updated•7 years ago
|
Component: DOM → DOM: Security
Updated•7 years ago
|
Updated•7 years ago
|
Status: NEW → ASSIGNED
Updated•7 years ago
|
Group: core-security → dom-core-security
Updated•6 years ago
|
Flags: sec-bounty? → sec-bounty-
Updated•5 years ago
|
Assignee: ckerschb → streich.mobile
Assignee | ||
Comment 2•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Keywords: checkin-needed
Comment 3•5 years ago
|
||
Keywords: checkin-needed
Comment 4•5 years ago
|
||
Group: dom-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
status-firefox70:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
Comment 5•5 years ago
|
||
Please nominate this for Beta & ESR68 approval when you get a chance.
status-firefox68:
--- → wontfix
status-firefox69:
--- → affected
status-firefox-esr60:
--- → wontfix
status-firefox-esr68:
--- → affected
tracking-firefox69:
--- → +
tracking-firefox70:
--- → +
tracking-firefox-esr68:
--- → 69+
Flags: needinfo?(streich.mobile)
Flags: in-testsuite+
Assignee | ||
Comment 6•5 years ago
|
||
Comment on attachment 9072981 [details]
Bug 1452037 - Fix Whitelisting of javascript-uris with csp hash r=ckerschb
Beta/Release Uplift Approval Request
- User impact if declined: The user is vulnerable to a csp-bypass, if the page owner did
set "script-src <someHash>" - Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): The patch is adding one new codepath in the csp-hash validation:
- if no script is given to calculate a hash on, we're blocking the script.
This is only the case for javascript uris.
- String changes made/needed:
ESR Uplift Approval Request
- If this is not a sec:{high,crit} bug, please state case for ESR consideration: If a page has set "script-src 'sha256-<someHash>' firefox currently allows
a script if it fails to get a sample to calculate its hash.
This allows any uri with "javascript:<code>" and and opens up a vector to bypass csp. - User impact if declined: The user is vulnerable to a csp-bypass.
- Fix Landed on Version: Nightly 70
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): The patch is adding one new codepath in the csp-hash validation:
- if no script is given to calculate a hash on, we're blocking the script.
This is only the case for javascript uris.
- String or UUID changes made by this patch:
Flags: needinfo?(streich.mobile)
Attachment #9072981 -
Flags: approval-mozilla-esr68?
Attachment #9072981 -
Flags: approval-mozilla-beta?
Comment 7•5 years ago
|
||
Comment on attachment 9072981 [details]
Bug 1452037 - Fix Whitelisting of javascript-uris with csp hash r=ckerschb
Low-risk csp bypass fix with automated tests. Approved for 69.0b6 and 68.1esr.
Attachment #9072981 -
Flags: approval-mozilla-esr68?
Attachment #9072981 -
Flags: approval-mozilla-esr68+
Attachment #9072981 -
Flags: approval-mozilla-beta?
Attachment #9072981 -
Flags: approval-mozilla-beta+
Comment 8•5 years ago
|
||
uplift |
Comment 9•5 years ago
|
||
uplift |
Updated•5 years ago
|
Flags: qe-verify-
Whiteboard: [domsecurity-active] → [domsecurity-active][post-critsmash-triage]
Updated•5 years ago
|
Alias: CVE-2019-11738
Whiteboard: [domsecurity-active][post-critsmash-triage] → [domsecurity-active][post-critsmash-triage][adv-main68+][adv-esr68.1+]
Updated•5 years ago
|
Whiteboard: [domsecurity-active][post-critsmash-triage][adv-main68+][adv-esr68.1+] → [domsecurity-active][post-critsmash-triage][adv-main69+][adv-esr68.1+]
Updated•4 years ago
|
Flags: sec-bounty-hof+
Updated•4 years ago
|
Group: core-security-release
Updated•4 months ago
|
Keywords: reporter-external
You need to log in
before you can comment on or make changes to this bug.
Description
•