1728321 - UAF in H264 encoder shutdown in VideoSendStreamImpl::OnEncodedImage

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, defect, P1)

Description

[Andreas Pehrson [:pehrsons] (PTO; back in Jan)]

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.

Comment 1

[Andreas Pehrson [:pehrsons] (PTO; back in Jan)]

Attachment: Bug 1728321 - Clean up encode callback during shutdown. r?ng!

Comment 2

[Andreas Pehrson [:pehrsons] (PTO; back in Jan)]

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

[Daniel Veditz [:dveditz] back January 5, 2026]

Comment on attachment 9238690 [details]
Bug 1728321 - Clean up encode callback during shutdown. r?ng!

sec-approval = dveditz

Comment 4

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Clean up encode callback during shutdown. r=ng
https://hg.mozilla.org/integration/autoland/rev/4e0617841a37c8ce6c7952d0ff92c026ac262a1f
https://hg.mozilla.org/mozilla-central/rev/4e0617841a37

Comment 5

[Andreas Pehrson [:pehrsons] (PTO; back in Jan)]

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

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9238690 [details]
Bug 1728321 - Clean up encode callback during shutdown. r?ng!

Approved for 91.2esr and 78.15esr.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/951862a51fd6
https://hg.mozilla.org/releases/mozilla-esr78/rev/5a16b1e04304

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

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.