ReviewOS

zig-utils/zig-js

Security: bound structured-clone depth and wire counts

#39
Closed chris opened this 9 days ago ·
9 days ago

Parent: #1

Problem

structured_clone.serialize() and deserialize() recurse directly through object/wrapper/view graphs with no explicit depth budget. A deeply nested JavaScript value or malformed byte stream can exhaust the host stack instead of producing a catchable clone failure.

The wire format also stores graph IDs, string lengths, collection counts, array element counts, and view offsets/lengths in fixed-width integers. Several serializer increments/casts and deserializer u64 -> usize casts currently rely on values fitting; oversized source state or hostile bytes can trap, wrap, or drive disproportionate work rather than fail closed.

Goal

Give structured clone an explicit, shared complexity contract for serialization, structural preflight, cleanup parsing, and deserialization. Reject excessive nesting and unrepresentable/impossible counts before recursion, allocation, or loops become unsafe.

Acceptance criteria

  • Serialization and deserialization enforce an explicit maximum nesting depth and return a catchable clone error instead of exhausting the native stack.
  • Structural preflight and releaseSerialized() use the same depth bound.
  • Serializer graph IDs, string lengths, property counts, collection counts, and array counts use checked conversion/increment.
  • Deserializer u64 -> usize conversions fail closed on narrower targets.
  • Wire counts are rejected when they are impossible for the remaining payload before large loops or allocations.
  • Focused tests cover depth-at-limit, depth-over-limit, oversized counts/lengths, and cleanup of retained SAB tokens in rejected deep/trailing payloads.
  • Worker/structuredClone regression tests remain green and the security limits are documented.
9 days ago

First security chunk landed in 29b00b4: structured-clone bytes are now versioned frames with an explicit payload length and SAB ownership manifest. Payload SAB tags use canonical manifest indices, so forged/out-of-order/duplicate/replayed references fail closed, while releaseSerialized() can release every retained token without recursively parsing a valid payload (including truncated or concatenated frames). Focused structured-clone tests passed locally, including forged/replay/trailing-token and truncated-frame cleanup coverage. CI: https://github.com/zig-utils/zig-js/actions/runs/29221524627

9 days ago

Framed SAB ownership in 29b00b4 passed the full production gate (31 success, 1 nightly-only skip, 0 failures): https://github.com/zig-utils/zig-js/actions/runs/29221524627

Second chunk 59286f8 is now on main. Serialization, structural preflight, and deserialization share a documented 256-level nesting ceiling; depth-at-limit succeeds, depth-over-limit is catchable, and an over-depth payload with an unreachable SAB tag still releases its manifest-owned retain without recursive cleanup. Current gate: https://github.com/zig-utils/zig-js/actions/runs/29223006097

9 days ago

The shared-depth chunk 59286f8 passed the full production gate (31 success, 1 nightly-only skip, 0 failures): https://github.com/zig-utils/zig-js/actions/runs/29223006097

Final acceptance chunk 71378ff is now on main. It checks serializer object IDs, string widths, property/collection/array counts, offsets and lengths; rejects noncanonical flags, references, tracking-view fields, invalid view bounds, and counts impossible for the remaining payload before large loops; and uses checked u64-to-usize conversion. Focused tests cover oversized string/collection/array fields, invalid references/flags, ResizableArrayBuffer tracking views, and the existing depth/SAB cleanup cases. Current gate: https://github.com/zig-utils/zig-js/actions/runs/29224680904

9 days ago

The 71378ff matrix found one unrelated latent runtime race in the existing api/condition-async-wait.js corpus case (asyncDrive running-state handoff); all structured-clone/unit jobs and 28 other required jobs were green. The race is tracked and fixed separately in #41 / 6ef776c with a deterministic normal+TSan witness. #39 remains open until the combined replacement matrix is green: https://github.com/zig-utils/zig-js/actions/runs/29226671310

9 days ago

Accepted and complete across 29b00b4, 59286f8, and 71378ff. Structured-clone frames now have rejection-safe SAB manifests; serialization, preflight, and deserialization share the documented depth bound; fixed-width IDs/lengths/counts and host-size conversions are checked; impossible/noncanonical wire fields fail before unsafe loops or view construction; and focused depth/count/SAB cleanup/tracking-view tests are green. The combined replacement production gate passed 31 jobs with 1 nightly-only skip and 0 failures: https://github.com/zig-utils/zig-js/actions/runs/29226671310

Sign in to comment on this issue.