ReviewOS

zig-utils/zig-js

Frontend: bound private-name duplicate validation

#540
Closed chris opened this 10 days ago ·
10 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

10 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.

10 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.