[2] Use-after-free in AudioBufferSourceNode via detached ArrayBuffer backing store

The audio buffer that stayed detachable until you pressed play

Severity: High | Component: WebCore Web Audio | a7e4fdb

Rated High because the diff removes a playback-state gate that left cached channel pointers unpinned, so a detached-then-freed ArrayBuffer backing store is read by the audio render thread; escalation to an info leak requires reclaiming the freed store under attacker control, which the diff does not itself provide.

When a page writes an ArrayBuffer of audio data to node.buffer, an AudioBufferSourceNode caches raw pointers into the AudioBuffer's channel data in m_sourceChannels[i], pointing into ArrayBuffer backing stores managed by JSC. Two prior commits (bug 270007, bug 272607) fixed this same bug by pinning the memory (making it non-detachable), but only pinned when the node was already playing — if the buffer is set before start() (UNSCHEDULED_STATE), the pin was skipped because isPlayingOrScheduled() is false. An attacker sets the buffer (spans cached, no pin), transfers the channel ArrayBuffer via structuredClone (detaches it), drops the copy and triggers GC (backing store freed), then starts playback with loop=true and playbackRate=0. The node reads freed memory through the dangling spans in renderFromBuffer(). The patch pins the memory unconditionally, so structuredClone now throws a TypeError, plus three defensive hardenings in renderFromBuffer.

Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp

LayoutTests/webaudio/audiobuffersource-detached-buffer-crash.html

Patch Details

The primary fix replaces the guarded if (isPlayingOrScheduled()) acquireBufferContent(); in setBufferForBindings with an unconditional acquireBufferContent();, which pins the channel ArrayBuffers non-detachable. Three hardenings are added to renderFromBuffer: an early if (!bufferLength) return false; before the cached m_sourceChannels pointers are read; a corrected loop-wrap clamp from std::min(m_virtualReadIndex, static_cast<double>(bufferLength - 1)) to static_cast<double>(bufferLength) - 1, avoiding size_t underflow to SIZE_MAX when bufferLength == 0; and a new if (readIndex >= bufferLength) return false; on the !pitchRate branch, which was the one read path lacking a bounds check. In ArrayBuffer.cpp, errorMessageForTransfer's assertion changes from ASSERT(buffer->isLocked()) to ASSERT(!buffer->isDetachable()) to match the non-detachable lifetime model.

Lifetime-pinning of externally-owned backing memory made conditional on object playback state, leaving cached raw pointers dangling when the buffer is set before scheduling.

Background

In Web Audio, an AudioBufferSourceNode plays an AudioBuffer whose per-channel samples live in Float32Arrays backed by JSC ArrayBuffers. Assigning node.buffer invokes setBufferForBindings, which caches raw channel span pointers into m_sourceChannels for lock-free render-thread access. Nodes have scheduling states — UNSCHEDULED_STATE until start() is called — and isPlayingOrScheduled() is false in UNSCHEDULED_STATE. Pinning via acquireBufferContent() marks the channel ArrayBuffers non-detachable, so structuredClone with a transfer list throws a TypeError instead of detaching them; isDetachable() reflects this state. ArrayBuffer transfer detaches the source buffer, and once no reference remains the backing store is reclaimed by GC. The audio render thread runs process() → renderFromBuffer(), reading m_sourceChannels[i][readIndex] directly from the cached pointers. Setting playbackRate.value = 0 yields pitchRate == 0, selecting the !pitchRate render branch.

Analysis

This is a use-after-free. setBufferForBindings caches raw span pointers into the AudioBuffer's channel data; to keep that memory alive the node must pin the underlying ArrayBuffers via acquireBufferContent(). Before this fix that pinning was gated on isPlayingOrScheduled(), so when a page assigns node.buffer while still in UNSCHEDULED_STATE, the spans are cached but the pin is skipped. The page can then structuredClone(channelBuffer, {transfer:[channelBuffer]}) to detach the ArrayBuffer, drop the transferred copy, and force a GC that frees the backing store — while m_sourceChannels still holds the now-dangling pointers.

Starting playback runs renderFromBuffer on the audio render thread, which reads m_sourceChannels[i][readIndex] from freed memory. The underflow bug compounds this: with bufferLength == 0 (the detached/empty state), bufferLength - 1 in size_t arithmetic wrapped to SIZE_MAX (the declared type of bufferLength is not shown in the diff, though the double casts make an unsigned type plausible), so std::min(m_virtualReadIndex, (double)SIZE_MAX) no longer clamped the read index. The !pitchRate (playbackRate == 0) read path additionally had no readIndex >= bufferLength guard — exactly the path the PoC selects.

This is exploitable as a use-after-free read on the audio render thread reachable directly from web content. A reliable read/leak would require heap grooming to reclaim the freed backing store before the render read; if that reclamation succeeds, the render read could disclose attacker-chosen bytes into the audio output bus (a read primitive readable back via the rendered PCM), and the interpolation math could propagate freed contents further. Escalation beyond an info leak would depend on what object reoccupies the freed slot.

This vulnerability weakens memory safety inside the WebContent process. The security model assumes memory referenced by a live audio node's cached channel pointers stays valid for the node's lifetime — an invariant supposed to be enforced by pinning the ArrayBuffer non-detachable. Before the fix, that held only when the buffer was set after scheduling; setting it first left the store detachable and freeable from script. Any primitive obtained is renderer-local; a separate sandbox escape would still be required to affect the system.

This is the third fix to the same underlying issue: two prior patches correctly identified that the channel ArrayBuffers must be pinned, but both conditioned the pin on isPlayingOrScheduled(). That condition was the actual defect — a security-relevant lifetime guarantee made state-dependent when the pointers are cached unconditionally. The general lesson: when a raw pointer is cached at assignment time, the keep-alive that protects it must be established at assignment time, not deferred to a later state transition. The extra underflow and the missing !pitchRate bounds check show a defense-in-depth pattern — even after removing the detach primitive, the render loop is hardened so a zero-length buffer cannot be read out of bounds.

Note: The prior-fix references, the branch association of the newly guarded read path, the declared type of bufferLength, and the exact behavior of acquireBufferContent() are inferred from the commit message and test rather than directly visible in the diff. The core detach-free-read flow and the compounding underflow are consistently supported by the patch.

Audit directions

• Cache-now, pin-later mismatch. A raw pointer or span cached into an object at property-set time, whose backing memory is kept alive only by a keep-alive established conditionally (on state, playback, attachment). Audit WebCore for such mismatches: grep the webaudio module for other acquireBufferContent-style pins and confirm they are unconditional relative to where spans are cached; start with AudioBufferSourceNode::setBufferForBindings and sibling nodes that hold m_sourceChannels/channel pointers.
• Unsigned length - 1 as an upper bound. A size_t length - 1 used in a std::min/clamp underflows to SIZE_MAX at length 0 and neutralizes the bound. Grep WebCore/WTF for std::min(.*, .*- 1) and size() - 1 cast to floating point or used in bounds math; verify each has a zero-length early-out. Audio, canvas, and media sample-buffer render loops are prime candidates.
• Real-time/worker threads reading JS-owned backing stores via cached pointers. Audit that every such cached pointer is protected by pinning (isDetachable()-aware) rather than only a lock (isLocked()); the ArrayBuffer.cpp assertion change from isLocked() to !isDetachable() signals these two lifetime models are easy to conflate. Review other cross-thread consumers of ArrayBuffer backing stores for the same confusion.
• Bounds-check parity across render branches. The fix shows the !pitchRate branch was the sole unguarded path among several. Verify every read branch in renderFromBuffer bounds-checks readIndex against bufferLength, and check for other multi-branch render/interpolation loops where one arithmetic branch skips a bounds check present in its siblings.