02e76c68f6  Fix crashes in RealtimeIncoming*Source destructors by ensuring sink removal before member destruction

The bug at a glance

Incoming WebRTC audio and video sources register themselves as sinks on libwebrtc track objects, so libwebrtc delivers media on dedicated audio/video threads via OnData/OnFrame callbacks. Because C++ destroys a derived object’s own members before running the base-class destructor — and it was the base destructor’s stop() that removed the sink — a source that was still producing data at destruction time had its derived members (e.g. m_audioBufferList, pixel-buffer pools) freed while a concurrent OnData/OnFrame callback could still touch them, a genuine cross-thread use-after-free. It is reachable whenever normal teardown via requestToEnd() is blocked, which the commit notes happens when a RealtimeMediaSourceObserver returns true from preventSourceFromEnding(). This rates high: a UAF driven by remote-peer media timing in the media/GPU process is memory corruption with an attacker (the remote peer) influencing the freed-memory contents and the race window.

This is a pure C++ destruction-order bug: the sink-removal call sat in the base-class destructor, but base destructors run last, after every derived member has already been destroyed. The fix moves stop() into each leaf subclass’s own destructor so the sink is torn down first, then replaces the base destructor’s stop() with ASSERT(!isProducingData()) to enforce that subclasses uphold the new contract. The race only matters because OnData/OnFrame run on libwebrtc’s own threads, so removal must be synchronized against an in-flight callback.

Root cause

RealtimeIncomingAudioSource and RealtimeIncomingVideoSource are the WebCore abstractions for a remote peer’s inbound media track. Each registers itself as a rtc::VideoSinkInterface / audio sink on the underlying libwebrtc webrtc::VideoTrackInterface / AudioTrackInterface. Once registered, libwebrtc pushes decoded media into the source from its internal media threads by invoking OnFrame(const webrtc::VideoFrame&) and OnData(…) — these callbacks run concurrently with the WebKit main thread, not on it.

The concrete sources are platform subclasses: RealtimeIncomingAudioSourceCocoa / RealtimeIncomingVideoSourceCocoa on Apple platforms and the …LibWebRTC variants for the GStreamer/libwebrtc port. These subclasses own the buffers the callbacks write into — for example the Cocoa audio source’s m_audioBufferList and the video source’s CVPixelBufferPool state. The sink is removed by stop(), which for the audio path calls RemoveSink() on the track; RemoveSink in RemoteAudioSource properly synchronizes with any in-progress OnData via sink_lock_, so once RemoveSink returns no further callback is running or will start.

The bug is where stop() was called. Before the patch only the base-class destructors (~RealtimeIncomingAudioSource / ~RealtimeIncomingVideoSource) called stop(). But C++ runs destructors bottom-up: when a RealtimeIncomingAudioSourceCocoa is destroyed, the derived destructor and derived member destructors run first, tearing down m_audioBufferList and friends, and only then does the base ~RealtimeIncomingAudioSource run and call stop() to remove the sink. If the source is still producing data at that point, a libwebrtc thread can be inside OnData/OnFrame — or can enter it — touching the already-freed derived members before the base destructor finally removes the sink. That is the use-after-free.

The commit explains why a source can still be producing data at destruction. Normally RTCPeerConnection::doClose() ends sources via requestToEnd(), which stops production before teardown. But requestToEnd() is blocked if any RealtimeMediaSourceObserver returns true from preventSourceFromEnding(); on those paths the source object can reach destruction while the sink is still attached and callbacks are still flowing.

The fix enforces the correct ordering. Each leaf subclass gains (or has added) an explicit destructor that calls stop() itself — RealtimeIncomingAudioSourceCocoa::~RealtimeIncomingAudioSourceCocoa, RealtimeIncomingVideoSourceCocoa::~RealtimeIncomingVideoSourceCocoa, and the two …LibWebRTC destructors (the video LibWebRTC one changing from an inline empty { } to a real out-of-line definition). Because the subclass destructor runs before any derived member is destroyed, stop() now removes the sink (synchronizing via sink_lock_ so no OnData/OnFrame is in flight) while the members it might touch are still alive. The base-class destructors replace their stop() call with ASSERT(!isProducingData()), turning the required ordering into a checked invariant: if any future subclass forgets to stop() first, the base destructor will assert that production has already ceased.

Key code

Base audio destructor no longer removes the sink itself; it now asserts production already stopped (subclasses must stop() first)

RealtimeIncomingAudioSource::~RealtimeIncomingAudioSource()
{
    // Subclasses must call stop() in their destructors to ensure the audio
    // track sink is removed BEFORE derived members are destroyed. Otherwise,
    // the OnData callback may access destroyed members on the audio thread.
    ASSERT(!isProducingData());
    m_audioTrack->UnregisterObserver(this);
}

Patch walkthrough

• Source/WebCore/platform/mediastream/RealtimeIncomingAudioSource.cpp — The base ~RealtimeIncomingAudioSource no longer calls stop(); it now asserts ASSERT(!isProducingData()) before m_audioTrack->UnregisterObserver(this). The sink-removal responsibility is delegated to subclasses, and the assert verifies they honoured it so the audio-thread OnData callback cannot fire after the derived members are gone.
• Source/WebCore/platform/mediastream/RealtimeIncomingVideoSource.cpp — Symmetric change for video: base ~RealtimeIncomingVideoSource replaces stop() with ASSERT(!isProducingData()) before m_videoTrack->UnregisterObserver(this), guarding against OnFrame firing on the video thread after derived members are destroyed.
• Source/WebCore/platform/mediastream/cocoa/RealtimeIncomingAudioSourceCocoa.cpp (+ .h) — Adds RealtimeIncomingAudioSourceCocoa::~RealtimeIncomingAudioSourceCocoa() that calls stop(), and declares it in the header. Running in the leaf destructor, stop() removes the audio sink (synchronizing via sink_lock_) before m_audioBufferList and other Cocoa members are destroyed.
• Source/WebCore/platform/mediastream/cocoa/RealtimeIncomingVideoSourceCocoa.mm (+ .h) — Adds RealtimeIncomingVideoSourceCocoa::~RealtimeIncomingVideoSourceCocoa() calling stop(), declared in the header, so the video sink is removed before the pixel-buffer-pool members are torn down.
• Source/WebCore/platform/mediastream/libwebrtc/gstreamer/RealtimeIncomingAudioSourceLibWebRTC.cpp (+ .h) — Adds an out-of-line ~RealtimeIncomingAudioSourceLibWebRTC() calling stop() and declares it, applying the same fix to the GStreamer/libwebrtc audio port.
• Source/WebCore/platform/mediastream/libwebrtc/gstreamer/RealtimeIncomingVideoSourceLibWebRTC.cpp (+ .h) — Changes the previously inline empty ~RealtimeIncomingVideoSourceLibWebRTC() { } into an out-of-line destructor that calls stop(), fixing the GStreamer/libwebrtc video port. The header declaration switches from the inline body to a plain declaration.

Background

C++ destruction order (base last, members before base) — When an object of a derived class is destroyed, C++ runs the most-derived destructor body first, then destroys that class’s non-static data members in reverse declaration order, and only then invokes the base-class destructor. This means any cleanup placed in a base destructor executes after all derived members have already been freed. Cleanup that must run while derived members are still alive — such as unregistering an external callback that touches those members — therefore cannot live in the base destructor; it must run in the derived destructor. That ordering rule is the entire root cause here.

libwebrtc sink interfaces and OnData/OnFrame threading — libwebrtc delivers decoded media to consumers registered as sinks: a video sink implements rtc::VideoSinkInterface::OnFrame(const webrtc::VideoFrame&) and an audio sink receives OnData(…). These callbacks are invoked on libwebrtc’s own media/worker threads, asynchronously to the WebKit main thread where objects are created and destroyed. A sink object’s lifetime must therefore be coordinated with these background callbacks: it must guarantee no callback is running, and none can start, before the memory the callback uses is freed.

RemoveSink and sink_lock_ synchronization — stop() removes the source from the track via RemoveSink(). In RemoteAudioSource, RemoveSink acquires sink_lock_, the same lock held while dispatching OnData, so RemoveSink blocks until any in-progress OnData completes and prevents new ones from starting. This makes stop() a proper barrier: after it returns, no OnData callback is or will be executing. The bug was not that RemoveSink was unsafe — it was that it ran too late (in the base destructor, after derived members were freed), so the synchronization protected the wrong lifetime window.

requestToEnd() and preventSourceFromEnding() — RealtimeMediaSource lifetime is normally ended cooperatively via requestToEnd(), which RTCPeerConnection::doClose() uses to stop sources before they are destroyed. However, requestToEnd() consults observers: if any RealtimeMediaSourceObserver returns true from preventSourceFromEnding(), the request is refused and the source keeps producing data. On such paths the source object can be destroyed while still active, which is precisely the condition that lets a still-attached sink deliver a callback into a partially-destroyed object.

isProducingData() as a destructor invariant — isProducingData() reports whether the source is currently active (sink attached, callbacks possible). By replacing the base destructor’s stop() with ASSERT(!isProducingData()), the patch converts an implicit ordering requirement into an explicit, checkable class invariant: by the time control reaches the base destructor, production must already have been stopped by the subclass. In debug builds a subclass that forgets to call stop() first will trip the assert during teardown, catching regressions of this exact UAF pattern early.

Incoming media sources in the process/threat model — RealtimeIncoming*Source objects represent media arriving from a remote WebRTC peer, decoded by libwebrtc. The remote peer influences both the timing and the content of OnData/OnFrame delivery, and the frequency of teardown races via how the call is negotiated and closed. That makes the freed-memory contents and the race window partially attacker-controllable, which is what elevates a teardown UAF here from a mere reliability bug to a security-relevant memory-corruption primitive in the media pipeline.

Vulnerability window

1. Design — RealtimeIncoming{Audio,Video}Source registered as libwebrtc sinks and placed sink removal (stop()) in the base-class destructor, implicitly assuming the source would always be stopped before destruction.
2. Latent UAF — Because base destructors run after derived members are destroyed, any destruction of a still-producing source freed derived members (m_audioBufferList, pixel-buffer pools) while OnData/OnFrame could still touch them on libwebrtc threads.
3. Reachable path — When preventSourceFromEnding() blocks requestToEnd(), doClose() cannot stop the source first, so sources reach destruction while actively producing data — opening the cross-thread race.
4. Crashes observed — Crashes in RealtimeIncoming*Source destructors were traced to callbacks accessing destroyed members during teardown (bug 308636 / rdar://162084447).
5. Branch fix — Fix first landed on a Safari release branch as 305413.429@rapid/safari-7624.2.5.110-branch (6d6607033ebc), rdar://176067300, indicating security-update urgency.
6. Mainline — David Kilzer’s fix (reviewed by Jean-Yves Avenard and Youenn Fablet) moved stop() into each leaf destructor and added ASSERT(!isProducingData()) to the base destructors; landed as commit 313616@main.

Triggering

No test or PoC ships with the patch. Conceptual trigger: establish an RTCPeerConnection receiving a remote audio and/or video track so RealtimeIncoming{Audio,Video}Source objects are created and actively receiving OnData/OnFrame. Install a RealtimeMediaSourceObserver (or reach a state) where preventSourceFromEnding() returns true so requestToEnd() is refused, then destroy the source (e.g. tear down the connection / drop the track) while remote media is still arriving. On an unpatched build, a libwebrtc audio/video thread executing OnData/OnFrame can touch derived members (m_audioBufferList, pixel buffer pool) after they are freed but before the base destructor’s stop() removes the sink — a use-after-free whose reliability depends on winning the teardown-vs-callback race. Reproduction is inherently racy and timing-dependent; there is no deterministic script trigger.

Exploitation

1. Setup — Remote peer negotiates an inbound audio/video track, creating an active RealtimeIncoming*Source that libwebrtc feeds via OnData/OnFrame on its media threads; the remote peer controls media cadence and payload.
2. Force still-producing teardown — Reach a state where requestToEnd() is blocked (a RealtimeMediaSourceObserver returning true from preventSourceFromEnding()), then destroy the source while data is still flowing, so the derived-member free races the in-flight callback.
3. Win the race — If a libwebrtc thread is inside (or enters) OnData/OnFrame in the window after derived members are freed and before the base destructor’s stop() removes the sink, the callback reads/writes freed memory (e.g. m_audioBufferList) — the UAF. The remote peer’s control over media timing/content helps shape the window and the contents landing in freed memory.
4. Impact / caveat — Observed public impact is crashes during teardown; turning the UAF into a controlled corruption requires grooming the freed allocation and reliably hitting a narrow cross-thread window, which is difficult but not precluded. In the field this is a media-process memory-corruption primitive rather than a demonstrated end-to-end exploit; treat crash reports on these destructors as potential exploitation attempts.

Detection & hunting

For defenders and SOC / detection engineers:

• **Crashes/ASan reports in RealtimeIncoming{Audio,Video}Source or Cocoa/LibWebRTC destructors, or in OnData/OnFrame, during call teardown — Flag media-process crashes whose stack shows a source destructor concurrent with an OnData/OnFrame frame, or a heap-use-after-free on m_audioBufferList / pixel-buffer-pool memory; these are the direct fingerprint of the race.
• Debug-build assertion !isProducingData() in ~RealtimeIncomingAudioSource / ~RealtimeIncomingVideoSource — On patched debug builds this assert firing means a subclass reached the base destructor while still producing data — a regression of the exact bug; alert on it in test/CI telemetry.
• WebRTC calls that tear down tracks while media is still flowing after an ending-prevention — Where instrumentation allows, correlate preventSourceFromEnding()==true states with subsequent source destruction while frames are still arriving as an anomalous, race-prone teardown pattern.

Audit directions

• All RealtimeMediaSource / sink subclasses — Enumerate every subclass of RealtimeIncomingAudioSource/RealtimeIncomingVideoSource (and analogous sink-registering sources) and confirm each leaf destructor calls stop() before member destruction, not relying on a base destructor; verify the new ASSERT(!isProducingData()) invariant holds on every path.
• Other objects that register external callbacks and free members in base destructors — Search WebCore/platform for the general anti-pattern of unregistering an observer/sink/listener in a base destructor while derived members feed that callback; the C++ destruction-order UAF generalizes beyond WebRTC (e.g. KVO, AVFoundation delegates, timers).
• requestToEnd()/preventSourceFromEnding() paths — Audit all RealtimeMediaSourceObserver implementations that can return true from preventSourceFromEnding() and confirm none leave a source destroyable while still producing data without a stop() barrier; these are the paths that make the race reachable.
• RemoveSink synchronization coverage — Verify that RemoveSink (and its equivalents across audio/video and Cocoa/LibWebRTC ports) genuinely blocks against in-flight OnData/OnFrame under the same lock (e.g. sink_lock_) on every platform, so that stop()-in-derived-destructor is a real barrier everywhere and not just on the audio Cocoa path.