[6] QueuedVideoOutput tests liveness without holding it

MediumWebCore AVFoundation media backendUAF

f90b4ea

Medium. The freed access is a plain member touch after a synchronous observer callback rather than a virtual dispatch, and triggering it requires an observer that tears down the media player from inside a frame notification. That precondition is engine-internal, which is what keeps this below the speech-synthesis and audio-listener cases.

Checking a weak pointer and then using the object it names is a lifetime pattern that only holds if nothing can release the object in between — and a synchronous callback into observers is exactly the kind of thing that can. WebCore's AVFoundation video path uses QueuedVideoOutput, a helper that pulls decoded CVPixelBuffers out of an AVPlayerItemVideoOutput and keeps a presentation-time-keyed queue of frames for the main-thread media player. It sits between AVFoundation's decode callbacks, delivered on a background dispatch queue, and WebCore's MediaPlayerPrivateAVFoundationObjC, notifying the player when the current frame changes.

The angle: a current-image-changed observer that tears down the media player synchronously frees QueuedVideoOutput while one of its own methods is still on the stack, leaving the remainder of that method operating on freed TZone memory.

The pre-fix pattern, repeated across every main-run-loop block in the file:

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

// each block captured only a WeakPtr, used operator bool as its sole check,
// then dereferenced it as a raw pointer:
//   parent->addVideoFrameEntries(...)
//   weakThis->purgeImagesBeforeTime(...)
//   weakThis->nextImageTimeReached()

Patch Details

Every main-run-loop block in QueuedVideoOutput.mm is converted from a bare WeakPtr null-test to a RefPtr upgrade: RefPtr protectedParent = parent.get() takes a reference that survives whatever the callee re-enters. The affected paths are addVideoFrameEntries(), purgeImagesBeforeTime(), purgeVideoFrameEntries(), rateChanged(), and nextImageTimeReached().

Weak-pointer liveness test used as a lifetime guarantee, with the raw pointer materialized from it outliving the object across a synchronous callback.

Background

Where this lives. MediaPlayerPrivateAVFoundationObjC is WebCore's AVFoundation-backed media player implementation. QueuedVideoOutput is its frame-delivery helper: it owns an ImageMap m_videoFrames keyed by presentation time and keeps the player supplied with decoded frames.

Ownership. QueuedVideoOutput is RefCounted<QueuedVideoOutput> with a single strong owner — the MediaPlayerPrivateAVFoundationObjC that created it through QueuedVideoOutput::create(). There is exactly one RefPtr to it in a healthy system.

WeakPtr and what a null-test establishes. A WeakPtr null-test establishes that the referent is alive at the instant of the test. It takes no reference and therefore establishes nothing about the remainder of the call. This is the distinction the whole bug turns on, and it is easy to miss in review because the null-test looks like a safety check.

WeakHashSet observers. QueuedVideoOutput.h declares WeakHashSet<CurrentImageChangedObserver> m_currentImageChangedObservers. Observers held in a weak set are notified synchronously, on the notifier's stack, which makes each notification a re-entrancy point into arbitrary WebCore code.

Analysis

The missing invariant is that an object must be pinned for the whole duration of a callback it has entered, not merely at entry. The bug class is a use-after-free driven by re-entrant teardown — a lifetime violation, not a data race on the object body.

  addVideoFrameEntries()  (main run loop block)
  ───────────────────────────────────────────────
  weakThis  ──► operator bool ✔        (t0: alive)
  raw this  ──► materialized
  notify m_currentImageChangedObservers
      └─► observer tears down media player
            └─► last RefPtr<QueuedVideoOutput> dropped
                  └─► ~QueuedVideoOutput() / invalidate()
                        weakThis cleared, raw this NOT   ◄── failure window opens
  ... remainder of addVideoFrameEntries()
      touches m_videoFrames, re-arms boundary observer
        └─► freed TZone memory

Per the commit message, addVideoFrameEntries() synchronously fires the current-image-changed observers. One of those observers can synchronously tear down the media player, dropping the only RefPtr<QueuedVideoOutput> and running ~QueuedVideoOutput() — which calls invalidate() — while addVideoFrameEntries() is still on the stack. The WeakPtr in the block is cleared by that destruction, but the raw this already materialized from it is not.

The same shape holds for purgeImagesBeforeTime(), purgeVideoFrameEntries(), rateChanged(), and nextImageTimeReached(), each of which can reach the same notification path — which is why the fix is applied uniformly across the file rather than at the one site the crash was observed on.

On exploitability: the freed accesses are member touches — m_videoFrames, the re-arming of the boundary time observer — rather than a virtual dispatch, so the immediate primitive is a write into or read from a freed TZone allocation at fixed member offsets rather than a hijacked indirect branch. Ordinary playback drives the delegate callbacks and the periodic-observer tick directly; the precondition is a current-image-changed observer that tears the player down inside the notification, which is engine-internal sequencing a page influences rather than schedules.

This vulnerability weakens WebContent-process memory safety in the media frame-delivery path, on a code path exercised by ordinary video playback.

Audit directions

• Weak-pointer test followed by raw-pointer use across a callback. The pattern is if (weakThis) { ... weakThis->doThing(); ... } where doThing() can notify observers or run script. The review tell is visually distinctive: a block that captures WeakPtr (not RefPtr) and contains more than one dereference of it, with anything resembling a notification between them. Convert the test into a hold and the whole class disappears. Start with the other AVFoundation helper classes that post blocks to the main run loop.
• WeakHashSet notification loops in singly-owned objects. An object with exactly one strong owner is trivially freed by an observer that reaches that owner. Audit types holding WeakHashSet<...Observer> for whether they protect themselves across the notification loop; the tell is a for (auto& observer : m_observers) loop in a method whose body continues touching members afterwards.

---