1322305 - Use-after-free in GMP process if GMPDecryptorChild actor destroyed before GMPVideoDecoderChild actor

Status: RESOLVED FIXED

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

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

The Widevine CDM gets a reference to the cdm::Host8 instance, and our implementation of this lives on the GMPDecryptorChild instance. We pass a pointer to this to the Widevine CDM.

However we also have a connection into the CDM via the GMPVideoDecoderChild instance. So if the GMPDecrytor IPC connection closes down before the GMPVideoDecoder IPC connection, the CDM can end up trying to talk to the Host stored on the free'd GMPDecryptorChild instance.

So the Widevine CDM ends up doing a use-after-free on the GMPDecryptorChild's vtable.

I'm pretty sure it's not possible for an attacker to write into that memory.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Ensure the CDMWrapper maintains a refcount on the GMPDecryptor, so it can't be freed before the CDM is destroyed

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8817071 [details] [diff] [review]
Ensure the CDMWrapper maintains a refcount on the GMPDecryptor, so it can't be freed before the CDM is destroyed

[Security approval request comment]
* How easily could an exploit be constructed based on the patch?

I think constructing an exploit would be hard in general, as it requires understanding our IPC system and how its actors correspond to other parts of our media stack.

I also don't see how an attacker could write over the memory stored at the vtable that we try to call. And this is a UAF in the GMP process, which is already sandboxed. So a sandbox escape would be needed to make this exploitable.


* 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?

In theory, Firefox 47 and later are affected. So release, beta, and aurora.


* If not all supported branches, which bug introduced the flaw?

N/A


* Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

The patch applies cleanly to beta, so I assume uplifting will be fine.


* How likely is this patch to cause regressions; how much testing does it need?

Low chance of regressions; this just ensures we keep an object alive for longer; until all clients of the object are done with it.

Comment 3

[Daniel Veditz [:dveditz]]

Comment on attachment 8817071 [details] [diff] [review]
Ensure the CDMWrapper maintains a refcount on the GMPDecryptor, so it can't be freed before the CDM is destroyed

sec-approval=dveditz

Comment 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ff8e8c79f02902e3e5e6075736d2bd1b0da2f41f
Bug 1322305. r=gerald

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 6

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8817071 [details] [diff] [review]
Ensure the CDMWrapper maintains a refcount on the GMPDecryptor, so it can't be freed before the CDM is destroyed

Approval Request Comment
[Feature/Bug causing the regression]:
[User impact if declined]: Potential sec-moderate exploit in sandboxed GMP process.
[Is this code covered by automated tests?]: Yes; external-media-tests Netflix tests.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: No; it's hard to repro.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: It just makes an object in the GMP process live longer.
[String changes made/needed]: None.

Comment 7

[Gerry Chang [:gchang]]

Comment on attachment 8817071 [details] [diff] [review]
Ensure the CDMWrapper maintains a refcount on the GMPDecryptor, so it can't be freed before the CDM is destroyed

Fix a sec-moderate. Beta51+ & Aurora52+. Should be in 51 beta 10.

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7753f144223e

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/83e4a4618cbb