ReviewOS

zig-utils/zig-js

Frontend: bound private-name duplicate validation

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

Parent: #493 Related: #496, #539, #460, #461

Problem

Parser.checkPrivateNames compares each private class member against every earlier member. A class with N distinct private fields therefore performs O(N²) exact-name comparisons before execution, giving attacker-controlled source a deterministic parser CPU denial-of-service surface and obscuring the allocation work isolated in #539.

Scope

  • Add parser-only 1K/2K/4K private-member growth rows with exact decoded member-name/index checks plus duplicate and accessor-pair controls.
  • Replace the nested scan with a collision-independent deterministic index/order that gives a declared O(N log N) or better bound without weakening exact byte equality.
  • Preserve the sole allowed duplicate form: one complementary private getter/setter pair at the same static/instance placement.
  • Preserve rejection of fields/method duplicates, get/get, set/set, third declarations, static/instance splits, and #constructor.

Security and correctness

  • Do not rely on predictable hash placement for attacker-controlled names; exact bytes decide groups.
  • Transient index storage must use freeable parser scratch backing and be released on success and every error.
  • Allocation failure must propagate atomically; no member state may be published before validation completes.
  • Do not impose an arbitrary private-member limit, skip early errors, special-case benchmark names, or change source order.

Acceptance

  • Benchmark source and exact member-name/semantic controls land before parser changes.
  • 1K/2K/4K work evidence proves the declared bound and materially reduces exact-parent CPU/instructions.
  • Every allowed/rejected getter/setter/field/method/static/constructor combination and OOM boundary is covered.
  • Parser/frontend suites, class/private-name test262, forced tree-walker/VM/eval paths, full units, TSan, and sanitizer gates pass without regressions.

Blocks: #539

9 days ago

Benchmark-first commit landed on main as 56233857: bench(frontend): add private-name growth rows (#540).

The parser-only runner now freezes one real class with 1,024/2,048/4,096 distinct private fields plus a 4,096 escaped-private control. Source construction is outside warmup and timing. Each parse validates the class declaration, exact member count, field shape, decoded #field prefix, and numeric suffix equal to source order before summing every private-name byte.

Frozen one-job checksums:

  • private 1,024: 10,154
  • private 2,048: 21,418
  • private 4,096: 43,946
  • escaped private 4,096: 43,946

No parser behavior or performance claim is included in this commit. The implementation will be compared against this exact parent.

9 days ago

Completed on main in f888b717, following the benchmark fixture in 56233857 and the collision matrix in 2ec8096c.

The parser now groups exact private-name bytes through a deterministic O(N log N) sort of scratch-backed member indexes. It does not depend on attacker-predictable hash placement, does not reorder AST members, preserves only the valid complementary getter/setter pair, and propagates scratch OOM.

Seven order-balanced exact-parent pairs against 2ec8096 (ReleaseFast, one timed parse after ten warmups, frozen checksums):

  • 1,024 fields: 0.874 ms -> 0.108 ms; retired instructions 0.163x parent.
  • 2,048 fields: 3.000 ms -> 0.219 ms; retired instructions 0.089x.
  • 4,096 fields: 13.033 ms -> 0.450 ms; retired instructions 0.036x.
  • escaped 4,096 control: 13.514 ms -> 0.595 ms; retired instructions 0.041x.
  • 1K -> 4K growth: 14.91x parent wall time versus 4.17x candidate.
  • median peak RSS stayed within 0.6%.

Correctness gates:

  • private collision/accessor/static/constructor matrix plus direct scratch OOM: pass.
  • parser units: 32/32; parser TSan: 32/32.
  • focused frontend: 12/12.
  • affected test262 class-element surface: 2,962 pass -> 2,962 pass; flips 0, regressions 0.
  • full sharded units: 1,599 passed, 1 skipped, 0 failed, 0 leaked.

This removes the blocker recorded on #539.

Sign in to comment on this issue.