Skip to content

Runtime: Copy only the view's window in swjs_load_typed_array - #810

Open
kateinoigakukun wants to merge 1 commit into
mainfrom
fix/load-typed-array-byteoffset
Open

Runtime: Copy only the view's window in swjs_load_typed_array#810
kateinoigakukun wants to merge 1 commit into
mainfrom
fix/load-typed-array-byteoffset

Conversation

@kateinoigakukun

@kateinoigakukun kateinoigakukun commented Sep 4, 2026

Copy link
Copy Markdown
Member

The bug

swjs_load_typed_array views the whole backing ArrayBuffer instead of the window the view it was handed describes:

swjs_load_typed_array: (ref: ref, buffer: pointer) => {
    const memory = this.memory;
    const typedArray = memory.getObject(ref);
    const bytes = new Uint8Array(typedArray.buffer);   // <- ignores byteOffset/byteLength
    this.getUint8Array().set(bytes, buffer >>> 0);
},

typedArray.buffer is the backing ArrayBuffer, which may be larger than the view and may start before it. Any view produced by subarray or new Uint8Array(buffer, offset, length) has a non-zero byteOffset, and the copy ignores it.

Minimal repro

let backing = JSObject.global.ArrayBuffer.function!.new(32)
let whole = JSTypedArray<UInt8>(unsafelyWrapping: JSObject.global.Uint8Array.function!.new(backing))
for i in 0..<32 { whole[i] = UInt8(0xA0 + i) }

// A view over bytes 8..<16 of that buffer.
let view = JSTypedArray<UInt8>(
    unsafelyWrapping: JSObject.global.Uint8Array.function!.new(backing, 8, 8)
)

print(view.withUnsafeBytes { Array($0) })
// expected: [0xA8, 0xA9, 0xAA, 0xAB, 0xAC, 0xAD, 0xAE, 0xAF]
// actual:   [0xA0, 0xA1, 0xA2, 0xA3, 0xA4, 0xA5, 0xA6, 0xA7]

The bytes come back shifted by byteOffset. Nothing throws — the result is just quietly wrong, which is what makes this class of bug expensive to find downstream.

Why this is a memory-safety issue, not only a correctness one

The guest sizes the destination from the view's element count, never from the backing buffer:

  • Sources/JavaScriptKit/BasicObjects/JSTypedArray.swiftlength is jsObject["length"], the view's element count.
  • copyMemory(to:) preconditions only buffer.count >= length before calling swjs_load_typed_array.
  • withUnsafeBytes / withUnsafeBytesAsync allocate exactly capacity: length.

So whenever the view is smaller than its backing buffer, JavaScript writes buffer.byteLength bytes into a destination sized for view.byteLength. The excess lands in whatever follows the destination in linear memory. The precondition cannot catch it — it is satisfied, and the overrun happens on the JS side of the boundary. There is no error, no trap, and no diagnostic; just corrupted memory after the buffer.

It is reachable from ordinary API use: JSTypedArray(unsafelyWrapping:) over any JS-side view, or a Uint8Array handed in from JavaScript that happens to be a subarray.

The fix

Respect the view's window:

const bytes = new Uint8Array(
    typedArray.buffer,
    typedArray.byteOffset,
    typedArray.byteLength,
);

This is the generic solution for every element type the runtime supports — all TypedArray types and DataView expose byteOffset and byteLength, and for a whole-buffer view (the common case, and everything swjs_create_typed_array produces) it is (buffer, 0, buffer.byteLength), identical to today's behaviour.

Follow-up not included here

The runtime still trusts that the guest sized the destination correctly; it has no way to know how many bytes the caller allocated. A defensive bounds check would need the length plumbed through the swjs_load_typed_array ABI, which is a larger change than this fix warrants, so it is left out deliberately. Worth considering separately if the ABI is revised.

Testing

Plugins/PackageToJS/Templates/runtime.mjs was regenerated with make regenerate_swiftpm_resources, not hand-edited; its diff is exactly the change to Runtime/src/index.ts. runtime.d.ts is unchanged (no type surface change).

New tests, both of which fail before the fix and pass after:

  • Runtime/test/load-typed-array.test.ts — drives swjs_load_typed_array against a real WebAssembly.Memory, following the pattern in pointer-normalization.test.ts. Covers Uint8Array and Int32Array views with a non-zero byteOffset, a DataView, the no-overrun property (guest memory past the view's byteLength must keep its sentinel), and a whole-buffer view as a control.

    Before the fix: 4 failed, 1 passed (the whole-buffer control). After: 5 passed.

  • Tests/JavaScriptKitTests/JSTypedArrayTests.swifttestTypedArrayWithByteOffset and testMultiByteTypedArrayWithByteOffset assert the same two properties through the Swift API: correct bytes via withUnsafeBytes, and that copyMemory(to:) leaves sentinel values past the destination untouched.

    With the runtime reverted to new Uint8Array(typedArray.buffer): 194 passed, 2 failed, 196 total, and the overrun assertions name the corrupted slots directly (copyMemory(to:) wrote past the destination at 8at 31). With the fix: 196 passed, 196 total.

Ran locally: npm run test:runtime, npx prettier --check Runtime/src, npm run check:bridgejs-dts, ./Utilities/format.swift (no changes), swift test --package-path ./Plugins/PackageToJS, swift test --package-path ./Plugins/BridgeJS, and make unittest BUILD_SYSTEM=native against a wasm32-unknown-wasip1 SDK (all suites pass, including JSTypedArrayTests).

🤖 Generated with Claude Code

`new Uint8Array(typedArray.buffer)` views the whole backing `ArrayBuffer`,
ignoring the `byteOffset` and `byteLength` of the view it was handed. A view
with a non-zero offset (anything from `subarray`, or
`new Uint8Array(buffer, offset, length)`) therefore arrives in the guest
shifted by that offset, silently wrong rather than failing.

It is also a memory-safety bug: `JSTypedArray.copyMemory(to:)` sizes the
destination from the view's own `length`, so whenever the view is smaller
than its backing buffer the runtime writes past the end of the destination
and corrupts whatever follows it in linear memory.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@kateinoigakukun
kateinoigakukun force-pushed the fix/load-typed-array-byteoffset branch from d8519f7 to f249c50 Compare September 4, 2026 06:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant