Closed Bug 1544181 (CVE-2020-6822) Opened 2 years ago Closed 9 months ago

Potential out-of-bounds write in GMPDecodeData

Categories

(Core :: WebRTC, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla76
Tracking Status
firefox-esr68 75+ fixed
firefox73 --- wontfix
firefox74 - wontfix
firefox75 + fixed
firefox76 + fixed

People

(Reporter: deian, Assigned: dminor)

Details

(Keywords: sec-moderate, Whiteboard: [post-critsmash-triage][adv-main75+][adv-esr68.7+])

Attachments

(2 files, 1 obsolete file)

I believe there is a potential out-of-bounds write in GMPDecodeData [1]:

  GMPDecodeData(const webrtc::EncodedImage& aInputImage, bool aMissingFrames,
                int64_t aRenderTimeMs)
      : mImage(aInputImage),
        mMissingFrames(aMissingFrames),
        mRenderTimeMs(aRenderTimeMs) {
    // We want to use this for queuing, and the calling code recycles the
    // buffer on return from Decode()
    mImage._length = aInputImage._length;
    mImage._size =
        aInputImage._length +
        webrtc::EncodedImage::GetBufferPaddingBytes(webrtc::kVideoCodecH264); // <-- pick huge enough _length such that _length + 8 overflows
    mImage._buffer = new uint8_t[mImage._size];                               // <-- allocate array smaller than _length
    memcpy(mImage._buffer, aInputImage._buffer, aInputImage._length);         // <-- copy _length bytes

I'm not sure if this is easy to trigger or if it's really any more serious than
a crash, but seemed worth reporting. Apologies if this turns out to not be a
concern.

[1] https://searchfox.org/mozilla-central/source/media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.h#129-134

Group: firefox-core-security → core-security
Component: General → WebRTC
Product: Firefox → Core
Group: core-security → media-core-security

a) could only apply to 32-bit builds (but that's valid)
b) _length and the buffer it points to would have to be an actual memory buffer of ~4GB (which is very unlikely to be possible)
c) encoded input images are very likely constrained in size before this point (to way less than 4G), but that can be checked
d) adding a backup error-out-on-too-big (or release-assert) here wouldn't hurt even if c) is correct that this can't happen currently.

Priority: -- → P3
Assignee: nobody → dminor
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment on attachment 9129110 [details]
Bug 1544181 - Check for large frames in GMPDecodeData; r=ng!

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: Not easily.
  • 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?: Attached patch should apply cleanly.
  • How likely is this patch to cause regressions; how much testing does it need?: Very unlikely, this adds an assertion for a condition we're unlikely to ever hit.
Attachment #9129110 - Flags: sec-approval?

Comment on attachment 9129110 [details]
Bug 1544181 - Check for large frames in GMPDecodeData; r=ng!

Approved to land and for uplift.

Attachment #9129110 - Flags: sec-approval? → sec-approval+
Attachment #9129110 - Flags: approval-mozilla-esr68+
Attachment #9129110 - Flags: approval-mozilla-beta+

Changing the priority to p1 as the bug is tracked by a release manager for the current beta.
See What Do You Triage for more information

Priority: P3 → P1
Group: media-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla76
Flags: qe-verify-
Whiteboard: [post-critsmash-triage]
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main75+]
Whiteboard: [post-critsmash-triage][adv-main75+] → [post-critsmash-triage][adv-main75+][adv-esr68.7+]

@Tom can you give Fraser Brown credit too :-) She's the real hero that built the tool that found this bug.

Alias: CVE-2020-6822
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.