Double free inside nsZipHandle upon FindDataStart failure
Categories
(Core :: Networking: JAR, defect, P1)
Tracking
()
People
(Reporter: bas.schouten, Assigned: bas.schouten)
References
(Regression)
Details
(Keywords: regression, sec-moderate, Whiteboard: [necko-triaged][post-critsmash-triage][adv-main76+r][adv-ESR68.8+r])
Crash Data
Attachments
(1 file)
47 bytes,
text/x-phabricator-request
|
jcristau
:
approval-mozilla-esr68+
dveditz
:
sec-approval+
|
Details | Review |
https://hg.mozilla.org/releases/mozilla-release/annotate/8260da04c9b13f7c0e9cc6984a75e689b5fcb8c8/modules/libjar/nsZipArchive.cpp#l217 de-allocated the map, however at that point it has already been set on the handle object, handle->mMap and handle->mFileStart are not dangling pointers.
In ~nsZipHandle it will proceed to attempt to de-allocate the memory and trigger a double free.
Assignee | ||
Comment 1•2 years ago
|
||
Double free sounds like a security issue
Comment 3•2 years ago
|
||
It can be a problem, but I think in this case there's a very small window between failure and the deconstructor so it'd be a hard race to win reliably. Calling it sec-moderate for now.
Assignee | ||
Comment 4•2 years ago
|
||
Comment on attachment 9122942 [details]
Bug 1611482: Prevent the existance of dangling pointers upon failure of FindDataStart. r=spohl
Security Approval Request
- How easily could an exploit be constructed based on the patch?: Window is pretty small as Daniel mentioned. And it's not even clear to me what triggers this situation. It's based on code analysis and crash stats.
- 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 I believe
- If not all supported branches, which bug introduced the flaw?: None
- Do you have backports for the affected branches?: No
- If not, how different, hard to create, and risky will they be?: Low risk, this patch should apply.
- How likely is this patch to cause regressions; how much testing does it need?: Not really.
Updated•2 years ago
|
Updated•2 years ago
|
Comment 5•2 years ago
|
||
Comment on attachment 9122942 [details]
Bug 1611482: Prevent the existance of dangling pointers upon failure of FindDataStart. r=spohl
It's too late to get into next week's releases, but looking at the crash volume we'll want this fix on ESR for next time too.
![]() |
||
Comment 6•2 years ago
|
||
https://hg.mozilla.org/integration/autoland/rev/0c0f77ca0a7eccbbb91a45df7ee63d687fac88ee
https://hg.mozilla.org/mozilla-central/rev/0c0f77ca0a7e
Comment 7•2 years ago
|
||
Please nominate this for ESR68 approval when you get a chance.
Assignee | ||
Comment 8•2 years ago
|
||
Comment on attachment 9122942 [details]
Bug 1611482: Prevent the existance of dangling pointers upon failure of FindDataStart. r=spohl
ESR Uplift Approval Request
- If this is not a sec:{high,crit} bug, please state case for ESR consideration: Sec moderate, but low risk and an easy fix to take.
- User impact if declined: Potential UAF
- Fix Landed on Version: 77
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): Simply removes a dangling pointer.
- String or UUID changes made by this patch: None.
Comment 9•2 years ago
|
||
Comment on attachment 9122942 [details]
Bug 1611482: Prevent the existance of dangling pointers upon failure of FindDataStart. r=spohl
approved for 68.8esr
Comment 10•2 years ago
|
||
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Description
•