diff --git a/changelog.d/7447-json-subnormal-deref.md b/changelog.d/7447-json-subnormal-deref.md new file mode 100644 index 0000000000..b465a1d51c --- /dev/null +++ b/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) diff --git a/crates/perry-runtime/src/json/mod.rs b/crates/perry-runtime/src/json/mod.rs index c8cc57a867..55c2f4e89b 100644 --- a/crates/perry-runtime/src/json/mod.rs +++ b/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 diff --git a/crates/perry-runtime/src/json/stringify_api.rs b/crates/perry-runtime/src/json/stringify_api.rs index 7c8bd2fe5a..74a3fcfeaf 100644 --- a/crates/perry-runtime/src/json/stringify_api.rs +++ b/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;