node:vm: block sloppy [[Set]] on read-only contextified globals

src/jsc/bindings/NodeVM.cpp

@@ -1086,6 +1086,19 @@ bool NodeVMGlobalObject::put(JSCell* cell, JSGlobalObject* globalObject, Propert
     }
     bool isDeclaredOnGlobalObject = slot.type() == JSC::PutPropertySlot::NewProperty;
     auto scope = DECLARE_THROW_SCOPE(vm);
+
+    // If the property already exists on the global object as read-only, do not
+    // forward the store to the sandbox. Matches Node's contextify
+    // PropertySetterCallback, which intercepts and returns early on ReadOnly.
+    {
+        PropertySlot existingSlot(thisObject, PropertySlot::InternalMethodType::GetOwnProperty, nullptr);
+        bool existsOnGlobal = thisObject->JSC::JSGlobalObject::getOwnPropertySlot(thisObject, globalObject, propertyName, existingSlot);
+        RETURN_IF_EXCEPTION(scope, false);
+        if (existsOnGlobal && (existingSlot.attributes() & PropertyAttribute::ReadOnly) != 0) {
+            RELEASE_AND_RETURN(scope, Base::put(cell, globalObject, propertyName, value, slot));
+        }
+    }
+
     PropertySlot getter(sandbox, PropertySlot::InternalMethodType::Get, nullptr);
     bool isDeclaredOnSandbox = sandbox->getPropertySlot(globalObject, propertyName, getter);
     RETURN_IF_EXCEPTION(scope, false);

test/js/node/vm/vm.test.ts

@@ -420,6 +420,33 @@ function testRunInContext({ fn, isIsolated, isNew }: TestRunInContextArg) {
       expect(context.baz).toEqual([undefined, "b", "c"]);
       expect(result).toBe(true);
     });
+    test("sloppy assignment to read-only globals is a no-op", () => {
+      const context = createContext({});
+      for (const name of ["NaN", "undefined", "Infinity"]) {
+        const result = fn(
+          `(function () {` +
+            `globalThis[${JSON.stringify(name)}] = 123;` +
+            `var d = Object.getOwnPropertyDescriptor(globalThis, ${JSON.stringify(name)});` +
+            `return { value: String(globalThis[${JSON.stringify(name)}]), writable: d.writable, configurable: d.configurable };` +
+            `})()`,
+          context,
+        );
+        expect({ name, ...result }).toEqual({ name, value: name, writable: false, configurable: false });
+        expect(Object.prototype.hasOwnProperty.call(context, name)).toBe(false);
+      }
+      expect(Object.getOwnPropertyNames(context)).toEqual([]);
+    });
+    test("strict assignment to read-only globals throws and does not mutate the sandbox", () => {
+      const context = createContext({});
+      const result = fn(
+        "(function(){'use strict'; try { globalThis.NaN = 5; return 'set'; } catch (e) { return e.constructor.name; } })()",
+        context,
+      );
+      expect(result).toBe("TypeError");
+      expect(fn("String(globalThis.NaN)", context)).toBe("NaN");
+      expect(Object.prototype.hasOwnProperty.call(context, "NaN")).toBe(false);
+      expect(Object.getOwnPropertyNames(context)).toEqual([]);
+    });
     test("cannot access `process`", () => {
       const context = createContext({});
       const result = fn("typeof process;", context);