1854669 - (CVE-2023-6860) VideoBridge allows any content process to use any texture produced by remote decoder

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Andrew Osmond [:aosmond] (he/him)]

The compositor process uses SurfaceDescriptorRemoteDecoder, which contains a source ID and handle, to link a TextureHost created by a helper process (RDD, media utility, GPU process) to a TextureHost created by a content process during display list building:
https://searchfox.org/mozilla-central/rev/48b6992e03fa66f77ac9688ba61c95d31a451bc1/gfx/layers/composite/TextureHost.cpp#340

As you can follow from the code above, we just accept the opaque source/handle without any additional validation.

The source ID is merely which utility process created it, not the process that requested it. We decide which VideoBridgeParent that maps to here:
https://searchfox.org/mozilla-central/rev/48b6992e03fa66f77ac9688ba61c95d31a451bc1/gfx/layers/composite/GPUVideoTextureHost.cpp#41

The handle is the unique ID associated with that particular texture for that utility process:
https://searchfox.org/mozilla-central/rev/077fc34d03b85b09add26b5f99f1a3a3a72c8720/gfx/layers/composite/GPUVideoTextureHost.cpp#47

Nothing stops a compromised content process from using an arbitrary ID. In fact, because we race between the content process creating its own wrapper texture and the utility process creating the actual texture, we gracefully handle the scenario where there is no texture available yet to map to.

We should have some sort of token validation here, where the utility process generates namespace IDs for each content process, shares that with the compositor process, and only allows wrapper textures created by the owning content process to wrap the actual texture from the VideoBridge. This would be similar to how we validate graphics resources in the WebRender display pipeline.

Comment 1

[Andrew Osmond [:aosmond] (he/him)]

I don't have any POC using this as an exploit but in theory, it wouldn't be that hard to construct after escaping the content process sandbox. What one could do with this, I'm not sure, but I'm looking at using the same infrastructure with WebGL to avoid copies into the content process and back into the compositor process when using GPUVideoImage for CPU memory backed textures. This is showing up in profiles with WebCodecs. In theory this will also be useful for DrawTargetWebgl / accelerated canvas (and D2D canvas if necessary/desired). Expanding the attack surface without fixing this is probably not good.

Comment 2

[Andrew Osmond [:aosmond] (he/him)]

I haven't checked, but it is possible there is a similar bug on the audio side.

Comment 3

[Andrew Osmond [:aosmond] (he/him)]

This isn't a problem on the readback path directly into the content process because it can only access textures it created:
https://searchfox.org/mozilla-central/rev/48b6992e03fa66f77ac9688ba61c95d31a451bc1/dom/media/ipc/RemoteDecoderManagerParent.cpp#285

Comment 4

[Daniel Veditz [:dveditz]]

Does comment 3 contradict comment 0 where you said

Nothing stops a compromised content process from using an arbitrary ID.

Or is comment 3 limited to just that one kind of use, and other APIs are not policed in the same way?

Is there more to a texture than just the graphics that get displayed (or audio that could be heard)? polluting images and such would be cross-origin data sharing trouble but not a Firefox compromise. But if there are other values associated that could be changed using the same ID then maybe that could get another process to do something bad.

Comment 5

[Andrew Osmond [:aosmond] (he/him)]

(In reply to Daniel Veditz [:dveditz] from comment #4)

Does comment 3 contradict comment 0 where you said

Nothing stops a compromised content process from using an arbitrary ID.

Or is comment 3 limited to just that one kind of use, and other APIs are not policed in the same way?

Is there more to a texture than just the graphics that get displayed (or audio that could be heard)? polluting images and such would be cross-origin data sharing trouble but not a Firefox compromise. But if there are other values associated that could be changed using the same ID then maybe that could get another process to do something bad.

The readback path is used in the content process, so comment 3 is referring to the fact that an arbitrary content process cannot readback a GPUVideoImage created for another content process.

Right now then, an arbitrary content process can only use an arbitrary GPUVideoImage on the rendering pipeline. I have patches that would like to allow canvas replays and WebGL in the compositor process to access an arbitrary GPUVideoImage, but it needs to be accessed controlled. Hence why I filed this bug first.

Comment 6

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1854669.

Comment 7

[Andrew McCreight [:mccr8]]

Andrew said that this could potentially allow a content process to inject video as though it was coming from another content process (but not read what the other process was doing), if the attacker page has already taken over the content process. So you could be watching a video on YouTube, but suddenly there's (extra?) video from the attacker page. That's sort of like a URL bar spoof, but the attacker can't read anything, so not really. YouTube probably isn't a great example because people probably don't have a ton of expectation for the validity of anything they watch (hopefully) but I guess if you are watching a video on a government webpage and suddenly it starts telling you to buy gift cards or whatever that might be confusing.

Comment 8

[Andrew Osmond [:aosmond] (he/him)]

They couldn't exploit the audio this way, since it has a different codepath. But yes, in theory an exploited content process could display content intended for another unexploited content process.

Comment 9

[Andrew McCreight [:mccr8]]

Dan, does sec-moderate seem reasonable for this bug or should it be higher? Thanks.

Comment 10

[Daniel Veditz [:dveditz]]

wfm. It's not really a sandbox "escape", more like a leak.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/774cda5d71b6
r=gfx-reviewers,lsalzman

Comment 12

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

https://hg.mozilla.org/mozilla-central/rev/774cda5d71b6

Comment 13

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

The patch landed in nightly and beta is affected.
:aosmond, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox121 to wontfix.

For more information, please visit BugBot documentation.

Comment 14

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1854669.

Original Revision: https://phabricator.services.mozilla.com/D194537

Comment 15

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 9367320 [details]
Bug 1854669.

Beta/Release Uplift Approval Request

• User impact if declined: Sec issue
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• 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): It touches a lot of files but it is almost entirely boilerplate. The actual changes are trivial.
• String changes made/needed:
• Is Android affected?: Yes

Comment 16

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 9365224 [details]
Bug 1854669.

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Sec issue
• User impact if declined:
• Fix Landed on Version: 122
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Mostly boilerplate changes, actual functional change is trivial to enforce access restrictions

Comment 17

[Ryan VanderMeulen [:RyanVM][PTO June 24-28]]

Comment on attachment 9365224 [details]
Bug 1854669.

Approved for 121.0b9 and 115.6esr.

Comment 18

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/651699e8abfb

Comment 19

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/78d4bed89462

Comment 20

[Jason Kratzer [:jkratzer]]

Attachment: advisory.txt

Comment 21

[Daniel Veditz [:dveditz]]

Bulk-unhiding security bugs fixed in Firefox 119-121 (Fall 2023). Use "moo-doctrine-subsidy" to filter