Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions src/jsc/bindings/NodeVM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -749,17 +749,12 @@ NodeVMGlobalObject* getGlobalObjectFromContext(JSGlobalObject* globalObject, JSV
{
auto scope = DECLARE_THROW_SCOPE(globalObject->vm());

if (contextValue.isUndefinedOrNull()) {
if (canThrow) {
ERR::INVALID_ARG_TYPE(scope, globalObject, "context"_s, "object"_s, contextValue);
}
return nullptr;
if (canThrow) {
validateContextArg(scope, globalObject, contextValue);
RETURN_IF_EXCEPTION(scope, nullptr);
}

if (!contextValue.isObject()) {
if (canThrow) {
ERR::INVALID_ARG_TYPE(scope, globalObject, "context"_s, "object"_s, contextValue);
}
return nullptr;
}

Expand Down Expand Up @@ -808,6 +803,14 @@ JSC::EncodedJSValue INVALID_ARG_VALUE_VM_VARIATION(JSC::ThrowScope& throwScope,
return {};
}

JSC::EncodedJSValue validateContextArg(JSC::ThrowScope& scope, JSGlobalObject* globalObject, JSValue contextArg)
{
if (!contextArg.isObject() || contextArg.isCallable()) {
return ERR::INVALID_ARG_TYPE(scope, globalObject, "object"_s, "object"_s, contextArg);
}
return JSValue::encode(jsUndefined());
}

bool isContext(JSGlobalObject* globalObject, JSValue value)
{
auto* zigGlobalObject = defaultGlobalObject(globalObject);
Expand Down Expand Up @@ -1654,9 +1657,8 @@ JSC_DEFINE_HOST_FUNCTION(vmModule_createContext, (JSGlobalObject * globalObject,
bool notContextified = getContextArg(globalObject, contextArg);
RETURN_IF_EXCEPTION(scope, {});

if (!contextArg.isObject()) {
return ERR::INVALID_ARG_TYPE(scope, globalObject, "context"_s, "object"_s, contextArg);
}
validateContextArg(scope, globalObject, contextArg);
RETURN_IF_EXCEPTION(scope, {});

JSValue optionsArg = callFrame->argument(1);

Expand Down Expand Up @@ -1711,13 +1713,11 @@ JSC_DEFINE_HOST_FUNCTION(vmModule_createContext, (JSGlobalObject * globalObject,

JSC_DEFINE_HOST_FUNCTION(vmModule_isContext, (JSGlobalObject * globalObject, CallFrame* callFrame))
{
ArgList args(callFrame);
JSValue contextArg = callFrame->argument(0);
VM& vm = globalObject->vm();
auto scope = DECLARE_THROW_SCOPE(vm);
if (!contextArg || !contextArg.isObject()) {
return ERR::INVALID_ARG_TYPE(scope, globalObject, "object"_s, "object"_s, contextArg);
}
validateContextArg(scope, globalObject, contextArg);
RETURN_IF_EXCEPTION(scope, {});
return JSValue::encode(jsBoolean(isContext(globalObject, contextArg)));
}

Expand Down
4 changes: 4 additions & 0 deletions src/jsc/bindings/NodeVM.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ JSC::EncodedJSValue INVALID_ARG_VALUE_VM_VARIATION(JSC::ThrowScope& throwScope,
// For vm.compileFunction we need to return an anonymous function expression. This code is adapted from/inspired by JSC::constructFunction, which is used for function declarations.
JSC::JSFunction* constructAnonymousFunction(JSC::JSGlobalObject* globalObject, const ArgList& args, const SourceOrigin& sourceOrigin, CompileFunctionOptions&& options, JSC::SourceTaintedOrigin sourceTaintOrigin, JSC::JSScope* scope);
JSPromise* importModule(JSGlobalObject* globalObject, JSString* moduleNameValue, RefPtr<JSC::ScriptFetchParameters> parameters, const SourceOrigin& sourceOrigin);
// Node's isContext() validates its argument with validateObject(object, "object", kValidateObjectAllowArray),
// and createContext()/runInContext()/runInNewContext() validate their context argument through it:
// arrays pass; null, primitives and functions throw ERR_INVALID_ARG_TYPE.
JSC::EncodedJSValue validateContextArg(JSC::ThrowScope& scope, JSC::JSGlobalObject* globalObject, JSValue contextArg);
bool isContext(JSC::JSGlobalObject* globalObject, JSValue);
bool getContextArg(JSC::JSGlobalObject* globalObject, JSValue& contextArg);
bool isUseMainContextDefaultLoaderConstant(JSC::JSGlobalObject* globalObject, JSValue value);
Expand Down
6 changes: 2 additions & 4 deletions src/jsc/bindings/NodeVMScript.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -633,10 +633,8 @@ JSC_DEFINE_HOST_FUNCTION(scriptRunInNewContext, (JSGlobalObject * globalObject,

bool notContextified = NodeVM::getContextArg(globalObject, contextObjectValue);

if (!contextObjectValue || !contextObjectValue.isObject()) [[unlikely]] {
throwTypeError(globalObject, scope, "Context must be an object"_s);
return {};
}
NodeVM::validateContextArg(scope, globalObject, contextObjectValue);
RETURN_IF_EXCEPTION(scope, {});

JSValue contextOptionsArg = callFrame->argument(1);
NodeVMContextOptions contextOptions {};
Expand Down
101 changes: 101 additions & 0 deletions test/js/node/vm/vm.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
compileFunction,
constants,
createContext,
isContext,
runInContext,
runInNewContext,
runInThisContext,
Expand Down Expand Up @@ -1153,6 +1154,106 @@ test("Loader is not defined in vm context", () => {
expect(runInContext("typeof Loader.registry;", customContext)).toBe("undefined");
});

describe("the context argument", () => {
// Node routes every context argument through isContext() (lib/vm.js), whose
// validateObject(object, "object", kValidateObjectAllowArray) accepts arrays
// and rejects functions along with null and primitives. Expected messages
// were checked against node v26.3.0.
const script = new Script("1 + 1;");
// These contextify the argument; undefined and DONT_CONTEXTIFY mean "a fresh one".
const contextifying: Record<string, (context: unknown) => unknown> = {
"createContext()": context => runInContext("1 + 1;", createContext(context as any)),
"vm.runInNewContext()": context => runInNewContext("1 + 1;", context as any),
"Script#runInNewContext()": context => script.runInNewContext(context as any),
};
// These expect an existing context.
const requiringContext: Record<string, (context: unknown) => unknown> = {
"isContext()": context => isContext(context as any),
"vm.runInContext()": context => runInContext("1 + 1;", context as any),
"Script#runInContext()": context => script.runInContext(context as any),
};

function invalidArgType(received: string | RegExp) {
return expect.objectContaining({
name: "TypeError",
code: "ERR_INVALID_ARG_TYPE",
message:
typeof received === "string"
? `The "object" argument must be of type object. Received ${received}`
: expect.stringMatching(received),
});
}

const rejected: [description: string, context: unknown, received: string | RegExp][] = [
["a function", function foo() {}, "function foo"],
["an anonymous function", function () {}, "function "],
["an arrow function", () => {}, "function "],
["an async function", async function foo() {}, "function foo"],
["a class", class Foo {}, "function Foo"],
// How these two are named differs from Node today; only the type matters here.
[
"a bound function",
function foo() {}.bind(null),
/^The "object" argument must be of type object\. Received function /,
],
[
"a callable Proxy",
new Proxy(function foo() {}, {}),
/^The "object" argument must be of type object\. Received function /,
],
["null", null, "null"],
["a number", 1, "type number (1)"],
["a string", "sandbox", "type string ('sandbox')"],
["a boolean", true, "type boolean (true)"],
["a symbol", Symbol("sandbox"), "type symbol (Symbol(sandbox))"],
];

describe.each(Object.entries({ ...contextifying, ...requiringContext }))("%s", (_, run) => {
test.each(rejected)("rejects %s", (_, context, received) => {
expect(() => run(context)).toThrow(invalidArgType(received));
});
});

describe.each(Object.entries(contextifying))("%s", (_, run) => {
test.each([
["an array", []],
["undefined", undefined],
["vm.constants.DONT_CONTEXTIFY", constants.DONT_CONTEXTIFY],
])("still contextifies %s", (_, context) => {
expect(run(context)).toBe(2);
});
});

describe.each(Object.entries(requiringContext))("%s", (_, run) => {
test("rejects undefined", () => {
expect(() => run(undefined)).toThrow(invalidArgType("undefined"));
});
});

test("a contextified array is a context everywhere", () => {
const sandbox: unknown[] = [];
expect(isContext(sandbox)).toBe(false);
expect(createContext(sandbox)).toBe(sandbox);
expect(isContext(sandbox)).toBe(true);
expect(runInContext("1 + 1;", sandbox)).toBe(2);
expect(script.runInContext(sandbox)).toBe(2);
});

test("an object that is not a context fails the vm.Context check, not the type check", () => {
const notAContext = expect.objectContaining({
name: "TypeError",
code: "ERR_INVALID_ARG_TYPE",
message: expect.stringContaining('The "contextifiedObject" argument must be an vm.Context'),
});
expect(isContext({})).toBe(false);
expect(isContext([])).toBe(false);
expect(() => runInContext("1 + 1;", {})).toThrow(notAContext);
expect(() => runInContext("1 + 1;", [])).toThrow(notAContext);
expect(() => script.runInContext({})).toThrow(notAContext);
expect(() => script.runInContext([])).toThrow(notAContext);
});
});

test("node:vm native Module prototype methods reject non-module receivers", async () => {
// The native NodeVMModule prototype (reachable via the kNative own-symbol on a
// vm.SourceTextModule instance) must validate its receiver. Calling its methods
Expand Down