INT32-tagged number is bit-identical to a class-ref for the same value: a crafted NaN whose payload equals a REGISTERED class id is indistinguishable from a real class reference #10601

Activity

1. proggeramlug

   added

   package-auditFound by the 2026 package audit: compiling real npm packages from source instead of native bindingsFound by the 2026 package audit: compiling real npm packages from source instead of native bindings

   on Sep 18, 2026
2. proggeramlug commented on Sep 18, 2026

   

   proggeramlug

   on Sep 18, 2026

   ContributorAuthor

   More actions

   Investigation result: diagnosed, not fixed (architectural)

   Branch (base: fix/10478-10479-instanceof-value-kinds @ a1a9628f1c — the #10592 head that has class_ref_id): wip/10601-dataview-nan-instanceof, containing a test-files/issue_10601_repro_*.ts harness (six scripts, each with an inline writeup) that isolates every claim below.

   The position-dependence is not codegen/IC/inlining — it's registered-class count

   --print-hir on the full 204-line test_gap_10479_instanceof_value_kinds.ts confirms class ids are assigned sequentially from 1 in declaration order (lower_module starts start_class_id at 1; fresh_class() increments once per class lowered). Up through the point badRhs runs, five classes have been declared: K=1, A=2, B=3, Even=4, ShortString=5.

   The DataView PoC's embedded id is 5. issue_10601_repro_class_id_confirm.ts proves the crafted value is being dispatched as ShortString's ref, not just landing on some registered id: on Perry it prints T F F for ("uri" instanceof craftedNaN), (newA instanceof craftedNaN), (null instanceof craftedNaN) — an exact match for ShortString[Symbol.hasInstance]("uri")=true, (object)=false, (null)=false (typeof v === "string" && v.length < 6), independently printed in the same file. In the minimal 2-entry repro only A (id 1) is ever declared, so id 5 is never registered and the value correctly falls through to TypeError — same binary, same code path, different runtime state. None of the codegen/IC/constant-folding candidates from the investigation brief apply; this reproduces or fails to reproduce purely as a function of how many classes had been declared before the check runs.

   Root cause: INT32_TAG-boxed numbers and class refs are bit-identical

   JSValue::int32() (crates/perry-runtime/src/value/jsvalue.rs): INT32_TAG | (value as u32 as u64).
   class_constructor_ref_value() (crates/perry-runtime/src/object/native_module/class_ref_values.rs): 0x7FFE_0000_0000_0000 | (class_id as u64 & 0xFFFF_FFFF).

   These are the same formula. A plain boxed int32 numeric value n is bit-for-bit identical to the constructor ref for class id n. class_ref_id's is_class_id_registered check (#10592) rejects the overwhelming majority of crafted payloads — anything that doesn't happen to match a currently-registered id — but it cannot reject a payload that does match one, because there is no bit left in the encoding to say "this is genuinely a codegen-emitted ref" vs "this is just a number that happens to equal a registered id." That distinction doesn't exist in the wire format.

   I checked whether this is reachable without DataView (issue_10601_repro_ordinary_int_no_collision.ts): a loop-accumulated int32 and an Int32Array element read, in the same 5-class context, both correctly throw on this build — ordinary numeric codegen paths apparently normalize through a real f64 conversion rather than the INT32_TAG bit-OR trick before a value becomes generically observable. So today this needs DataView's raw byte write to reach; it is not (as far as this session checked) reachable through ordinary arithmetic.

   Why I didn't fix it

   Full fix (extra discriminator bit in the class-ref encoding): ordinary INT32-boxed numbers never set bits 32+ (only 32 bits used), so an unused bit (e.g. bit 33, alongside the existing CLASS_PROTOTYPE_REF_FLAG at bit 32) could mark "genuinely a class ref." But the encoding is baked in directly (not through one shared constructor) at 10+ call sites across perry-codegen — expr/property_get.rs, expr/arrays_finds.rs, expr/dyn_extern_i18n.rs, stmt/let_stmt.rs, codegen/closure.rs, codegen/method_static.rs, codegen/static_fields.rs, codegen/helpers.rs, lower_call/new.rs (×2), lower_call/property_get/static_dispatch.rs — plus the runtime encoder, plus several INT32_TAG_TOP16_I64/INT32_TAG_I64 fast-path comparisons (expr/compare.rs, expr/index_get.rs, expr/index_set.rs, codegen/typed_abi.rs, codegen/typed_entry.rs) that would need auditing for whether they also assume the old 2-state encoding. crates/perry-runtime/src/value/tags.rs documents this exact space as "part of the wire-level contract... renumbering is an ABI break." This is a coordinated, ABI-affecting change across two crates needing full gap-suite validation, not a surgical diff.

   Narrower "purify the NaN payload on DataView read" fix (the standard purifyNaN/PNaN technique): I built and ran it against the Node oracle before writing any code (issue_10601_repro_dataview_roundtrip_preserves_bits.ts) and it doesn't match Node's actual behavior. Node preserves the crafted value's exact bits through a getFloat64/setFloat64 round-trip (7ffe0000 5 survives unchanged) while still correctly reporting Number.isNaN() === true — V8 doesn't use NaN-boxing for its own tagging (HeapNumbers are boxed behind an ordinary tagged pointer), so it has no analogous ambiguity to resolve by canonicalizing. Squashing the payload to a canonical NaN in Perry would fix Number.isNaN for this exact repro but diverge from Node on any round-trip-sensitive case — trading the reported symptom for a different, currently-untested one. A real fix needs NaN classification to be correct without discarding the payload, which is the same registered/valid-id disambiguation problem #10592 solved for instanceof — but for every consumer of a NaN-boxed value (typeof, Number.isNaN, and implicitly everything else), not just one. Per the investigation brief, I did not implement this.

   I did not touch class_ref_id/is_class_id_registered (#10592's fix is correct as-is and should not be weakened to paper over this).

   Side note (not investigated further, not filed)

   Float64Array element reads of the same crafted bytes hit a different, apparently V8-intrinsic quirk: Number.isNaN(new Float64Array(dv.buffer)[0]) is false on Node itself for this payload (likely V8's own reserved "hole" NaN space), while a same-type Float64Array construction and plain element-to-element assignment both preserve the raw bits exactly. I didn't check what Perry currently does here since matching Node (however unintuitive) is the bar, not spec purity, and this looked like a distinct code path from the one in this issue.

   What's next

   Leaving this as diagnosed-not-fixed per the investigation brief's guidance for an architectural-scope finding. If a wire-format change to the class-ref encoding is ever undertaken for other reasons, folding in a genuine-class-ref discriminator bit at that time would close this for free.
3. proggeramlug

   changed the title [-]Crafted NaN payload (DataView) can collide with Perry's INT32-class-ref tag band, in a context-dependent way[/-] [+]INT32-tagged number is bit-identical to a class-ref for the same value: a crafted NaN whose payload equals a REGISTERED class id is indistinguishable from a real class reference[/+] on Sep 18, 2026
4. proggeramlug commented on Sep 18, 2026

   

   proggeramlug

   on Sep 18, 2026

   ContributorAuthor

   More actions

   Retitled to name the root cause established below rather than the symptom. Summary for anyone triaging: this is an encoding-level ambiguity in the NaN-boxed wire format, not a codegen or inline-cache interaction — my original framing (four named dispatch sites, position-dependent optimization) was wrong on both counts.

   The practical consequence is that is_class_id_registered is the strongest check possible within the current encoding: it rejects unregistered ids, but a payload matching a live class id is genuinely indistinguishable from a codegen-emitted class ref. Closing that needs a discriminator bit, which tags.rs documents as an ABI-level contract — a deliberate change, not a patch.

   Noted as explicitly NOT the fix: canonicalizing NaN on DataView read. Node preserves the crafted bits through a getFloat64/setFloat64 round-trip while still reporting Number.isNaN() === true, so purifying would trade this symptom for a fresh divergence from the oracle.
5. proggeramlug commented on Sep 20, 2026

   

   proggeramlug

   on Sep 20, 2026

   ContributorAuthor

   More actions

   Fixed on main (verified on v0.5.1617). Both symptoms are gone.

   Direct reproducer now gives number true, matching node — it was number false. The harder second symptom, the crafted value as the 7th badRhs entry in the nested for…of, was rebuilt exactly and all 21 lines match node.

   Worth recording for whoever maintains the coverage: the in-tree gap test test_gap_10479_instanceof_value_kinds.ts no longer carries that 7th entry, so it does not cover the crafted-NaN case. It passes 81/81 on its own terms, but the case this issue found is only covered by this issue. If that case matters, it wants a fixture.
6. proggeramlug

   closed this as completedon Sep 20, 2026