9d4cf39edf715c164e25ec9d3bf58f3bab47906e  [MSE] Track change callbacks on VideoTrackPrivate/AudioTrackPrivate can be destroyed while they are running

The bug at a glance

AudioTrackPrivate::setEnabled() and VideoTrackPrivate::setSelected() invoked a plain Function member (m_enabledChangedCallback / m_selectedChangedCallback) with an unsynchronized check-then-call, while setEnabledChangedCallback/setSelectedChangedCallback could replace (and thus destroy) that Function from another thread. This is a cross-thread data race on a heap-allocated closure that is provably a use-after-free of the Function’s captured state when a setter destroys the old callback during an in-flight invocation (high severity for a media-pipeline UAF is supported). Escalation from the UAF to controlled code execution is (inferred); the diff proves the lifetime/race defect, not a specific exploit primitive.

A one-liner setter (m_cb = WTF::move(callback)) paired with an unguarded if (m_cb) m_cb(...) invocation - the callback destroyed on one thread exactly while another thread is calling through it.

Root cause

The violated invariant is that a Function must not be destroyed while another thread is executing it or reading its captures. AudioTrackPrivate (Source/WebCore/platform/graphics/AudioTrackPrivate.h) and the parallel VideoTrackPrivate exposed a setter that did m_enabledChangedCallback = WTF::move(callback); with no lock, and setEnabled() did a two-step if (m_enabledChangedCallback) m_enabledChangedCallback(*this, m_enabled); reading the member in place. The racing sequence: 1. Thread A (e.g. media playback / a MediaPlayer worker thread) enters setEnabled(), passes the if (m_enabledChangedCallback) null test, and begins invoking the stored Function through the member reference; 2. Thread B (e.g. main thread reconfiguring the track) calls setEnabledChangedCallback(newCb), which does m_enabledChangedCallback = WTF::move(newCb) and destroys the old Function object - including the heap buffer holding its captured lambda state; 3. Thread A is still mid-call, now dereferencing freed closure captures - a use-after-free; between the null-check and the call the member could also be cleared, giving a torn/garbage callable.

The fix changes the member type from a bare EnabledChangedCallback Function to RefPtr<SharedEnabledChangedCallback> m_enabledChangedCallback WTF_GUARDED_BY_LOCK(m_lock), where SharedEnabledChangedCallback = Shared is a ref-counted holder for the Function. setEnabled() now takes m_lock only long enough to copy the RefPtr into a local, then invokes enabledChangedCallback->get()(*this, m_enabled) outside the lock. Because the local RefPtr holds a reference, the Shared (and the Function it wraps) stays alive for the whole invocation even if a setter swaps the member meanwhile.

The setter now builds a new Shared under RefPtr, then std::exchange(m_enabledChangedCallback, WTF::move(sharedCallback)) under m_lock to publish it atomically with respect to readers, and - crucially - lets the previous RefPtr (previousCallback) drop its reference after the lock is released: the comment states ‘an in-flight setEnabled() may still hold it’, so the old Function is only truly destroyed once no invocation still references it. This is a ref-counted-holder + lock-guarded-pointer pattern that converts an unsynchronized Function into a safe cross-thread callback. The same transformation is applied verbatim to VideoTrackPrivate’s m_selectedChangedCallback / setSelected().

Key code

Pre-patch unguarded invoke vs. post-patch locked RefPtr copy + invoke-outside-lock (AudioTrackPrivate)

// pre-patch:
if (m_enabledChangedCallback)
    m_enabledChangedCallback(*this, m_enabled);
// post-patch:
RefPtr<SharedEnabledChangedCallback> enabledChangedCallback;
{
    Locker locker { m_lock };
    enabledChangedCallback = m_enabledChangedCallback;
}
if (enabledChangedCallback)
    enabledChangedCallback->get()(*this, m_enabled);

Fix approach

The patch replaces the plain Function member with a lock-guarded RefPtr to a ref-counted Shared holder. Readers (setEnabled/setSelected) copy the RefPtr under m_lock and invoke through the local Ref outside the lock, so the closure cannot be freed mid-call. The setter wraps the new Function in a Shared, swaps it in with std::exchange under m_lock, and deliberately releases the previous RefPtr after dropping the lock so an in-flight invocation that still holds a reference keeps the old closure alive until it finishes. Net effect: the callback’s lifetime is now reference-counted and its member slot lock-published, collapsing the former race window; the previous ownership is no longer a bare move-destroy on the writer thread.

Patch walkthrough

• Source/WebCore/platform/graphics/AudioTrackPrivate.h — Changes m_enabledChangedCallback to RefPtr<Shared> guarded by m_lock; rewrites setEnabled()’s invocation to copy the RefPtr under the lock and call through the local Ref, and setEnabledChangedCallback() to build a Shared, std::exchange it under the lock, and release the previous holder after the lock.
• Source/WebCore/platform/graphics/VideoTrackPrivate.h — Identical transformation for m_selectedChangedCallback / setSelected() / setSelectedChangedCallback(), using Shared guarded by m_lock.

Background

WTF::Function — WebKit’s move-only std::function equivalent; it heap-allocates a callable adapter holding the lambda’s captures. Destroying or move-assigning over a Function frees that adapter, so calling through a Function while another thread destroys it is a use-after-free of the captures.

Shared + RefPtr — Shared wraps a value in a ref-counted container; a RefPtr<Shared> keeps it alive as long as any holder exists. Here it gives the callback a reference count so a reader can pin it for the duration of a call.

WTF_GUARDED_BY_LOCK / Locker — A Clang thread-safety annotation stating a member may only be touched while holding the named Locker’s lock (m_lock). The compiler flags unguarded access; the Locker RAII-acquires the lock for the enclosing scope.

std::exchange under lock — Atomically (w.r.t. other m_lock holders) installs the new callback and returns the old RefPtr, so publication and retrieval of the previous holder happen as one guarded step; the old holder is then dropped outside the lock.

AudioTrackPrivate / VideoTrackPrivate — Platform-layer track objects in the media pipeline whose enabled/selected changes are reported both to DOM clients (notifyClients) and to an owner-supplied Function. These objects are touched from both the main thread and media worker/playback threads.

Vulnerability window

1. Setup — A media element/MediaSource track has an enabledChanged/selectedChanged callback installed by its owner (e.g. MediaPlayer/track-list glue) via setEnabledChangedCallback.
2. Race window — Thread A calls setEnabled()/setSelected() and begins invoking the stored Function; Thread B concurrently calls the setter to replace or clear it.
3. Trigger — Thread B’s WTF::move assignment destroys the old Function’s heap closure while Thread A is still executing it or reading its captures.
4. Fault — Thread A dereferences freed closure state (use-after-free) or invokes a torn callable, corrupting/leaking memory in the WebContent/media process.
5. Post-patch — The RefPtr copy taken under m_lock pins the Shared for the whole call; the setter’s released-after-lock previous holder ensures the old closure outlives any in-flight invocation.

Proof of concept

No test ships in the diff. Conceptually: on a media element, repeatedly toggle track enabled/selected state (driving setEnabled/setSelected on a media thread) while, from the main thread, repeatedly reinstall or clear the track’s change callback (setEnabledChangedCallback). A sufficiently tight interleaving destroys the Function during an in-flight call, tripping a UAF under ASan/TSan. The precise web API path that reinstalls the callback on another thread is (inferred) - the diff only shows the header-level primitive.

Exploitation

1. Reach — Use media APIs (audio/video tracks, MediaSource/MSE) that install and later replace enabled/selected change callbacks, with track state driven from a media worker thread.
2. Trigger — Race a callback replacement (setter on one thread) against a state change that invokes the callback (setEnabled/setSelected on another) to free the Function mid-call.
3. Primitive — Primary primitive is a use-after-free of the heap-allocated Function closure (its captured pointers/state). Secondary effect is a torn read of the member between the null-check and the call.
4. Escalation (inferred) — Open questions gating anything beyond a crash: whether the freed closure can be reclaimed by attacker-controlled bytes before Thread A dereferences it, and whether the captured state steers a call/vtable - neither is shown by the diff, and the window is a narrow cross-thread race that may be hard to widen.

Detection & hunting

For defenders and detection engineers:

• TSan/ASan reports on track callback members — Data-race or heap-use-after-free reports referencing m_enabledChangedCallback/m_selectedChangedCallback or Function invocation inside AudioTrackPrivate::setEnabled / VideoTrackPrivate::setSelected.
• Crashes invoking freed callables in media tracks — Crash signatures where the faulting call target is inside a destroyed WTF::Function during track enable/select changes, especially under concurrent media reconfiguration.
• Concurrent setter/invoke interleaving — Instrument or review callers that invoke setEnabledChangedCallback off the thread that drives setEnabled to find remaining unsynchronized patterns.

Remaining risk in the patch

• Reader invokes outside the lock — The call enabledChangedCallback->get()(*this, m_enabled) runs after the lock is released; this is intentional (to avoid holding m_lock across arbitrary callback code) and is safe for the closure’s lifetime via the RefPtr, but the callback still observes m_enabled read without the lock, so a concurrent state change could deliver a slightly stale enabled/selected value - a correctness, not memory-safety, residual.
• m_lock scope not shown — m_lock’s declaration and what else it guards are not in the supplied diff, so whether every other access to these members (e.g. in TrackPrivateBase or destructors) is also guarded is unverified from here.

Audit directions

• Other TrackPrivateBase callbacks — Audit TextTrackPrivate and other TrackPrivateBase subclasses for bare Function members invoked check-then-call and settable from another thread.
• WTF::Function members as callbacks — Grep WebCore platform/graphics and media for Function<...> m_*Callback members assigned via a trivial setter and invoked without a lock/local copy.
• notifyClients vs. direct callback — Compare the client-notification path (which is presumably synchronized) with the raw owner callback path to find other cases where only one of the two is thread-safe.
• Shared rollout — Check whether sibling classes that need the same cross-thread callback safety have (or still lack) the RefPtr<Shared<…» + WTF_GUARDED_BY_LOCK treatment.