diff --git a/changelog.d/7802-android-json-subnormal.md b/changelog.d/7802-android-json-subnormal.md new file mode 100644 index 0000000000..62569f869d --- /dev/null +++ b/changelog.d/7802-android-json-subnormal.md @@ -0,0 +1,19 @@ +**`perry-ui-android`'s JSON walk no longer dereferences positive subnormal numbers** (#7448). + +`crates/perry-ui-android/src/json.rs` carried a verbatim copy of the predicate #7447 removed from the main runtime: + +```rust +exponent == 0 && mantissa != 0 && sign == 0 +``` + +That is bit-for-bit the IEEE-754 **positive-subnormal** test, so every positive denormal `Number` was classified as an untagged heap pointer and dereferenced. In the main runtime the identical code SIGSEGV'd on `JSON.stringify(1e-317)` and returned a silent `null` for `5e-324` — both reachable from untrusted input through `JSON.stringify(JSON.parse(text))`. + +The issue asked for two decisions to be made alongside the fix, and both point the same way: **no bit test can decide this**. A raw untagged pointer and a positive subnormal occupy the same bit patterns by construction, which is why the runtime's own version burned through two failed narrowings (`top16 < 0x7FF8`, then `top16 == 0`) before landing on allocation membership. A third divergent copy is how that happened in the first place. + +So the copy is deleted rather than ported: `perry_runtime::json::ptr_is_tracked_heap_object` is now exported (it was `pub(super)`, which is what blocked #7447 from fixing this) and the android path calls it. It answers from the page map and the malloc registry, both dereference-free, so a forged or unmapped address is rejected before any field is read. + +#7447 left this unfixed because the crate could not be built or verified on the machine that made the fix. That is not true here — `cargo check -p perry-ui-android` builds on macOS arm64 in under a second, and both it and `perry-runtime` check clean with this change. + +The second question the issue raises — whether the untagged-pointer branch is reachable on android at all, given it measured *dead* in the main runtime (155,540 rejections, zero admissions across the JSON corpus) — is deliberately left open. Deleting the branch outright would be the kill-policy answer, but that needs a measurement on a device, and this change makes the branch safe either way. + +Verified: `JSON.stringify` of `1e-317`, `5e-324`, a nested object of subnormals, and a `JSON.stringify(JSON.parse(…))` round-trip all match Node v26.5.1 exactly through the runtime path; `perry-runtime`'s json unit tests are 78 passed / 0 failed and `test_gap_json` is 7/7. The only remaining matches for the old bit pattern in the tree are the two doc comments that quote it as history. diff --git a/crates/perry-runtime/src/json/mod.rs b/crates/perry-runtime/src/json/mod.rs index 59275a7c88..6f33733fc7 100644 --- a/crates/perry-runtime/src/json/mod.rs +++ b/crates/perry-runtime/src/json/mod.rs @@ -45,6 +45,15 @@ pub use parse_api::{ pub use raw_json::{js_json_is_raw_json, js_json_raw_json}; pub use replacer::{js_json_stringify_full, js_json_stringify_with_replacer}; pub use reviver::js_json_parse_with_reviver; +/// #7448: exported so the cross-host UI crates can ASK the runtime whether an +/// address is GC-tracked instead of each carrying its own bit-pattern guess. +/// `perry-ui-android`'s copy tested for a positive IEEE-754 subnormal, which +/// classifies every denormal `Number` as an untagged heap pointer — the exact +/// predicate #7447 removed here after it SIGSEGV'd on `JSON.stringify(1e-317)`. +/// No bit test can decide this: a raw pointer and a positive subnormal occupy +/// the same bit patterns by construction, so the answer has to come from +/// allocation membership, which only the runtime can answer. +pub use stringify::ptr_is_tracked_heap_object; pub use stringify_api::{ js_json_get_bool, js_json_get_number, js_json_get_string, js_json_is_valid, js_json_stringify, js_json_stringify_bool, js_json_stringify_null, js_json_stringify_number, diff --git a/crates/perry-runtime/src/json/stringify.rs b/crates/perry-runtime/src/json/stringify.rs index d1d05f89cc..6e14b560af 100644 --- a/crates/perry-runtime/src/json/stringify.rs +++ b/crates/perry-runtime/src/json/stringify.rs @@ -33,7 +33,7 @@ use super::stringify_shape_template::{ /// yet point at an UNMAPPED page — `is_valid_obj_ptr` accepts it and the /// subsequent `(*obj).keys_array` read then SIGSEGVs. Mirrors the `path.rs` / /// `current_heap_header_for_user_ptr` Unknown→malloc rule. -pub(super) unsafe fn ptr_is_tracked_heap_object(ptr: *const u8) -> bool { +pub unsafe fn ptr_is_tracked_heap_object(ptr: *const u8) -> bool { let addr = ptr as usize; if crate::value::addr_class::is_handle_band(addr) { return false; diff --git a/crates/perry-ui-android/src/json.rs b/crates/perry-ui-android/src/json.rs index 301c73909e..e251b2fa43 100644 --- a/crates/perry-ui-android/src/json.rs +++ b/crates/perry-ui-android/src/json.rs @@ -315,20 +315,33 @@ const TYPE_UNKNOWN: u32 = 0; const TYPE_OBJECT: u32 = 1; const TYPE_ARRAY: u32 = 2; -#[inline] -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 -} - +/// #7448: an UNTAGGED heap pointer that reached a type-erased JSON walk. +/// +/// This used to be a hand-rolled bit test: +/// +/// ```ignore +/// exponent == 0 && mantissa != 0 && sign == 0 +/// ``` +/// +/// which is bit-for-bit the IEEE-754 POSITIVE-SUBNORMAL predicate, so every +/// positive denormal `Number` was classified as a pointer and dereferenced. In +/// the main runtime the identical code SIGSEGV'd on `JSON.stringify(1e-317)` +/// and returned a silent `null` for `5e-324`, both reachable from untrusted +/// input through `JSON.stringify(JSON.parse(text))` (#7447). +/// +/// No bit test can fix it: a raw pointer and a positive subnormal occupy the +/// same bit patterns by construction, which is why the runtime's version went +/// through two failed narrowings (`top16 < 0x7FF8`, then `top16 == 0`) before +/// landing on allocation membership. So this asks the runtime instead of +/// keeping a third divergent copy — `ptr_is_tracked_heap_object` decides from +/// the page map and the malloc registry, both dereference-free, so a forged or +/// unmapped address is rejected before any field is read. #[inline] unsafe fn extract_pointer(bits: u64) -> Option<*const u8> { let tag = bits & 0xFFFF_0000_0000_0000; if tag == POINTER_TAG { Some((bits & POINTER_MASK) as *const u8) - } else if is_raw_pointer(bits) { + } else if perry_runtime::json::ptr_is_tracked_heap_object(bits as *const u8) { Some(bits as *const u8) } else { None