fix(json): stop dereferencing positive subnormal doubles as heap pointers

changelog.d/7447-json-subnormal-deref.md

@@ -0,0 +1 @@
+- **`JSON.stringify` no longer dereferences subnormal numbers as heap pointers.** `JSON.stringify(1e-317)` segfaulted and `5e-324`/`1e-320` printed `null`, all reachable from untrusted input via `JSON.stringify(JSON.parse(text))`. Two independent admission tests were at fault — `json/mod.rs::is_raw_pointer` (bit-for-bit the IEEE-754 positive-subnormal predicate) and `stringify_api.rs::try_stringify_lazy_array`'s `top16 == 0`, which runs first, so fixing either alone only moved the crash. A raw pointer and a positive subnormal share bit patterns by construction, so both sites now decide by GC allocation membership (`ptr_is_tracked_heap_object`, dereference-free page-map and registry lookups) instead of by bit pattern. (#7447)

crates/perry-runtime/src/json/mod.rs

@@ -549,12 +549,62 @@ pub(crate) unsafe fn str_from_header<'a>(ptr: *const StringHeader) -> Option<&'a
     Some(std::str::from_utf8_unchecked(bytes))
 }
 
+/// Minimum alignment of a GC allocation's *user* address. Every arena object
+/// is `header + GC_HEADER_SIZE` with an 8-aligned header (`arena_alloc_gc`
+/// pads `total` to `max(align, 8)`), and `gc_malloc` inherits the system
+/// allocator's ≥16-byte alignment, so a misaligned word is never one.
+const RAW_POINTER_ALIGN: u64 = 8;
+
+/// True when `bits` carry no NaN-box tag and *could* address a heap object:
+/// no tag bits set, non-zero, and [`RAW_POINTER_ALIGN`]-aligned.
+///
+/// NECESSARY, NEVER SUFFICIENT — this is a cheap pre-filter, not a decision.
+/// Call sites must use [`is_raw_pointer`]; see its doc for why.
+#[inline]
+pub(crate) fn untagged_pointer_bits(bits: u64) -> bool {
+    // Every NaN-box tag lives in the top 12 bits (`0x7FF9`..=`0x7FFF`, plus the
+    // sign bit for negative NaNs), so an untagged pointer has `bits >> 52 == 0`.
+    // `0` is `+0.0`, never a pointer.
+    bits >> 52 == 0 && bits != 0 && bits % RAW_POINTER_ALIGN == 0
+}
+
+/// True when `bits` — which carry no NaN-box tag — name a heap object that the
+/// JSON walk may dereference.
+///
+/// ## Why the bit pattern alone is not an answer (remote-triggerable SIGSEGV)
+///
+/// The historical test was `exponent == 0 && mantissa != 0 && sign == 0`, which
+/// is bit-for-bit the IEEE-754 definition of a **positive subnormal double**.
+/// Every positive denormal reaching `JSON.stringify` was therefore classified
+/// as an untagged heap pointer and dereferenced. `JSON.stringify(1e-317)`
+/// SIGSEGV'd and `JSON.stringify(5e-324)` printed `null` — both reachable from
+/// untrusted input as `JSON.stringify(JSON.parse(text))`.
+///
+/// No bit-pattern test can fix that, and narrowing the mask is not a fix. A raw
+/// untagged pointer and a positive subnormal occupy the *same* bit patterns by
+/// construction; that is a property of the value representation, not a wrong
+/// constant. `addr_class::is_plausible_heap_addr` does not help either — it is
+/// `0x1000..0x8000_0000_0000` on macOS/Linux, while positive subnormals span
+/// `0..0x0010_0000_0000_0000`, so the overlap is nearly the whole band. The
+/// #3576 module-slot object pointers this branch exists to serve live squarely
+/// inside the subnormal range, so any bit test wide enough to admit them still
+/// admits denormals.
+///
+/// The decision therefore has to come from *outside* the bits: ask whether the
+/// address is an allocation the GC actually tracks.
+/// [`stringify::ptr_is_tracked_heap_object`] answers that from the arena page
+/// map and the malloc-object registry — set-membership lookups, no dereference
+/// — so a denormal is rejected without ever touching memory and falls through
+/// to the `write_number` arm that should have had it all along.
+///
+/// What remains is exactly the irreducible ambiguity of the representation: a
+/// denormal whose bits *equal* a live tracked allocation's address is still
+/// indistinguishable from that pointer. Removing that residue means tagging
+/// these values at the producer (see `value::equality::normalize_raw_object_bits`
+/// / #3576), not writing a better predicate here.
 #[inline]
-pub(crate) fn is_raw_pointer(bits: u64) -> bool {
-    let exponent = (bits >> 52) & 0x7FF;
-    let mantissa = bits & 0x000F_FFFF_FFFF_FFFF;
-    let sign = bits >> 63;
-    exponent == 0 && mantissa != 0 && sign == 0
+pub(crate) unsafe fn is_raw_pointer(bits: u64) -> bool {
+    untagged_pointer_bits(bits) && stringify::ptr_is_tracked_heap_object(bits as *const u8)
 }
 
 #[inline]
@@ -654,6 +704,93 @@ mod tests {
         assert!(unsafe { js_json_parse_result(text) }.is_ok());
     }
 
+    /// The bare bit test is *exactly* the IEEE-754 positive-subnormal
+    /// predicate. Pinning that here so nobody "simplifies"
+    /// `untagged_pointer_bits` back into a sufficient condition: whatever it
+    /// accepts, it accepts real `Number`s too, which is why `is_raw_pointer`
+    /// must consult the GC's allocation set.
+    #[test]
+    fn untagged_pointer_bits_cannot_separate_pointers_from_subnormals() {
+        for v in [5e-324f64, 1e-320, 1e-317, 1e-310, 2.2250738585072009e-308] {
+            assert!(v.is_subnormal() && v > 0.0);
+            let bits = v.to_bits();
+            let old_predicate = (bits >> 52) & 0x7FF == 0 && bits & 0x000F_FFFF_FFFF_FFFF != 0;
+            assert!(
+                old_predicate,
+                "{v:e} is a positive subnormal, so the pre-fix bit test called it a pointer"
+            );
+        }
+        // ...and the bit test still admits the aligned ones. Only the
+        // GC-tracking check below rejects them.
+        assert!(untagged_pointer_bits(0x0000_0012_3456_7888));
+        assert!(!untagged_pointer_bits(0));
+        assert!(!untagged_pointer_bits(1.5f64.to_bits()));
+        assert!(!untagged_pointer_bits(POINTER_TAG | 0x1000));
+    }
+
+    /// A positive subnormal `Number` must serialize as that number. Pre-fix
+    /// `5e-324` printed `null` (its bits, `1`, landed in the handle band) and
+    /// `1e-317` SIGSEGV'd (its bits pass every magnitude check but address an
+    /// unmapped page). Both are reachable from untrusted input via
+    /// `JSON.stringify(JSON.parse(text))`.
+    #[test]
+    fn stringify_positive_subnormal_is_a_number_not_a_pointer() {
+        unsafe {
+            for (value, expected) in [
+                (5e-324f64, "5e-324"),
+                (1e-320, "1e-320"),
+                (1e-317, "1e-317"),
+                (1e-310, "1e-310"),
+                (-1e-310, "-1e-310"),
+                (2.2250738585072009e-308, "2.225073858507201e-308"),
+            ] {
+                let output = js_json_stringify(value, TYPE_UNKNOWN);
+                assert_eq!(
+                    str_from_header(output).unwrap(),
+                    expected,
+                    "JSON.stringify({value:e})"
+                );
+            }
+        }
+    }
+
+    /// Subnormals nested where the object/array field decoders read them —
+    /// the `is_raw_pointer` call sites in `stringify.rs` /
+    /// `stringify_shape_template.rs`, not just the top-level dispatch.
+    #[test]
+    fn stringify_subnormal_object_and_array_members() {
+        let input = br#"{"n":1e-310,"m":5e-324,"a":[1e-317,1.5],"s":"hi"}"#;
+        let text = js_string_from_bytes(input.as_ptr(), input.len() as u32);
+        let value = unsafe { js_json_parse(text) };
+        let output = unsafe { js_json_stringify(f64::from_bits(value.bits()), TYPE_UNKNOWN) };
+        assert_eq!(
+            unsafe { str_from_header(output) }.unwrap(),
+            r#"{"n":1e-310,"m":5e-324,"a":[1e-317,1.5],"s":"hi"}"#
+        );
+    }
+
+    /// The branch `is_raw_pointer` exists for: an object whose heap pointer
+    /// travels UNTAGGED in an f64 slot (#3576 module-level object/array
+    /// variables). Rejecting subnormals must not reject these — the fix keys
+    /// on GC allocation membership, so a real allocation still resolves.
+    #[test]
+    fn stringify_untagged_raw_object_pointer_still_serializes() {
+        let input = br#"{"a":1,"b":"two"}"#;
+        let text = js_string_from_bytes(input.as_ptr(), input.len() as u32);
+        let value = unsafe { js_json_parse(text) };
+        let addr = value.bits() & POINTER_MASK;
+        // No tag at all — exactly the shape a module-slot object variable has.
+        assert!(
+            unsafe { is_raw_pointer(addr) },
+            "0x{addr:x} must be admitted"
+        );
+        let output = unsafe { js_json_stringify(f64::from_bits(addr), TYPE_UNKNOWN) };
+        assert_eq!(
+            unsafe { str_from_header(output) }.unwrap(),
+            r#"{"a":1,"b":"two"}"#
+        );
+    }
+
     #[test]
     fn stringify_set_serializes_as_empty_object() {
         // Regression: a Set/Map header is NOT an ObjectHeader, so the old

crates/perry-runtime/src/json/stringify_api.rs

@@ -28,9 +28,11 @@ pub(crate) unsafe fn redirect_lazy_to_materialized(value: f64) -> f64 {
     };
     // A synthetic handle-band id (fetch/Blob/socket/… < HANDLE_BAND_MAX) is not
     // a real heap object; `gc_header = ptr - 8` would deref unmapped memory →
-    // SIGSEGV on e.g. `JSON.stringify(new Blob())` (#6240/#6241). It's never a
-    // LazyArrayHeader, so bail before the header read.
-    if ptr.is_null() || crate::value::addr_class::is_handle_band(ptr as usize) {
+    // SIGSEGV on e.g. `JSON.stringify(new Blob())` (#6240/#6241). Nor is an
+    // above-the-band payload automatically real — require a GC-tracked
+    // allocation (dereference-free page-map/registry lookups) before the
+    // header read, the same rule `is_object_pointer` uses.
+    if ptr.is_null() || !stringify::ptr_is_tracked_heap_object(ptr) {
         return value;
     }
     let gc_header = ptr.sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader;
@@ -68,21 +70,27 @@ pub(crate) unsafe fn try_stringify_lazy_array(value: f64) -> Option<*mut StringH
     let maybe_ptr = if top16 == 0x7FFD {
         // POINTER_TAG NaN-box: lower 48 bits are the user pointer.
         (bits & 0x0000_FFFF_FFFF_FFFF) as *const u8
-    } else if top16 == 0 {
-        // Raw heap pointer (no NaN-box tag). User-space addresses on
-        // 64-bit systems fit in the lower 48 bits, so a real raw
-        // pointer has top16 == 0. The previous `top16 < 0x7FF8` check
-        // also accepted regular f64 numbers (e.g. 42.0 has top16
-        // 0x4045) and `gc_header = bits - 8` then dereferenced random
-        // memory, segfaulting `JSON.stringify(42)` at
-        // `0x4044_FFFF_FFFF_FFF8`.
+    } else if is_raw_pointer(bits) {
+        // Raw heap pointer (no NaN-box tag). `top16 == 0` alone is NOT enough:
+        // it is a superset of the positive-subnormal doubles, so it classified
+        // every denormal `Number` as a pointer and dereferenced it —
+        // `JSON.stringify(1e-317)` (bits `0x1ee257`) SIGSEGV'd at `0x1ee24f`,
+        // reachable from untrusted input via
+        // `JSON.stringify(JSON.parse(text))`. It was already the second
+        // narrowing of this test (`top16 < 0x7FF8` before it, which crashed
+        // `JSON.stringify(42)`); a third narrowing would not have helped
+        // either, because a raw pointer and a denormal share bit patterns by
+        // construction. `is_raw_pointer` decides by GC allocation membership
+        // instead — see its doc in `json/mod.rs`.
         bits as *const u8
     } else {
         return None;
     };
     // Handle-band synthetic ids are never lazy arrays and must not be deref'd
-    // one word below the id (#6240/#6241).
-    if maybe_ptr.is_null() || crate::value::addr_class::is_handle_band(maybe_ptr as usize) {
+    // one word below the id (#6240/#6241). Above the band is not proof either:
+    // require a GC-tracked allocation before the header read. (Redundant for
+    // the `is_raw_pointer` arm, load-bearing for the `POINTER_TAG` one.)
+    if maybe_ptr.is_null() || !stringify::ptr_is_tracked_heap_object(maybe_ptr) {
         return None;
     }
     let gc_header = maybe_ptr.sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader;