UAF in H264 encoder shutdown in VideoSendStreamImpl::OnEncodedImage
Categories
(Core :: WebRTC: Audio/Video, defect, P1)
Tracking
()
People
(Reporter: pehrsons, Assigned: pehrsons)
References
Details
(Keywords: crash, csectype-uaf, sec-high, Whiteboard: [sec-survey][adv-main93+r][adv-esr78.15+r][adv-esr91.2+r])
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-esr78+
RyanVM
:
approval-mozilla-esr91+
dveditz
:
sec-approval+
|
Details |
See below the description for bug 1417797 which was the same bug but for the GmpH264 decoder. I have not seen any proof of this bug on crash-stats, but I have triggered it with the latest (WIP) libwebrtc update (bug 1654112). See the exhaustive analysis in pernosco.
+++ This bug was initially created as a clone of Bug #1417797 +++
During shutdown of a PeerConnection (in this case navigating away from http://mixer.com/channelone to see other channels), it's possible that the "external" H264 decoder is released and destroyed while a frame decode is still pending from the GMP decoder, which is asynchronous.
Fairly certain this affects all current versions.
Updated•3 years ago
|
Assignee | ||
Comment 1•3 years ago
|
||
Assignee | ||
Comment 2•3 years ago
|
||
Comment on attachment 9238690 [details]
Bug 1728321 - Clean up encode callback during shutdown. r?ng!
Security Approval Request
- How easily could an exploit be constructed based on the patch?: Seems hard. One can deduce quite a bit but we have no hard proof of this race condition being an issue in the wild.
- 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?: all
- 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?: trivial, might apply cleanly even
- How likely is this patch to cause regressions; how much testing does it need?: Very unlikely. We've already made the same change for the decoder and that was fine.
Comment 3•3 years ago
|
||
Comment on attachment 9238690 [details]
Bug 1728321 - Clean up encode callback during shutdown. r?ng!
sec-approval = dveditz
![]() |
||
Comment 4•3 years ago
|
||
Clean up encode callback during shutdown. r=ng
https://hg.mozilla.org/integration/autoland/rev/4e0617841a37c8ce6c7952d0ff92c026ac262a1f
https://hg.mozilla.org/mozilla-central/rev/4e0617841a37
Updated•3 years ago
|
Assignee | ||
Comment 5•3 years ago
|
||
Comment on attachment 9238690 [details]
Bug 1728321 - Clean up encode callback during shutdown. r?ng!
ESR Uplift Approval Request
- If this is not a sec:{high,crit} bug, please state case for ESR consideration:
- User impact if declined: No proof of crashes in the wild, but there's a theoretical risk of use-after-free.
- Fix Landed on Version: 93
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): This is mirroring a previous change made to the decoder for an identical issue (bug 1417797).
- String or UUID changes made by this patch:
Comment 6•3 years ago
|
||
Comment on attachment 9238690 [details]
Bug 1728321 - Clean up encode callback during shutdown. r?ng!
Approved for 91.2esr and 78.15esr.
Comment 7•3 years ago
|
||
uplift |
Comment 8•3 years ago
|
||
As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.
Please visit this google form to reply.
Assignee | ||
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•2 years ago
|
Description
•