Bug 291070 – AudioData.copyTo() ignores frameCount and crashes the tab if target is too small to fit whole AudioData

• Component: Web Audio
• Status: RESOLVED FIXED

[Szymon Witamborski]

Created attachment 474827 [details]
Proof of concept fix. Take into account copyElementCount when calculating subSource

AudioData.copyTo ignores frameCount and it will overwrite the target. In case the target is too small to fit the whole AudioData, the tab will crash.

Tested on: 
- Safari TP 216
- Safari 18.4 (with audio codecs feature flag)
- local build (checked out 2025-04-03, commit a1eacd4a9f8194738ded60b1dba19f23b70825b7) on macOS 15.4, M1 MBP

I'm working on a PR (needs tests) but attaching a proof of concept patch.

Repro:

1) case 1, no crash but incorrect result

const data = new AudioData({
  format: 'f32-planar',
  sampleRate: 48000,
  numberOfChannels: 1,
  numberOfFrames: 5,
  data: new Float32Array([1, 2, 3, 4, 5]),
  timestamp: 0,
});
const output = new Float32Array(data.numberOfFrames);
data.copyTo(output, {
  planeIndex: 0,
  frameOffset: 0,
  frameCount: data.numberOfFrames - 2,
});
data.close();
console.log(`copied. expected: 1,2,3,0,0. result: ${output}`);

Will output:

copied. expected: 1,2,3,0,0. result: 1,2,3,4,5

So there are two extraneous elements being copied despite of asking to copy only `data.numberOfFrames - 2`

2) destination made smaller with `subarray`. It should fit the request but copyTo will try to copy the whole AudioData and crash:

const data = new AudioData({
  format: 'f32-planar',
  sampleRate: 48000,
  numberOfChannels: 1,
  numberOfFrames: 5,
  data: new Float32Array([1, 2, 3, 4, 5]),
  timestamp: 0,
});
const output = new Float32Array(data.numberOfFrames);
data.copyTo(output.subarray(0, data.numberOfFrames - 2), {
  planeIndex: 0,
  frameOffset: 0,
  frameCount: data.numberOfFrames - 2,
});
data.close();
console.log(`copied. expected: 1,2,3,0,0. result: ${output}`);

The tab crashes on https://github.com/WebKit/WebKit/blob/2f82f9ba319643e6752f5055b6b1f92056c9429d/Source/WebCore/platform/audio/cocoa/PlatformRawAudioDataCocoa.cpp#L259

Interactive version: https://codepen.io/brainshave/pen/gbOqxym
- "copy fragment" is the one that doesn't crash but gives wrong result
- red buttons trigger the crash

[Radar WebKit Bug Importer]

<rdar://problem/148601714>

[Jer Noble]

Thanks for the report, and especially the reproduction steps! Did you want to turn this into a PR yourself? You've already got a test-case written in that code-pen. 

Otherwise, I'd be happy to take this patch over and convert it into a PR.

[Szymon Witamborski]

Pull request: https://github.com/WebKit/WebKit/pull/43730

[Szymon Witamborski]

Created a PR here: https://github.com/WebKit/WebKit/pull/43730

When writing tests, I noticed a few things and I added two additional changes there:

1. noticed that stereo audiodata wasn't using the memcpy path and I think it should be relatively easy to make it do so
2. noticed that the optimisation for using memcpy for interleaved data might be copying data for other channels than `planeIndex`

I've added some comments on the PR itself where there's more relevant context but just in general, these additional changes can be considered optional.

If there was someone on the WebKit's side wanting to have a look, that would be greatly appreciated, thanks!

While I was writing this comment I noticed that some tests are failing on at last one platform (WPE). Looks like I will need to look into that.

(Side note: Looks like the git-webkit script errored when trying to set assignment on the PR. But I can't do it from the web either so I guess I just don't have that permission on the repo.)

[Szymon Witamborski]

> 2. noticed that the optimisation for using memcpy for interleaved data might be copying data for other channels than `planeIndex`

Correction: after closer reading of the spec, I think this behaviour was indeed correct. Reverted a change on the PR that tried to copy only one interleaved channel at once.

Ref: https://w3c.github.io/webcodecs/#compute-copy-element-count
Specifically points 3 and 11:

> 3. If destFormat describes an interleaved AudioSampleFormat and options.planeIndex is greater than 0, throw a RangeError.
> …
> 11. If destFormat describes an interleaved AudioSampleFormat, multiply elementCount by [[number of channels]]

[EWS]

Committed 296217@main (e1a2977e3a83): <https://commits.webkit.org/296217@main>

Reviewed commits have been landed. Closing PR #43730 and removing active labels.