1846694 - (CVE-2023-4576) Integer Overflow in RecordedSourceSurfaceCreation

Status: RESOLVED FIXED

(Core :: Graphics, defect, P2)

Description

[fffvr]

Attachment: patch.diff

An integer overflow occurs in RecordedSourceSurfaceCreation::RecordedSourceSurfaceCreation[1], which results in a heap buffer overflow[2].
ReturnWrite allows attacker to leak GPU process's heap data to the content process.

The provided patch.diff file simulates a compromised content process that may potentially cause the GPU process to crash. To reproduce the vulnerability, apply the patch and open the index.html.

[1] https://searchfox.org/mozilla-central/source/gfx/2d/RecordedEventImpl.h#3215
[2] https://searchfox.org/mozilla-central/source/gfx/layers/RecordedCanvasEventImpl.h#364

Comment 1

[fffvr]

Attachment: crash.asan

Comment 2

[fffvr]

Attachment: index.html

Comment 3

[Daniel Veditz [:dveditz]]

I can't tell from the the ASAN output that this is the GPU process crashing, but if it is then this could be a stepping stone to a sandbox escape. If it's crashing the same patched process then this wouldn't be valid. I'll rate it based on the reporter's claims but have not verified it.

Comment 4

[Tyson Smith [:tsmith]]

I was unable to reproduce on Linux with an ASan build.

Comment 5

[fffvr]

I apologize for the lack of information. This bug can only be triggered on windows.

Comment 6

[Kelsey Gilbert [:jgilbert]]

We just need CheckedInt<size_t> validation like here: https://searchfox.org/mozilla-central/rev/0816653c3ab851fa0e362eaec48c643fb764eaf4/dom/canvas/ImageData.cpp#56-60

Comment 7

[Brad Werth [:bradwerth]]

Attachment: Bug 1846694: Additional validation in RecordedSourceSurfaceCreation.

Comment 8

[Brad Werth [:bradwerth]]

Comment on attachment 9347266 [details]
Bug 1846694: Additional validation in RecordedSourceSurfaceCreation.

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Moderately. It's clear that the patch is validating size data sent in a stream, which implies that the size itself is the source of vulnerability.
• 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?: Easy to create.
• How likely is this patch to cause regressions; how much testing does it need?: Unlikely to cause regressions.
• Is Android affected?: Unknown

Comment 9

[Tom Ritter [:tjr]]

Comment on attachment 9347266 [details]
Bug 1846694: Additional validation in RecordedSourceSurfaceCreation.

Approved to land and uplift

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a3eb32145f15
Additional validation in RecordedSourceSurfaceCreation. r=gfx-reviewers,jrmuizel

Comment 11

[Brad Werth [:bradwerth]]

Comment on attachment 9347266 [details]
Bug 1846694: Additional validation in RecordedSourceSurfaceCreation.

Beta/Release Uplift Approval Request

• User impact if declined: Hijacked recordings could cause a leak of GPU heap to content process.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Fairly simple bounds checking by the deserializer that matches the existing bounds check done by the serializer.
• String changes made/needed:
• Is Android affected?: Unknown

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration:
• User impact if declined: Hijacked recordings could cause a leak of GPU heap to content process.
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Fairly simple bounds checking by the deserializer that matches the existing bounds check done by the serializer.

Comment 12

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

https://hg.mozilla.org/mozilla-central/rev/a3eb32145f15

Comment 13

[Dianna Smith [:diannaS] - on Leave, back on August 9th]

Comment on attachment 9347266 [details]
Bug 1846694: Additional validation in RecordedSourceSurfaceCreation.

This will be uplifted to 117 beta and will not be included in the next 116 dot release. If for any reason we do need to take this in the dot release, please feel free to NI me.

Comment 14

[Donal Meehan [:dmeehan]]

Comment on attachment 9347266 [details]
Bug 1846694: Additional validation in RecordedSourceSurfaceCreation.

Approved for 117.0b7

Comment 15

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/fe2aaa3aafcf

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9347266 [details]
Bug 1846694: Additional validation in RecordedSourceSurfaceCreation.

Approved for 115.2esr and 102.15esr.

Comment 17

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/bc14a71e04a2

Comment 18

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr102/rev/5528da6e9d7b

Comment 19

[June Wilde (she/her) [:jewilde]]

Attachment: advisory.txt