This bug was filed from the Socorro interface and is report bp-5597f645-ecac-4afb-a2c3-2160e2160312. ============================================================= Currently at #6 in Top Crashers for Firefox 46.0b. Potentially exploitable: Crash Reason EXCEPTION_ACCESS_VIOLATION_READ Crash Address 0xffffffffe5e5e5e5 All crashes are on Windows: Operating System Percentage Windows 7 53.66% Windows XP 39.02% Windows Vista 4.07% Windows 10 1.63% Windows 8.1 1.63% Seems to have started in v46: Product Version Percentage Firefox 47.0a1 58.54% Firefox 46.0a2 18.70% Firefox 48.0a1 15.45% Firefox 45.0a2 3.25% Firefox 47.0a2 2.44% Firefox 45.0a1 0.81% Firefox 46.0b1 0.81% Stack: mozilla::GMPVideoDecoder::GMPInitDone(GMPVideoDecoderProxy*, GMPVideoHost*) mozilla::gmp::GetGMPContentParentForVideoDecoderDone::Done(mozilla::gmp::GMPContentParent*) mozilla::gmp::RunCreateContentParentCallbacks::Run() nsThread::ProcessNextEvent(bool, bool*) NS_ProcessNextEvent(nsIThread*, bool) mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) MessageLoop::RunHandler() MessageLoop::Run() nsThread::ThreadFunc(void*) _PR_NativeRunThread

Note that an older bug 1203763 with the same signature resolved as WFM might be related.

[Tracking Requested - why for this release]: This is 2.7% of all Firefox 46.0b1 crashes, and therefore by far a topcrash.

This has the same signature as Bug 1203763. This is not OpenH264; it's playback (EME). Chris -- This is marked as a regression, blocking Fx46 release and a top crasher. Can you take it and drive this to a fix? Or find someone on your team to drive this to a fix? Thanks.

Chris -- FYI: Since this *may* be related to bug 1256064 (and it's also a regression that's blocking release), I made you the owner for both bugs to drive these forward. Thanks.

Sec-critical, recent regression, tracking

#3 topcrash in beta 46 so far. I agree, being the very top crashes and sec-critical should probably make this a release blocker.

There are two problems here, caused by races in shutdown. The first problem here is that GMP{Audio,Video}Decoder::mInitPromise is set to null in GMP{Audio,Video}Decoder::Shutdown() after it's rejected. So if the GMP decoder is shutdown after Init() is called, but before GMPInitDone() is called, mInitPromise can be null, and that's how we get the null pointer deref on mInitPromise. The other case we see is the UAF in GMP{Audio,Video}Decoder::mConfig in GMPInitDone. This is because mConfig is a const reference to a struct probably held by the MediaFormatReader or demuxer, which must have been freed. We can just make a copy of the config.

(In reply to Chris Pearce (:cpearce) from comment #7) > There are two problems here, caused by races in shutdown. Note: I mean shutdown of the decoder, not shutdown of Firefox.

* Check whether GMP{Audio,Video}Decoder::mInitPromise is empty and bail if so in GMPInitDone. This prevents null pointer deref. * Make a copy of the MediaInfo passed to GMP{Audio,Video}Decoder, rather than keeping a reference to it. This means we can't get a UAF if the thing that owns the MediaInfo is destroyed before the GMP decoder is. That can happen because the callback object maintains a reference to the GMP decoder while we wait for a GMPInitDone callback.

Comment on attachment 8730519 [details] [diff] [review] Patch [Security approval request comment] How easily could an exploit be constructed based on the patch? I think it could be hardish. Attacker would need to construct an MP4 file with valid audio/video stream metadata that populates the MediaInfo/AudioInfo/VideoInfo structs to create valid executable code, and then if unencrypted decoding using GeckoMediaPlugins is enabled, play the video inside a regular non-EME <video> element and spam shutting down and starting up. Unencrypted decoding is enabled in Firefox 46 and later. In theory an attacker could load this video inside an EME <video> as well, though we have no crash reports that I can find proving this is possible. We only have crash reports for the unencrypted video case AFAICT. EME is enabled in Firefox 42 and later. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? I think a skilled attacker could work backwards from the class/struct names to figure out what the flaw was. Figuring out that they could exploit this using unencrypted decoding and/or ClearKey EME would be a bit of a leap. Which older supported branches are affected by this flaw? 46 is affected in the unencrypted video decoding case. 45 and ESR45 in theory are affected in the EME video case, though we've not seen any crash reports in the EME case. 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? Not yet. We need Release 45. ESR45, and Aurora, Beta, Central. Should be easy enough. How likely is this patch to cause regressions; how much testing does it need? Not likely to cause regressions. Basically adding null-check type guards. I'm not 100% sure I fixed all the cases, since I can't repro, so testing this not-too-late in the beta cycle would be good. We can also land this in inconspicuously in bug 1256064 to make it less visible that it's an exploitable bug.

sec-approval+ for trunk. Please make and nominate branch patches as well.

https://hg.mozilla.org/integration/mozilla-inbound/rev/49aa9281700cdacc521a7fa851abfc019831f1eb Bug 1256065. r=gerald

Remember to request uplift.

Comment on attachment 8730519 [details] [diff] [review] Patch [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: User impact if declined: sec-crit. Fix Landed on Version: 48, mozilla-central. Risk to taking this patch (and alternatives if risky): Fairly low, basically adding (if shutdown, bail) checks. String or UUID changes made by this patch: None. See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info. Approval Request Comment [Feature/regressing bug #]: Unencrypted decoding via Adobe GMP, and EME playback [User impact if declined]: Crashes when using unencrypted decoding via Adobe EME plugin, and potentially if using regular EME. [Describe test coverage new/current, TreeHerder]: Test included, and we have lots of EME mochitest, and specific test for unencrypted decoding via GMP. [Risks and why]: Fairly low, basically adding (if shutdown, bail) checks. [String/UUID change made/needed]: None.

Comment on attachment 8730519 [details] [diff] [review] Patch Sec-crit, Aurora47+, Beta46+, ESR45.1.0+

https://hg.mozilla.org/releases/mozilla-aurora/rev/c4a154a87655 https://hg.mozilla.org/releases/mozilla-beta/rev/48ca64d2b5b1 https://hg.mozilla.org/releases/mozilla-esr45/rev/ce0f864c5ecd

No crashes reported in builds built roughly after when this landed. Looking good so far...