2043032 - Differential behavior: array.init_data skips destination bounds check on dropped data segment

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, defect, P3)

Description

[Christian Holler (:decoder)]

Summary

array.init_data with a dropped data segment skips the destination array bounds check, accepting an out-of-bounds destination index that should trap per the WebAssembly GC spec. V8 correctly traps; SpiderMonkey silently succeeds.

Root Cause

In js/src/wasm/WasmInstance.cpp, Instance::arrayInitData (line ~1722) takes an early-return path when the data segment has been dropped (null pointer). The dropped-segment check at line ~1742:

if (!seg) {
  return 0;  // returns success without checking destination index
}

returns before ArrayCopyFromData (line ~1563) ever runs, so the destination array range check (arrayIndex + numElements > numElements_) is never performed. The spec requires this check unconditionally — even when n == 0 and even when the segment is dropped, dstIndex > len(array) must trap.

This is only reachable when numElements == 0 (the earlier (!seg && numElements != 0) guard forces this), so no out-of-bounds copy actually occurs — the bug is a spec-compliance / correctness defect, not a memory-safety violation. It is the wasm-GC array analogue of the known memory.init dropped-segment bounds-check-skip pattern.

Note the asymmetry: the element-segment sibling arrayInitElem does NOT have this bug, because dropped element segments become empty (length-0) rather than null, so ArrayCopyFromElem always runs its destination check.

Reproduction

Tested on mozilla-central build 20260521-b754e06486cf (fuzzing-asan-opt, Linux x86-64).

$ m-c-fuzzing-asan-opt/dist/bin/js --fuzzing-safe poc.js
dropped-segment OOB index (idx=10, len=1, n=0): no-trap
live-segment    OOB index (idx=10, len=1, n=0): trap
RESULT: BUG PRESENT - dropped-segment array.init_data skipped the array bounds check

$ d8 poc.js
dropped-segment OOB index (idx=10, len=1, n=0): trap
live-segment    OOB index (idx=10, len=1, n=0): trap
RESULT: spec-correct - dropped-segment array.init_data trapped

No special flags or prefs are required. Reproduces on baseline, Ion, and with --test-wasm-await-tier2.

The testcase poc.js builds a wasm module with a (array (mut i32)) type and a passive data segment, creates a length-1 array, drops the segment via data.drop, then calls array.init_data with destination index 10 (out of bounds) and length 0.

Suggested Fix

Perform the destination array index range check (index + numElements > array.numElements_) before the if (!seg) return 0; early return, or route the dropped-segment case through ArrayCopyFromData with an explicit zero-length segment so the existing destination check always runs (mirroring how arrayInitElem always reaches ArrayCopyFromElem).

Comment 1

[Christian Holler (:decoder)]

Attachment: poc.js - array.init_data dropped-segment bounds check bypass testcase

Comment 2

[Ryan Hunt [:rhunt]]

Is this a new case or was this just fixed?

Comment 3

[Ben Visness [:bvisness]]

Heh, this one is different. The other one was about memory.init.

Comment 4

[Ben Visness [:bvisness]]

Like bug 2042287, this is not a security bug, it is simply a case where we no-op when we should trap. No out of bounds access actually occurs. Can we please declassify this?

Comment 5

[Ben Visness [:bvisness]]

Attachment: Bug 2043032: Better spec compliance for array bounds checks. r=rhunt

The array.(new|init)_(data|elem) instructions had some slightly
incorrect behavior around bounds checks, where a zero-length copy out of
bounds would no-op instead of trapping. The spec mandates that the
bounds check is performed regardless of the length of the copy.

Comment 6

[Ben Visness [:bvisness]]

Attachment: Bug 2043032: Improve test coverage for other wasm bounds checks. r=rhunt

We were seemingly missing test coverage (within our non-spec tests,
anyway) for some of the fussier elements of bounds checks on various
range operations. This just fills them in for completeness. All of them
pass with no changes.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/ae0e802bd400
https://hg.mozilla.org/integration/autoland/rev/fbbdaee4ef52
Better spec compliance for array bounds checks. r=rhunt
https://github.com/mozilla-firefox/firefox/commit/db18c2618c81
https://hg.mozilla.org/integration/autoland/rev/43a205233f16
Improve test coverage for other wasm bounds checks. r=rhunt

Comment 8

[Goloman Adrian[:agoloman]]

https://hg.mozilla.org/mozilla-central/rev/fbbdaee4ef52
https://hg.mozilla.org/mozilla-central/rev/43a205233f16