Skip to content

Commit

Permalink
src: fix crash when lazy getter is invoked in a vm context
Browse files Browse the repository at this point in the history
V8 should invoke native functions in their creation context,
preventing dynamic context by the caller. However, the lazy getter has
no JavaScript function representation and has no creation context. It
is not invoked in the original creation context. Fix the null realm by
retrieving the creation context via `this` argument.

PR-URL: #57168
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
  • Loading branch information
legendecas authored and targos committed Feb 25, 2025
1 parent a025d7b commit ef314dc
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 3 deletions.
2 changes: 2 additions & 0 deletions src/node_errors.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ void OOMErrorHandler(const char* location, const v8::OOMDetails& details);
V(ERR_INVALID_ARG_TYPE, TypeError) \
V(ERR_INVALID_FILE_URL_HOST, TypeError) \
V(ERR_INVALID_FILE_URL_PATH, TypeError) \
V(ERR_INVALID_INVOCATION, TypeError) \
V(ERR_INVALID_PACKAGE_CONFIG, Error) \
V(ERR_INVALID_OBJECT_DEFINE_PROPERTY, TypeError) \
V(ERR_INVALID_MODULE, Error) \
Expand Down Expand Up @@ -200,6 +201,7 @@ ERRORS_WITH_CODE(V)
"Context not associated with Node.js environment") \
V(ERR_ILLEGAL_CONSTRUCTOR, "Illegal constructor") \
V(ERR_INVALID_ADDRESS, "Invalid socket address") \
V(ERR_INVALID_INVOCATION, "Invalid invocation") \
V(ERR_INVALID_MODULE, "No such module") \
V(ERR_INVALID_STATE, "Invalid state") \
V(ERR_INVALID_THIS, "Value of \"this\" is the wrong type") \
Expand Down
23 changes: 20 additions & 3 deletions src/node_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -350,9 +350,25 @@ static void IsInsideNodeModules(const FunctionCallbackInfo<Value>& args) {

static void DefineLazyPropertiesGetter(
Local<v8::Name> name, const v8::PropertyCallbackInfo<Value>& info) {
Realm* realm = Realm::GetCurrent(info);
Isolate* isolate = realm->isolate();
auto context = isolate->GetCurrentContext();
Isolate* isolate = info.GetIsolate();
// This getter has no JavaScript function representation and is not
// invoked in the creation context.
// When this getter is invoked in a vm context, the `Realm::GetCurrent(info)`
// returns a nullptr and. Retrieve the creation context via `this` object and
// get the creation Realm.
Local<Value> receiver_val = info.This();
if (!receiver_val->IsObject()) {
THROW_ERR_INVALID_INVOCATION(isolate);
return;
}
Local<Object> receiver = receiver_val.As<Object>();
Local<Context> context;
if (!receiver->GetCreationContext().ToLocal(&context)) {
THROW_ERR_INVALID_INVOCATION(isolate);
return;
}

Realm* realm = Realm::GetCurrent(context);
Local<Value> arg = info.Data();
Local<Value> require_result;
if (!realm->builtin_module_require()
Expand All @@ -368,6 +384,7 @@ static void DefineLazyPropertiesGetter(
}
info.GetReturnValue().Set(ret);
}

static void DefineLazyProperties(const FunctionCallbackInfo<Value>& args) {
// target: object, id: string, keys: string[][, enumerable = true]
CHECK_GE(args.Length(), 3);
Expand Down
26 changes: 26 additions & 0 deletions test/parallel/test-vm-util-lazy-properties.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
'use strict';
require('../common');

const vm = require('node:vm');
const util = require('node:util');
const assert = require('node:assert');

// This verifies that invoking property getters defined with
// `require('internal/util').defineLazyProperties` does not crash
// the process.

const ctx = vm.createContext();
const getter = vm.runInContext(`
function getter(object, property) {
return object[property];
}
getter;
`, ctx);

// `util.parseArgs` is a lazy property.
const parseArgs = getter(util, 'parseArgs');
assert.strictEqual(parseArgs, util.parseArgs);

// `globalThis.TextEncoder` is a lazy property.
const TextEncoder = getter(globalThis, 'TextEncoder');
assert.strictEqual(TextEncoder, globalThis.TextEncoder);

0 comments on commit ef314dc

Please sign in to comment.