ReviewOS

zig-utils/zig-js

Reflection: remove fixed-hash integer own-key membership

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

Parent: #493 Related: #521, #759

Problem

Interpreter.objectOwnKeysList uses three transient std.AutoHashMapUnmanaged(usize, void) membership tables while merging integer-index keys for String exotic objects, Arrays, and ordinary/host objects.

Canonical array-index names are script-controlled. Zig's auto hash for usize is Wyhash(0, asBytes(key)), so an attacker can precompute an arbitrarily large family that lands in the same probe bucket and amplify Object.keys, Reflect.ownKeys, JSON/property traversal, and every other consumer of [[OwnPropertyKeys]].

The result is already sorted numerically after membership filtering, so a hash table is unnecessary.

Required design

  • Collect all candidate canonical integer-index keys.
  • Sort once by their numeric index as ECMA-262 requires.
  • Collapse adjacent equal canonical indices in place.
  • Preserve String exotic virtual indices, dense/sparse/accessor merging, Array length placement, ordinary string insertion order, symbols, and host-hook overlap semantics exactly.
  • Remove the fixed-seed numeric membership tables and their scratch allocations.
  • Do not add caps, input rejection, fallback seeds, linear duplicate scans, or semantic shortcuts.

This gives deterministic O(n log n) worst-case work and avoids secret state for a surface whose required final order already pays for sorting.

Acceptance

  • All three numeric own-key paths share one sort-and-deduplicate primitive.
  • A deterministic family colliding under the removed default hash is returned once each in exact numeric order.
  • Ordinary objects, Arrays, String exotic objects, sparse/accessor keys, and symbols retain exact own-key semantics.
  • Focused Debug, ReleaseSafe, and TSan tests pass.
  • Full sharded units, relevant Test262 ownKeys/Object.keys/JSON subtrees, threadfuzz, C API, and private ABI gates show no regressions.
  • Commit and closure evidence link exact commands/results.
9 days ago

Implemented and verified on main in fix(reflect): bound integer own-key merging (#861).

What changed:

  • Replaced all three fixed-seed integer membership tables in [[OwnPropertyKeys]] with one shared stable numeric sort plus adjacent canonical-index deduplication.
  • Preserved String exotic virtual keys, dense/sparse/accessor merging, Array length, ordinary string insertion order, symbols, and host overlap semantics.
  • Removed the scratch hash allocations entirely; worst-case ordering work is now deterministic O(n log n) using Zig's documented worst-case-bounded stable sort.
  • Added a 32-index family whose usize encodings all collide in the removed default Wyhash(0, ...) bucket, including reverse-order duplicates and end-to-end ordinary object, Array, and String exotic coverage.

Evidence:

  • focused Debug: 1/1 passed; broader ownKeys: 4/4 passed
  • focused ReleaseSafe: 1/1 passed
  • focused TSan: 1/1 passed, no races/leaks
  • full sharded units: 2,276 passed, 1 skipped, 0 failed, 0 leaked
  • exact-parent Test262 (6bfa5ccb -> 19508195): Reflect/ownKeys 13 -> 13; Object/keys 59 -> 59; Object/getOwnPropertyNames 45 -> 45; JSON/stringify 66 -> 66; all 0 fail and 0 flips
  • deterministic threadfuzz: 400 programs from seed 1, 0 failures
  • public C API: 117/117 declarations complete and public symbols present
  • Home private ABI: 447/447 implemented private symbols; 392/392 linked runtime matrix in Debug, ReleaseSafe, and TSan
  • formatting and diff checks pass

Sign in to comment on this issue.