Closed Bug 1493629 Opened 3 years ago Closed 3 years ago

AddressSanitizer: heap-use-after-free [@ fetch_sub] with WRITE of size 8 in WorkerCSPEventListener


(Core :: DOM: Security, defect, P1)




Tracking Status
firefox-esr60 --- unaffected
firefox62 --- unaffected
firefox63 + fixed
firefox64 + fixed


(Reporter: decoder, Assigned: baku)


(Blocks 1 open bug)


(4 keywords, Whiteboard: [domsecurity-active][post-critsmash-triage])


(3 files)

The attached crash information was submitted via the ASan Nightly Reporter on mozilla-central-asan-nightly revision 64.0a1-20180920220102-

For detailed crash information, see attachment.
Flags: sec-bounty?
Needinfo for :baku, could you take a look at this since you are the author of WorkerCSPEventListener? Thanks!
Flags: needinfo?(amarchesini)
Group: core-security → dom-core-security
Assigning to :baku as a likely victim, but if it's not you please hand it off to a specific someone to look at this sec-high bug rather than dropping it back to "nobody".
Assignee: nobody → amarchesini
Priority: -- → P1
Whiteboard: [domsecurity-active]
Attached patch csp.patchSplinter Review
Flags: needinfo?(amarchesini)
Attachment #9013920 - Flags: review?(ckerschb)
Duplicate of this bug: 1492392
Comment on attachment 9013920 [details] [diff] [review]

Review of attachment 9013920 [details] [diff] [review]:

I guess that works, thanks. r=ckerschb
Attachment #9013920 - Flags: review?(ckerschb) → review+
Comment on attachment 9013920 [details] [diff] [review]

[Security Approval Request]

How easily could an exploit be constructed based on the patch?: Not easy at all. It's use-after-free hard to reproduce.

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

Which older supported branches are affected by this flaw?: m-b

If not all supported branches, which bug introduced the flaw?: Bug 1472927

Do you have backports for the affected branches?: No

If not, how different, hard to create, and risky will they be?: Easy

How likely is this patch to cause regressions; how much testing does it need?: None. Note that this could be not enough to fix for the crash: the patch adds security checks in the CSP EventListener creation, but maybe the bug is elsewhere.
Attachment #9013920 - Flags: sec-approval?
We only have two betas left. This bug will need release management approval, so we can backport to affected branches, before I can give sec-approval.


We should take this since it only affects mozilla-central and beta.
Flags: needinfo?(rkothari)
Yes, we'll take it still.
Flags: needinfo?(rkothari) → needinfo?(abillings)
Comment on attachment 9013920 [details] [diff] [review]

sec-approval+. Please nominate branch patches as well.
Flags: needinfo?(abillings)
Attachment #9013920 - Flags: sec-approval? → sec-approval+
Comment on attachment 9013920 [details] [diff] [review]

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1472927

User impact if declined: a crash can occur

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: No

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): This patch introduces some security checks on how CSP worker event listener is used.
No big changes.

String changes made/needed: none
Attachment #9013920 - Flags: approval-mozilla-beta?
Group: dom-core-security → core-security-release
Closed: 3 years ago
QA Contact: ckerschb
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Comment on attachment 9013920 [details] [diff] [review]

Uplift approved for 63 beta 13, thanks.
Attachment #9013920 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Backed out from beta for wpt failures on ContentSecurityPolicy:

Push with failures:

debug failure:

[task 2018-10-05T15:56:30.194Z] 15:56:30     INFO - TEST-PASS | /content-security-policy/inside-worker/dedicated-inheritance.html | Filesystem and blob. 
[task 2018-10-05T15:56:30.195Z] 15:56:30     INFO - TEST-PASS | /content-security-policy/inside-worker/dedicated-inheritance.html | Same-origin 'fetch()' in http:?pipe=sub|header(Content-Security-Policy,default-src%20*) 
[task 2018-10-05T15:56:30.195Z] 15:56:30     INFO - TEST-UNEXPECTED-TIMEOUT | /content-security-policy/inside-worker/dedicated-inheritance.html | Same-origin 'fetch()' in http:?pipe=sub|header(Content-Security-Policy,connect-src%20%27none%27) - Test timed out
[task 2018-10-05T15:56:30.195Z] 15:56:30     INFO - TEST-INFO | expected FAIL

Please also check the Nightly and DevEdition wpt failures (there are 2 per platform compared to 1 on debug).
Flags: needinfo?(amarchesini)
Duplicate of this bug: 1496794
We need to uplift bug 1490165 as well.
Flags: needinfo?(amarchesini)
Depends on: 1490165
Actually, instead of uplifting the 3 patches of bug 1490165, I can create a custom patch for m-b. This is better because we don't need all those 3 patches from bug 1490165.
Attached file m-b
Blocks: 1472927
Flags: sec-bounty? → sec-bounty+
Flags: qe-verify-
Whiteboard: [domsecurity-active] → [domsecurity-active][post-critsmash-triage]
Depends on: 1498510
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.