[6] Stale video-frame observer survives capture-proxy destruction

MediumWebKit GPU media captureUAF

Teardown asked a boolean whether it was registered; the map said otherwise

b732463

Medium. The register/unregister pair is asymmetric: a state-change callback adds the observer back without touching the flag that teardown consults, so the removal is skipped on a registration that exists. The stale raw pointer is only dereferenced when a second proxy keeps the same source alive and producing.

Camera, microphone and screen capture do not run in the process that renders the page — the GPU process owns the capture source and forwards frames to WebContent over IPC. Each capture session gets a proxy object in the GPU process that registers against the source twice: once as a state observer for configuration changes, and once as a frame observer for the actual media data, with the second registration held only while the session is producing. The source keeps its frame observers in a lock-guarded map keyed by a bare, non-owning pointer, so membership in that map has to be exactly equivalent to the proxy's own record of whether it is observing.

The angle: a WebContent process that can emit raw capture IPC and holds a display-capture grant can order StopProducingData, Clone, StartProducingData, a configuration change and RemoveSource so that a freed pointer stays in the source's frame-observer map, which the capture thread virtual-dispatches through on the next frame.

Patch Details

The re-registration guard inside UserMediaCaptureManagerProxySourceProxy::sourceConfigurationChanged() gains a leading m_isObservingMedia && conjunct. Pre-fix the body — source->removeVideoFrameObserver(*this) immediately followed by source->addVideoFrameObserver(*this, { m_widthConstraint, m_heightConstraint }, m_frameRateConstraint) — ran whenever the device was Screen/Window, m_videoConstraints was set, and updateVideoConstraints() reported a change, regardless of producing state; post-fix it is confined to proxies already in the observing state that observeMedia() sets and unobserveMedia() clears. A regression test clones a stopped source, starts the clone, and then removes the original.

A secondary registration site updates the external container but not the bookkeeping flag the teardown path checks, so the unregister is skipped on a registration that exists.

  construct ──► addObserver (state, WeakHashSet)
      |
  observeMedia() ──► addVideoFrameObserver, m_isObservingMedia = true
      |
  unobserveMedia() ► removeVideoFrameObserver, m_isObservingMedia = false
      |
  sourceConfigurationChanged()
      └─► addVideoFrameObserver(*this)   flag stays false   <-- window opens
      |
  ~proxy ──► unobserveMedia() early-returns on the flag
      └─► raw key survives in m_videoFrameObservers
      |
  capture thread ──► videoFrameAvailable() through freed memory

Background

Where this lives. UserMediaCaptureManagerProxy and its per-source UserMediaCaptureManagerProxySourceProxy are the GPU-process endpoint that owns a WebCore RealtimeMediaSource — camera, microphone, screen or window capture — observes its state and its frames, and forwards media to WebContent over IPC.

Two registrations, two containers. The RealtimeMediaSourceObserver registration receives state-change notifications such as sourceConfigurationChanged and lives in the source's WeakHashSet. The VideoFrameObserver (or AudioSampleObserver) registration receives the actual media data and lives in m_videoFrameObservers, a map keyed by a bare, non-owning, non-weak VideoFrameObserver* and guarded by m_videoFrameObserversLock. The destructor's m_source->removeObserver(*this) implies the state registration is established once at construction and lasts the proxy's lifetime.

Threading. Frame delivery runs on a background capture thread; observer registration and IPC handling run on the main thread.

Source cloning. RealtimeMediaSource::clone() is implemented in the base class as return *this, so a clone can be the same source instance rather than a new one, with UserMediaCaptureManagerProxy::Clone producing a second proxy holding a Ref to it.

Analysis

The root cause is an asymmetric register/unregister pair. Membership in m_videoFrameObservers must be exactly equivalent to m_isObservingMedia == true, because the only teardown path — unobserveMedia(), called from the destructor — is gated on the flag rather than performing an unconditional idempotent removal. sourceConfigurationChanged() is a state-change callback, so it is delivered to every attached proxy regardless of producing state, and pre-fix it re-inserted the raw self-pointer without consulting or updating the flag.

A stopped or never-started proxy therefore has m_isObservingMedia == false while its raw pointer sits in the source's map. For a Screen or Window device with cached m_videoConstraints and a constraint change, the callback performs the insertion; removeVideoFrameObserver(*this) is a no-op in that state. On destruction, unobserveMedia() returns immediately, only the WeakHashSet state entry is cleaned up, and the raw key survives the free.

Whether that stale key is ever dereferenced depends on the source outliving the proxy. m_source is a Ref<RealtimeMediaSource>, so with a single proxy the source dies alongside it and the stale key is inert. The load-bearing condition is source aliasing across two proxies: the base clone() returns *this, and the commit message states DisplayCaptureSourceCocoa does not override it, so cloning yields a second proxy holding a Ref to the same instance. The regression test's shape corroborates that reading — cloning the stopped source, starting the clone, and only then removing the original is meaningful as a crash trigger only if both proxies share one source, and the test substitutes IPCTestingAPIEnabled=true for the ability to emit that message ordering directly.

Under that aliasing the surviving clone keeps the source alive and producing, and the next capture-thread frame delivery walks the observer map and virtual-dispatches videoFrameAvailable() through freed memory. This vulnerability weakens object lifetime inside the GPU process along a path that needs a display-capture grant plus an IPC ordering the honest WebContent-side UserMediaCaptureManager would not emit, with the free and the dispatch landing on different threads.

Audit directions

Registration flags versus container membership. When a teardown path is gated on a boolean rather than performing an idempotent removal, every code path that touches the container without touching the boolean is a leak of a raw key past the object's lifetime. Audit the other RealtimeMediaSource observer kinds — AudioSampleObserver in particular — and every call site that reaches addVideoFrameObserver / addAudioSampleObserver outside observeMedia(). The review tell is an add*Observer call in a callback body when the class also owns an m_isObserving* flag.
Non-weak keys in observer registries. A registry keyed by a bare pointer converts any missed unregistration into a use-after-free rather than a no-op, and the cost of the safe shape here is one idempotent erase in the destructor. Sweep the GPU-process capture and media registries for containers whose key type is a raw observer pointer and whose removal is conditional.
Sources shared by cloning. A base-class clone() that returns *this means "two owners, one object" for every subclass that does not override it. Enumerate the RealtimeMediaSource subclasses and check which ones inherit the aliasing implementation, since aliasing is what turns an inert stale key into a live dereference.