[6] MediaPlayerPrivateMediaSourceAVFObjC re-entrant KVO during destruction

Severity: Medium | Component: WebCore platform media | 4193d2c

Rated Medium because the diff fixes a UAF-during-teardown of MediaPlayerPrivateMediaSourceAVFObjC via KVO re-entrancy reading the already-destroyed m_logger; attacker influence is limited to driving the teardown shape, and the immediate effect is a renderer crash rather than a controllable read/write.

Revoke all weak pointers at the start of ~MediaPlayerPrivateMediaSourceAVFObjC() to prevent re-entrant callbacks during member destruction. C++ destroys m_logger before m_mediaSourcePrivate (reverse declaration order), and the m_mediaSourcePrivate destructor can trigger synchronous KVO notifications that invoke WeakPtr-guarded callbacks on the partially-destroyed object. Since the CanMakeWeakPtr base class destructor runs after all member destructors, WeakPtr::get() still succeeds, allowing callbacks to access already-destroyed members.

Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm

Patch Details

A single line is added at the top of the destructor: weakPtrFactory().revokeAll(). This invalidates all outstanding WeakPtrs pointing at this object so any KVO notifications fired later during destruction of members like m_mediaSourcePrivate cannot resolve their WeakPtr back to the now partially-destroyed instance.

Failure to invalidate weak references before destroying members reachable by re-entrant callbacks during teardown.

Background

WeakPtr in WebKit is a non-owning reference backed by a WeakPtrFactory stored on the target via the CanMakeWeakPtr CRTP base; WeakPtr::get() returns null only after the factory has been revoked or destroyed. In C++, members of a derived class are destroyed in reverse declaration order, then base destructors run; the CanMakeWeakPtr destructor that revokes the factory is a base destructor and therefore runs after all derived-class member destructors. KVO (Key-Value Observing) is Cocoa's observer pattern, and AVFoundation objects fire KVO notifications synchronously to registered observers. MediaPlayerPrivateMediaSourceAVFObjC owns m_mediaSourcePrivate (a MediaSourcePrivateAVFObjC) and a Logger reference m_logger. setNetworkState() is the standard MediaPlayer hook used to surface network state changes and in this implementation it logs through m_logger.

Analysis

The destructor relied on the default CanMakeWeakPtr teardown ordering: the WeakPtrFactory is only revoked when the base destructor runs, which is after all derived-class members have already been destroyed. During member destruction (specifically m_mediaSourcePrivate's destructor), AVFoundation can synchronously fire KVO notifications. Those notifications are wired through WeakPtr-guarded callbacks; because the factory has not yet been revoked, WeakPtr::get() still returns a non-null pointer.

The callbacks then invoke methods like setNetworkState() that touch members declared earlier (m_logger) which have already been destroyed in reverse declaration order. This is a use-after-destruction on a partially-destroyed object, manifesting as a crash in setNetworkState during destruction. The fix preempts that window by revoking all WeakPtrs at the very start of the destructor, so any subsequent KVO-driven callback finds a null WeakPtr and bails out before touching destroyed state.

This vulnerability weakened memory safety in the WebContent process during media element teardown. The invariant that an object's WeakPtr consumers see a null pointer once the object is being destroyed is violated for objects whose member destructors can fire callbacks — the CanMakeWeakPtr base revokes too late. Any class that derives from CanMakeWeakPtr and whose member destructors can synchronously invoke callbacks (KVO, dispatch sources, NSNotificationCenter observers, AVFoundation delegates, Core Media listeners) is vulnerable to the same teardown-time re-entrancy. The defensive idiom established here — call weakPtrFactory().revokeAll() as the first line of the destructor — should arguably be the default for any AVFoundation / Cocoa-observer-owning class. The commit flags this as a regression from r301011.

Audit directions

• CanMakeWeakPtr-derived classes whose member destructors can synchronously fire callbacks (KVO, NSNotificationCenter, AVFoundation/Core Media delegates, dispatch handlers). Audit every class in Source/WebCore/platform/graphics/avfoundation/objc/ and Source/WebCore/platform/mediastream/ that both inherits from CanMakeWeakPtr and owns Objective-C members observed via KVO or registered as delegates; if its destructor does not begin with weakPtrFactory().revokeAll(), treat it as a candidate. Start by grepping for class .* : .*CanMakeWeakPtr intersected with addObserver: / removeObserver: usage in .mm files.
• Member declaration order that places infrastructure (m_logger, identifiers, factories) BEFORE owners of callback-capable objects. This guarantees the infrastructure dies first while the callback owner is still being destroyed. Review member declaration order in media/graphics player classes and cross-reference with setNetworkState, setReadyState, and other state-callback entry points that touch m_logger or other early-declared fields.
• The r301011 regression. Use git log -p and git blame on the affected destructor and on the surrounding KVO observer registration sites to confirm whether r301011 added a new KVO subscription, changed destruction ordering, or removed a prior revokeAll() call.
• Synchronous-callback re-entrancy during destruction beyond KVO. Timer::stop(), dispatchToRendererQueue flushes, and NativePromise rejection paths can also re-enter. Audit destructors that call cancelPendingSeek, m_seekTimer.stop(), or any dispatch_sync near teardown to verify weak references are revoked before those calls, not after.