d4aabfbeb55f80f7c2a1fcd4a626805d97c35a75 MediaPlayer::attemptSniffAndReload() frees the media engine outside the audio process lock
Triage note: Bracketed by the RC_REMOVE_SEC_ACCEL_3 macro (strong silent-security-fix signal): adds setPlayerPrivate() asserting main-thread/initializing-engine, and in attemptSniffAndReload replaces m_private = nullptr with cancelLoad() because 'Clearing m_private here would free the engine while the audio render thread reads it in audioSourceProvider()'. Also adds a stale-settled-promise guard (if (!m_sniffer) return). Concrete cross-thread use-after-free on the media engine, reachable via MediaElementAudioSourceNode.
The bug at a glance
WebCore MediaPlayer cross-thread use-after-free. m_private (the RefPtr that is the live media engine) is read on the audio render thread through audioSourceProvider(), while MediaPlayer::attemptSniffAndReload’s completion callback cleared it with m_private = nullptr on the main thread. Because this MediaPlayer holds the only reference, that assignment destroys the engine out from under the audio render thread -> a concrete cross-thread UAF, reachable from the web via MediaElementAudioSourceNode routing a media element into Web Audio. The patch replaces the clear with cancelLoad(), funnels all m_private writes through a main-thread/process-lock-guarded setPlayerPrivate(), and drops a stale settled-promise callback. High UAF is well supported by the diff’s own comments and lifetime reasoning.
A sniff-and-reload callback freed the media engine as a simple RefPtr clear, ignoring that the audio render thread concurrently reads the same pointer via audioSourceProvider().
Root cause
The invariant is that m_private may only be replaced on the main thread while MediaElementAudioSourceNode’s process lock is held (the window between mediaPlayerWillInitializeMediaEngine() and mediaPlayerDidInitializeMediaEngine()), because the audio render thread dereferences m_private through audioSourceProvider() concurrently. MediaPlayer is ThreadSafeRefCounted, but m_private itself is a single-owner RefPtr whose target (MediaPlayerPrivateInterface) is kept alive only by that one reference.
Pre-patch sequence: 1. A load fails the engine fallback chain; attemptSniffAndReload fetches a prefix and, on promise resolution, runs a callback on the main thread. 2. The callback sets protectedThis->m_private = nullptr directly. 3. Since m_private held the only ref, deref drops the engine’s refcount to zero and destroys the MediaPlayerPrivateInterface. 4. Concurrently the audio render thread is inside audioSourceProvider() reading m_private / the engine object -> it dereferences freed memory (cross-thread UAF). A second latent bug: the promise callback could fire after a newer load already cleared m_sniffer via cancelSniffer(), so a stale settled promise delivered results for a load that is no longer current.
The fix makes three changes. (a) It adds setPlayerPrivate(RefPtr&&) which ASSERT(isMainThread()) and ASSERT(m_initializingMediaEngine) and performs the actual m_private = move assignment, and routes every loadWithNextMediaEngine write (nullptr, the new engine, and NullMediaPlayerPrivate::create) through it so the engine is only ever swapped inside the initializing window that holds the process lock. (b) In attemptSniffAndReload it replaces protectedThis->m_private = nullptr with protectedThis->cancelLoad(), whose comment states clearing m_private there would free the engine while the audio render thread reads it; loadWithNextMediaEngine() then replaces m_private under the process lock instead. (c) It adds an early if (!protectedThis->m_sniffer) return; so an already-settled promise whose load was superseded (m_sniffer cleared by cancelSniffer) does not run against stale state. The header is annotated that m_private is read on the audio render thread and may be written only through setPlayerPrivate.
Key code
Sniff-reload no longer frees the engine on the main thread; writes funnel through a guarded setter
// attemptSniffAndReload completion (fixed branch):
if (!protectedThis->m_sniffer)
return;
...
// Clearing m_private here would free the engine while the audio render thread reads it in
// audioSourceProvider(). loadWithNextMediaEngine() replaces it under the process lock.
protectedThis->cancelLoad();
protectedThis->loadWithNextMediaEngine(nullptr);
// new guarded writer:
void MediaPlayer::setPlayerPrivate(RefPtr<MediaPlayerPrivateInterface>&& playerPrivate)
{
ASSERT(isMainThread());
ASSERT(m_initializingMediaEngine);
m_private = WTF::move(playerPrivate);
}
Fix approach
The patch establishes a single writer discipline for m_private: a new setPlayerPrivate() asserts it runs on the main thread during the m_initializingMediaEngine window (which holds MediaElementAudioSourceNode’s process lock, i.e. the audio render thread is excluded), and every assignment in loadWithNextMediaEngine goes through it. In the sniff-and-reload callback the dangerous main-thread free (m_private = nullptr) is replaced by cancelLoad(), deferring the actual engine swap to loadWithNextMediaEngine() under the lock; the engine is thus never destroyed while the render thread could be reading it. A stale-promise guard (if (!m_sniffer) return;) drops callbacks for superseded loads. Net semantic change: m_private transitions are serialized against the audio render thread by the process lock rather than happening ad hoc on the main thread.
Patch walkthrough
Source/WebCore/platform/graphics/MediaPlayer.cpp— Adds setPlayerPrivate() with main-thread/initializing-engine asserts; routes the three loadWithNextMediaEngine m_private writes (nullptr, created engine, NullMediaPlayerPrivate) through it; in attemptSniffAndReload replaces m_private = nullptr with cancelLoad() and adds theif (!m_sniffer) return;stale-promise guard.Source/WebCore/platform/graphics/MediaPlayer.h— Declares setPlayerPrivate(RefPtr&&) and documents that m_private is read on the audio render thread via audioSourceProvider() and must be written only through setPlayerPrivate().
Background
MediaPlayerPrivateInterface (m_private) — The concrete media engine backing a MediaPlayer; held by a single RefPtr m_private, so clearing that RefPtr can be the sole reference and destroy the engine.
audioSourceProvider() — Supplies audio samples to Web Audio from the media engine; it runs on the real-time audio render thread and reads m_private concurrently with the main thread.
MediaElementAudioSourceNode / process lock — The Web Audio node that pipes a media element into the audio graph; its process lock serializes engine changes against the audio render thread and is held across the mediaPlayerWillInitializeMediaEngine/DidInitializeMediaEngine window.
attemptSniffAndReload — On exhausting the engine fallback chain, fetches a short prefix of the resource to determine the real Content-Type and reloads with a better engine; its completion is an async promise callback.
cancelLoad / cancelSniffer / m_sniffer — cancelLoad tears down the current load safely; cancelSniffer clears m_sniffer when a new load starts, so a still-pending sniff promise can fire for a load that is no longer current.
Vulnerability window
- Setup — A media element is routed into Web Audio via MediaElementAudioSourceNode, so the audio render thread reads m_private through audioSourceProvider().
- Trigger — The initial engine fails format/decode; attemptSniffAndReload fetches a prefix and schedules a completion callback.
- Free — Pre-patch the callback runs m_private = nullptr on the main thread, destroying the only-referenced media engine.
- Use — Concurrently the audio render thread dereferences the freed engine inside audioSourceProvider() -> cross-thread UAF.
- Mitigated — Post-patch cancelLoad defers the swap and loadWithNextMediaEngine replaces m_private under the process lock via setPlayerPrivate.
Proof of concept
No regression test ships. Conceptual trigger (web path): create an / element whose resource initially resolves to an engine that fails the fallback chain (format/decode error) so attemptSniffAndReload runs, and connect it through a MediaElementAudioSourceNode into an active AudioContext so the audio render thread is actively calling audioSourceProvider(). When the sniff completion fires on the main thread and clears m_private, the render thread’s concurrent read of the freed engine is the UAF. Reliability depends on the element being actively pulled by the audio graph at the instant of the reload callback.
Exploitation
- Reach — Pure web content: a media element plus Web Audio MediaElementAudioSourceNode, both scriptable, drive the vulnerable sniff-and-reload path in WebContent.
- Trigger — Force engine fallback exhaustion to invoke attemptSniffAndReload while the audio graph actively consumes the element.
- Primitive — Use-after-free: the audio render thread reads/calls through a MediaPlayerPrivateInterface freed by the main-thread m_private = nullptr.
- Reclaim (inferred) — Escalation past a crash requires reclaiming the freed engine allocation with controlled bytes and steering a virtual call made by the render thread through audioSourceProvider(); the cross-thread timing window is attacker-tunable by load scheduling but reclaim control is not shown.
- Scope — Corruption occurs in the WebContent/renderer process on the real-time audio thread, a vtable-dispatch-rich target that favors control-flow hijack if reclaim succeeds.
Detection & hunting
For defenders and detection engineers:
- m_private cleared off the init window —
- UAF in audioSourceProvider —
- Stale sniff promise —
Remaining risk in the patch
- Assert-only thread guard — setPlayerPrivate enforces the main-thread/initializing-engine precondition via ASSERT (debug), not a release-build gate; correctness in release relies on all callers genuinely holding the process lock during the init window, which the diff asserts but cannot prove for every caller.
- audioSourceProvider body not shown — The exact read of m_private on the audio render thread lives in audioSourceProvider(), which is not in the diff, so the precise race window and whether it takes the same process lock is (inferred) from the code comments.
- cancelLoad completeness — The fix depends on cancelLoad() fully quiescing the prior engine before loadWithNextMediaEngine swaps m_private; cancelLoad’s body is not in the diff, so residual interactions are unverified.
Audit directions
- Other m_private writers — Grep MediaPlayer for any remaining direct m_private assignments not routed through setPlayerPrivate, especially in error/teardown paths.
- Cross-thread media state — Audit other fields read by the audio render thread (sample providers, output device state) for main-thread mutation without the process lock.
- Settled-promise callbacks — Review other async media callbacks for stale-load guards equivalent to
if (!m_sniffer) return;after cancelSniffer/cancelLoad.