Branch: refs/heads/webkitglib/2.52
Home: https://github.com/WebKit/WebKit
Commit: a3630df607da4f1b31ea87bc631eaedd14c0b4b4
https://github.com/WebKit/WebKit/commit/a3630df607da4f1b31ea87bc631eaedd14c0b4b4
Author: Anthony Tarbinian <[email protected]>
Date: 2026-07-02 (Thu, 02 Jul 2026)
Changed paths:
A LayoutTests/webaudio/audiobuffersource-detached-buffer-crash-expected.txt
A LayoutTests/webaudio/audiobuffersource-detached-buffer-crash.html
M Source/JavaScriptCore/runtime/ArrayBuffer.cpp
M Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp
Log Message:
-----------
[WebCore] Unconditionally pin AudioBuffer data in
AudioBufferSourceNode::setBufferForBindings
https://bugs.webkit.org/show_bug.cgi?id=313857
rdar://174651279
Reviewed by Chris Dumez.
When a page tries to write an ArrayBuffer of audio data to an audio node
(writing to node.buffer), the page can free the ArrayBuffer's backing
store and leave
the
audio node to read the freed memory through a cached
pointer.
An AudioBufferSourceNode caches raw pointers into an AudioBuffer's channel
data when you set node.buffer. This happens in
AudioBufferSourceNode::setBufferForBindings
where a pointer from a span is cached in m_sourceChannels[i].
Those pointers point into ArrayBuffer backing stores managed by JSC.
If the memory is not pinned, or detachable, then
the page can force the ArrayBuffer's backing store to be freed by
dropping the reference to the result of structuredClone on the original buffer
and kicking off a GC.
Two previous commits (one was a re-land), fixed this same bug by pinning
the memory (making it non-detachable), so that the backing store can't
be detached and leave a copy to be freed. In fact, pinning the memory
will cause structuredClone to fail with a TypeError.
1. rdar://123510096, https://bugs.webkit.org/show_bug.cgi?id=270007
2.
rdar://126326144,
https://bugs.webkit.org/show_bug.cgi?id=272607
Those patches are supposed to pin those ArrayBuffers (make them non-detachable)
to keep the memory alive, but it only pins if the node is already playing.
If you set the buffer before calling start() (i.e. when the node is in
UNSCHEDULED_STATE),
the pin is skipped (since isPlayingOrScheduled() is false).
An attacker exploits this by:
1. Setting the buffer (spans cached, no pin)
2. Transferring the channel ArrayBuffer via structuredClone (detaches it)
3. Dropping the copy and triggering GC (backing store freed)
4. Starting playback with loop=true and playbackRate=0
The audio node then reads from freed memory through the dangling spans
during AudioBufferSourceNode::renderFromBuffer() when it reads
m_sourceChannels.
This patch removes the condition to pin the audio buffer
(mark it as non-detachable). Previously, the audio buffer was only
pinned when isPlayingOrScheduled() was true, however, this
patch
pins the memory
unconditionally. This way if the page tries
to call structuredClone, it will now throw a TypeError and
avoid the opportunity to free the backing store.
In addition, this patch adds the following defensive hardenings
in case the same exploit is triggered via another path:
1. Early return if bufferLength == 0 in
AudioBufferSourceNode::renderFromBuffer()
before the freed m_sourceChannels is read. bufferLength would be 0
if the memory was detached (not pinned)
2. Fix the following code that calculates the index into
the audio buffer in AudioBufferSourceNode::renderFromBuffer():
m_virtualReadIndex = std::min(m_virtualReadIndex, static_cast<double>(
bufferLength - 1));
This patch changes the static cast from
static_cast<double>(bufferLength - 1)
to
static_cast<double>(bufferLength) - 1
since bufferLength is unsigned (size_t) so when
bufferLength is 0, it previously could cause the unsigned
(size_t) to underflow
making the upper limit of the std::min
bounds check effectively useless.
3. Add a guard to bounds check readIndex in the case where !pitchRate.
This happens where the attacker sets node.playbackRate.value = 0;
Every other branch already bounds-checks readIndex. This was the
one path that didn't.
As a side note, an alternative approach to fix this bug, could be to
acquire an internal copy of the audio data which is not accessible from JS.
This is described in the webaudio spec and was attempted to be implemented
in a previous fix but caused increased memory usage and the tradeoff to
detach the memory was chosen instead.
See the message and FIXME from
https://flagged.apple.com:443/proxy?t2=dd2I9v9kO4&o=aHR0cHM6Ly9jb21taXRzLndlYmtpdC5vcmcvMjcyNDQ4LjkyNUBzYWZhcmktNzYxOC1icmFuY2g=&emid=0a8dc937-c72e-4a74-a7a7-f75cb2408291&c=11
This also updates an ASSERT in ArrayBuffer::errorMessageForTransfer
from ASSERT(buffer->isLocked()) to
ASSERT(!buffer->isDetachable())
since the regression test is hitting this path and not keeping the buffer
alive via locking, but instead is pinning it (which
isDetachable()
is aware of, unlike isLocked()).
Test: webaudio/audiobuffersource-detached-buffer-crash.html
* LayoutTests/webaudio/audiobuffersource-detached-buffer-crash-expected.txt:
Added.
* LayoutTests/webaudio/audiobuffersource-detached-buffer-crash.html: Added.
* Source/JavaScriptCore/runtime/ArrayBuffer.cpp:
(JSC::errorMessageForTransfer):
* Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp:
(WebCore::AudioBufferSourceNode::renderFromBuffer):
(WebCore::AudioBufferSourceNode::setBufferForBindings):
Originally-landed-as: 305413.815@safari-7624-branch (7aaeec1bad67).
rdar://180436186
Canonical link:
https://flagged.apple.com:443/proxy?t2=Dw3n4o9Qu4&o=aHR0cHM6Ly9jb21taXRzLndlYmtpdC5vcmcvMzA1ODc3Ljg4NkB3ZWJraXRnbGliLzIuNTI=&emid=0a8dc937-c72e-4a74-a7a7-f75cb2408291&c=11
To unsubscribe from
these emails, change your notification settings at
https://github.com/WebKit/WebKit/settings/notifications
