fix(runtime): reject the handle band before reading a symbol receiver's GcHeader

changelog.d/11327-fetch-symbol-property-handle-band-segv.md

@@ -0,0 +1,18 @@
+Fixed `Reflect.set`/`obj[sym] = v` segfaulting on a native Web Fetch handle
+(`Request`/`Response`/`Headers`) — e.g. `Reflect.set(new Request(url), Symbol.for("x"), v)`,
+the pattern Astro's `@astrojs/node` adapter uses to tag every request with its
+client address. `set_symbol_property`'s frozen/non-extensible guard (and its
+`symbol_property_is_non_writable` mirror) gated the `GcHeader` dereference on a
+hand-typed `obj_key >= 0x10000` floor — an order of magnitude below
+`HANDLE_BAND_MAX` (0x100000) — paired with `is_valid_obj_ptr`, whose doc
+explicitly warns it does *not* reject the handle band by itself. A Web Fetch
+handle id (`[0x40000, 0xE0000)`) sailed through both checks and
+`(obj_key - GC_HEADER_SIZE) as *const GcHeader` read unmapped memory. Both call
+sites now use `try_read_gc_header`, which classifies the whole handle band
+(and alignment) before any dereference and returns `None` for handle
+receivers — the same guard already used elsewhere in the runtime for this
+exact class of bug (#1843/#4004/#4800).
+
+Test: `test-parity/node-suite/fetch/symbol-property-set.ts` (Reflect.set/get,
+direct `obj[sym]`, overwrite, `Object.getOwnPropertySymbols`, delete, and an
+untyped-parameter write, on `Request`/`Response`/`Headers`).

crates/perry-runtime/src/symbol/properties.rs

@@ -422,24 +422,27 @@ unsafe fn set_symbol_property(obj_f64: f64, sym_f64: f64, value_f64: f64) -> f64
     // like string-keyed ones: an existing prop is non-writable when frozen
     // (or its per-symbol attrs say so), a new prop is forbidden when
     // non-extensible. Only heap receivers carry the GC flag word.
-    if !native_async_resource
-        && (obj_f64.to_bits() >> 48) == 0x7FFD
-        && obj_key >= 0x10000
-        && crate::object::is_valid_obj_ptr(obj_key as *const u8)
-    {
-        let gc = (obj_key - crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader;
-        let flags = (*gc)._reserved;
-        if has_own_data {
-            if flags & crate::gc::OBJ_FLAG_FROZEN != 0 {
-                return value_f64;
-            }
-            if let Some(attrs) = get_symbol_property_attrs(obj_key, sym_key) {
-                if !attrs.writable() {
+    //
+    // `try_read_gc_header` rejects the whole handle band (and misaligned or
+    // slab addresses) before dereferencing. A hand-typed `>= 0x10000` floor
+    // plus `is_valid_obj_ptr` is not enough: Web Fetch handles
+    // (Request/Response/Headers) live above that floor, and
+    // `Reflect.set(new Request(url), sym, v)` read an unmapped GcHeader.
+    if !native_async_resource && (obj_f64.to_bits() >> 48) == 0x7FFD {
+        if let Some(gc) = crate::value::addr_class::try_read_gc_header(obj_key) {
+            let flags = gc._reserved;
+            if has_own_data {
+                if flags & crate::gc::OBJ_FLAG_FROZEN != 0 {
                     return value_f64;
                 }
+                if let Some(attrs) = get_symbol_property_attrs(obj_key, sym_key) {
+                    if !attrs.writable() {
+                        return value_f64;
+                    }
+                }
+            } else if flags & crate::gc::OBJ_FLAG_NO_EXTEND != 0 {
+                return value_f64;
             }
-        } else if flags & crate::gc::OBJ_FLAG_NO_EXTEND != 0 {
-            return value_f64;
         }
     }
     if !has_own_data {
@@ -498,15 +501,14 @@ pub(crate) fn symbol_property_is_non_writable(obj_f64: f64, sym_f64: f64) -> boo
     if obj_key == 0 || sym_key == 0 {
         return false;
     }
-    // Only heap receivers carry the GC integrity flag word.
-    if (obj_f64.to_bits() >> 48) == 0x7FFD
-        && obj_key >= 0x10000
-        && crate::object::is_valid_obj_ptr(obj_key as *const u8)
-    {
-        let gc = (obj_key - crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader;
-        let flags = unsafe { (*gc)._reserved };
-        if flags & crate::gc::OBJ_FLAG_FROZEN != 0 {
-            return true;
+    // Only heap receivers carry the GC integrity flag word; see
+    // `set_symbol_property` for why this must reject the whole handle band.
+    if (obj_f64.to_bits() >> 48) == 0x7FFD {
+        if let Some(gc) = unsafe { crate::value::addr_class::try_read_gc_header(obj_key) } {
+            let flags = gc._reserved;
+            if flags & crate::gc::OBJ_FLAG_FROZEN != 0 {
+                return true;
+            }
         }
     }
     get_symbol_property_attrs(obj_key, sym_key).is_some_and(|attrs| !attrs.writable())

test-parity/node-suite/fetch/symbol-property-set.ts

@@ -0,0 +1,47 @@
+// Symbol-keyed own properties on native fetch objects (Request / Response /
+// Headers). Frameworks tag these objects with private symbols — e.g. Astro's
+// node adapter does `Reflect.set(request, clientAddressSymbol, ip)` on every
+// request — so the write must behave like on any ordinary object.
+
+const registered = Symbol.for("parity.fetch.registered");
+const local = Symbol("parity.fetch.local");
+
+function exercise(label: string, obj: any) {
+  console.log(label, "Reflect.set registered:", Reflect.set(obj, registered, "r1"));
+  console.log(label, "Reflect.get registered:", Reflect.get(obj, registered));
+
+  obj[local] = "l1";
+  console.log(label, "direct get local:", obj[local]);
+
+  obj[registered] = "r2";
+  console.log(label, "overwrite registered:", obj[registered]);
+
+  const syms = Object.getOwnPropertySymbols(obj);
+  console.log(label, "own symbols include registered:", syms.includes(registered));
+  console.log(label, "own symbols include local:", syms.includes(local));
+
+  console.log(label, "delete local:", delete obj[local], obj[local]);
+}
+
+function viaUntypedParam(label: string, target: any) {
+  // Reach the write through an untyped parameter, the shape framework code uses.
+  console.log(label, "param Reflect.set:", Reflect.set(target, local, "p1"));
+  console.log(label, "param get:", target[local]);
+}
+
+const req = new Request("http://localhost/path", { method: "POST" });
+const res = new Response("body", { status: 201 });
+const headers = new Headers({ "x-a": "1" });
+
+exercise("request", req);
+exercise("response", res);
+exercise("headers", headers);
+
+viaUntypedParam("request", new Request("http://localhost/"));
+viaUntypedParam("response", new Response(null));
+viaUntypedParam("headers", new Headers());
+
+// The native state must survive the symbol writes.
+console.log("request still works:", req.method, req.url);
+console.log("response still works:", res.status);
+console.log("headers still works:", headers.get("x-a"));

test-parity/node_suite_baseline.json

@@ -68,8 +68,8 @@
       "total": 69
     },
     "fetch": {
-      "pass": 12,
-      "total": 12
+      "pass": 13,
+      "total": 13
     },
     "fs": {
       "pass": 168,