diff --git a/src/jsc/bindings/NodeVM.cpp b/src/jsc/bindings/NodeVM.cpp index fd006bcc3ad1..ee1a27618fa4 100644 --- a/src/jsc/bindings/NodeVM.cpp +++ b/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); diff --git a/test/js/node/vm/vm.test.ts b/test/js/node/vm/vm.test.ts index 296971c440be..3d0e2a40ac5d 100644 --- a/test/js/node/vm/vm.test.ts +++ b/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);