2024457 - Heap use-after-free in [@ RTCEncodedVideoFrame::~RTCEncodedVideoFrame]: mData nulled before ArrayBuffer detach

Status: RESOLVED DUPLICATE of bug 2021769

(Core :: WebRTC: Signaling, defect)

Description

[Bugmon [:jkratzer for issues]]

Summary

Heap use-after-free in RTCEncodedVideoFrame's destruction sequence. The derived-class destructor at /firefox/dom/media/webrtc/jsapi/RTCEncodedVideoFrame.cpp:119 nulls mData before the base-class destructor can detach the JS ArrayBuffer whose backing store is owned by the native webrtc::TransformableFrameInterface. The ArrayBuffer remains attached after its backing memory is freed, allowing JS to read (and potentially write) freed heap memory.

Affected Code

File: /firefox/dom/media/webrtc/jsapi/RTCEncodedVideoFrame.cpp, lines 116–123

RTCEncodedVideoFrame::~RTCEncodedVideoFrame() {
  // Clear JS::Heap<> members before unregistering as a script holder,
  // so their destructors don't barrier against a finalized JS object.
  mData = nullptr;  // from RTCEncodedFrameBase (protected)   <-- BUG
  // Base class needs this, but can't do it itself because of an assertion in
  // the cycle-collector.
  mozilla::DropJSObjects(this);
}

File: /firefox/dom/media/webrtc/jsapi/RTCEncodedFrameBase.cpp, lines 56–71

RTCEncodedFrameBase::~RTCEncodedFrameBase() { DetachData(); }

void RTCEncodedFrameBase::DetachData() {
  // We might have handled this in unlink already
  if (mGlobal) {
    AutoJSAPI jsapi;
    if (NS_WARN_IF(!jsapi.Init(mGlobal))) {
      return;
    }

    JS::Rooted<JSObject*> rootedData(jsapi.cx(), mData);  // mData == nullptr here
    if (rootedData) {                                      // branch NOT taken
      JS::DetachArrayBuffer(jsapi.cx(), rootedData);       // never called
    }
  }
}

File: /firefox/dom/media/webrtc/jsapi/RTCEncodedFrameBase.cpp, lines 46–48 (where the dangling pointer originates)

// Avoid a copy
mData = JS::NewArrayBufferWithUserOwnedContents(
    jsapi.cx(), mState.mFrame->GetData().size(),
    (void*)(mState.mFrame->GetData().data()));

Why this is vulnerable:

C++ destroys a derived class in the order: derived destructor body → base destructors in reverse declaration order. RTCEncodedVideoFrame inherits RTCEncodedVideoFrameData first, then RTCEncodedFrameBase. Destruction order:

1. ~RTCEncodedVideoFrame() body runs → sets mData = nullptr
2. ~RTCEncodedFrameBase() runs → calls DetachData() → mData is null → JS::DetachArrayBuffer is skipped
3. ~RTCEncodedVideoFrameData() → ~RTCEncodedFrameState() → destroys std::unique_ptr<TransformableFrameInterface> mFrame → frees the EncodedImageBuffer

The ArrayBuffer created with JS::NewArrayBufferWithUserOwnedContents does not own its backing store — it merely aliases mFrame->GetData().data(). Detaching it is mandatory before that memory is freed. The comment at line 117–118 ("Clear JS::Heap<> members before unregistering as a script holder") was added to avoid GC write-barrier issues, but it accidentally defeats the DetachData() safety mechanism in the base destructor.

This path is only hit when the object is destroyed via direct delete (refcount→0, SnowWhiteKiller) rather than CC Unlink. The CC Unlink path (RTCEncodedFrameBase.cpp:18) calls DetachData() before nulling mData, so it's safe. Frames constructed via new RTCEncodedVideoFrame(orig) have mOwner = nullptr (see RTCEncodedVideoFrame.cpp:73), forming no cycles, and are therefore destroyed via SnowWhiteKiller — the vulnerable path.

Correct Pattern

RTCEncodedVideoFrame::~RTCEncodedVideoFrame() {
  // Detach the user-owned-contents ArrayBuffer BEFORE nulling mData,
  // because mFrame (which owns the backing store) is about to be freed
  // by ~RTCEncodedFrameState().
  DetachData();
  // Clear JS::Heap<> members before unregistering as a script holder,
  // so their destructors don't barrier against a finalized JS object.
  mData = nullptr;
  mozilla::DropJSObjects(this);
}

Exploit Chain

1. Page obtains a genuine RTCEncodedVideoFrame via RTCRtpScriptTransform → worker → postMessage back to the main thread (structured clone path creates a frame with mOwner = nullptr).
2. Page calls new RTCEncodedVideoFrame(orig) (JS constructor). This invokes RTCEncodedVideoFrame::Constructor → aOriginalFrame.Clone() → webrtc::CloneVideoFrame() → allocates a fresh 8192-byte EncodedImageBuffer on the heap. The new frame's mData is a JS::NewArrayBufferWithUserOwnedContents aliasing that buffer. The clone has mOwner = nullptr.
3. Page reads clone.data and stores the returned ArrayBuffer in a long-lived array. This creates an independent JS root for the ArrayBuffer, decoupled from the RTCEncodedVideoFrame wrapper's lifetime.
4. Page drops all references to the clone (local variable goes out of scope). Only the ArrayBuffer remains rooted.
5. GC collects the RTCEncodedVideoFrame JS wrapper → native refcount drops to 0 → SnowWhiteKiller deletes the C++ object. Because mOwner is null, there is no cycle and CC Unlink is not used.
6. ~RTCEncodedVideoFrame() nulls mData, then ~RTCEncodedFrameBase()'s DetachData() sees null and does nothing, then ~RTCEncodedFrameState() frees the EncodedImageBuffer.
7. Page calls structuredClone(stolenBuffer). The ArrayBuffer still reports byteLength > 0 (never detached). JSStructuredCloneWriter::writeArrayBuffer memcpy's 4072+ bytes from the freed heap region → heap-use-after-free READ.

Note: Because the ArrayBuffer is never detached, JS can also construct a Uint8Array view over it and write to the freed region, enabling heap corruption beyond mere info-leak.

Steps to Reproduce

• Pref: fuzzing.enabled = true (implied by the harness; needed for FuzzingFunctions to deterministically drive GC/CC). No other environmental setup required — no source patch, no external servers.
• Open the provided test.html. The testcase sets up a local WebRTC loopback (two RTCPeerConnection objects in the same page) with a canvas captureStream and an RTCRtpScriptTransform, then clones the resulting frame 50 times and stashes each clone's .data ArrayBuffer. It then loops GC/CC/memory-pressure followed by structuredClone of each stashed buffer.
• Timeout: ~120 seconds (WebRTC negotiation + encoder warmup + GC loop).

Security Impact

• Severity: High (sec-high). Heap use-after-free reachable from unprivileged web content with no user interaction beyond visiting a page. Reads 4KB+ of freed heap data per access. Because the ArrayBuffer is fully writable from JS (never detached), an attacker can also write to the freed region after it has been reallocated, giving a heap-write primitive for RCE in the content process.
• Attacker capability: Read and write an 8192-byte freed heap region of attacker-controlled size (the size is controlled by tmp.data = buf → SetData → the subsequent clone copies that size). With heap grooming, the attacker can overlap this region with a victim object and corrupt it.
• Preconditions: WebRTC must be enabled (default). RTCRtpScriptTransform must be available (shipping in release Firefox). No permission prompts are required for canvas-backed WebRTC.

Suggested Fix

Call DetachData() in the derived destructor before nulling mData:

// dom/media/webrtc/jsapi/RTCEncodedVideoFrame.cpp
RTCEncodedVideoFrame::~RTCEncodedVideoFrame() {
  // Detach the user-owned-contents ArrayBuffer first: its backing store
  // (mState.mFrame) is about to be freed by ~RTCEncodedFrameState(), and
  // nulling mData below would make the base dtor's DetachData() a no-op.
  DetachData();
  // Clear JS::Heap<> members before unregistering as a script holder,
  // so their destructors don't barrier against a finalized JS object.
  mData = nullptr;
  mozilla::DropJSObjects(this);
}

Apply the same fix to RTCEncodedAudioFrame::~RTCEncodedAudioFrame() in /firefox/dom/media/webrtc/jsapi/RTCEncodedAudioFrame.cpp, which has the identical pattern. DetachData() is idempotent (it checks if (rootedData)), so the subsequent base-destructor call is harmless.

Comment 1

[Bugmon [:jkratzer for issues]]

Attachment: README.txt

Comment 2

[Bugmon [:jkratzer for issues]]

Attachment: Crash stack trace

Comment 3

[Bugmon [:jkratzer for issues]]

Attachment: Testcase: test.html

Comment 4

[Bugmon [:jkratzer for issues]]

Attachment: prefs.txt