Closed Bug 1728321 Opened 3 years ago Closed 3 years ago

UAF in H264 encoder shutdown in VideoSendStreamImpl::OnEncodedImage

Categories

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

defect

Tracking

()

RESOLVED FIXED
93 Branch
Tracking Status
firefox-esr78 93+ fixed
firefox-esr91 93+ fixed
firefox91 --- wontfix
firefox92 --- wontfix
firefox93 + fixed

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)

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.

Group: core-security → media-core-security

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.
Attachment #9238690 - Flags: sec-approval?

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

sec-approval = dveditz

Attachment #9238690 - Flags: sec-approval? → sec-approval+
Group: media-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 93 Branch

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:
Attachment #9238690 - Flags: approval-mozilla-esr91?
Attachment #9238690 - Flags: approval-mozilla-esr78?

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

Approved for 91.2esr and 78.15esr.

Attachment #9238690 - Flags: approval-mozilla-esr91?
Attachment #9238690 - Flags: approval-mozilla-esr91+
Attachment #9238690 - Flags: approval-mozilla-esr78?
Attachment #9238690 - Flags: approval-mozilla-esr78+

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.

Flags: needinfo?(apehrson)
Whiteboard: [sec-survey]
Flags: needinfo?(apehrson)
Flags: qe-verify-
Whiteboard: [sec-survey] → [sec-survey][adv-main93+r]
Whiteboard: [sec-survey][adv-main93+r] → [sec-survey][adv-main93+r][adv-esr78.15+r]
Whiteboard: [sec-survey][adv-main93+r][adv-esr78.15+r] → [sec-survey][adv-main93+r][adv-esr78.15+r][adv-esr91.2+r]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: