Crash [@ nsJARChannel::OnStartRequest] EXEC and WRITE with wildptr addresses
Categories
(Core :: Networking: JAR, defect, P2)
Tracking
()
People
(Reporter: jesup, Assigned: valentin)
References
Details
(Keywords: crash, csectype-uaf, sec-high, Whiteboard: [startupcrash][necko-triaged][necko-priority-queue][post-critsmash-triage][adv-main108+r][adv-esr102.6+r])
Crash Data
Attachments
(1 file)
48 bytes,
text/x-phabricator-request
|
diannaS
:
approval-mozilla-beta+
RyanVM
:
approval-mozilla-esr102+
tjr
:
sec-approval+
|
Details | Review |
+++ This bug was initially created as a clone of Bug #237206 +++
Quite likely this is a different bug than the original one with this signature.
Updated•1 year ago
|
Assignee | ||
Comment 1•1 year ago
|
||
I'm still looking into this.
I'm thinking this is some kind of race in nsInputStreamPump, maybe with off main thread delivery?
Assignee | ||
Updated•1 year ago
|
Assignee | ||
Comment 2•1 year ago
|
||
Assignee | ||
Comment 3•1 year ago
•
|
||
Comment on attachment 9303670 [details]
Bug 1797370 - Do not access nsInputStream::mListener without holding mutex r=jesup
Security Approval Request
- How easily could an exploit be constructed based on the patch?: Not trivial. It's still not very clear how one would have caused the listener to change while the mutex was unlocked.
- 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?: all
- If not all supported branches, which bug introduced the flaw?: None
- Do you have backports for the affected branches?: Yes
- If not, how different, hard to create, and risky will they be?:
- How likely is this patch to cause regressions; how much testing does it need?: Low chance of regressions. Side effects would be an extra addref for each OnStart/Stop/Data call.
- Is Android affected?: Yes
Comment 4•1 year ago
|
||
Comment on attachment 9303670 [details]
Bug 1797370 - Do not access nsInputStream::mListener without holding mutex r=jesup
Approved to land and request uplift
Comment 5•1 year ago
|
||
Do not access nsInputStream::mListener without holding mutex r=jesup,necko-reviewers
https://hg.mozilla.org/integration/autoland/rev/4fd714ed786b44873c0b08b2072df586b8c1ecf1
https://hg.mozilla.org/mozilla-central/rev/4fd714ed786b
Assignee | ||
Comment 6•1 year ago
|
||
Comment on attachment 9303670 [details]
Bug 1797370 - Do not access nsInputStream::mListener without holding mutex r=jesup
Beta/Release Uplift Approval Request
- User impact if declined: Possible UAF
- Is this code covered by automated tests?: Yes
- 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): Avoids using mutex protected variable while the mutex is unlocked by putting variable into nsCOMPtr before unlocking. There should be no behaviour change at all.
- String changes made/needed:
- Is Android affected?: Unknown
Comment 7•1 year ago
|
||
Comment on attachment 9303670 [details]
Bug 1797370 - Do not access nsInputStream::mListener without holding mutex r=jesup
Approved for 108.0b7
Comment 8•1 year ago
|
||
uplift |
Comment 9•1 year ago
|
||
Please nominate this for ESR102 approval also.
Assignee | ||
Comment 10•1 year ago
|
||
Comment on attachment 9303670 [details]
Bug 1797370 - Do not access nsInputStream::mListener without holding mutex r=jesup
ESR Uplift Approval Request
- If this is not a sec:{high,crit} bug, please state case for ESR consideration:
- User impact if declined: Possible UAF
- Fix Landed on Version: 109
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): Avoids using mutex protected variable while the mutex is unlocked by putting variable into nsCOMPtr before unlocking. There should be no behaviour change at all.
Comment 11•1 year ago
|
||
Comment on attachment 9303670 [details]
Bug 1797370 - Do not access nsInputStream::mListener without holding mutex r=jesup
Approved for 102.6esr.
Comment 12•1 year ago
•
|
||
uplift |
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Updated•9 months ago
|
Description
•