Skip to content

Commit bc86e5e

Browse files
committed
src: fix crash when lazy getter is invoked in a vm context
V8 should invoke native functions in their creation context, preventing dynamic context by the caller. However, the lazy getter is not invoked in the creation context and leads to a null realm. Before V8 fixes this issue, retrieve the creation context via `this` argument.
1 parent 8fc919d commit bc86e5e

File tree

3 files changed

+52
-3
lines changed

3 files changed

+52
-3
lines changed

src/node_errors.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ void OOMErrorHandler(const char* location, const v8::OOMDetails& details);
8484
V(ERR_INVALID_ARG_TYPE, TypeError) \
8585
V(ERR_INVALID_FILE_URL_HOST, TypeError) \
8686
V(ERR_INVALID_FILE_URL_PATH, TypeError) \
87+
V(ERR_INVALID_INVOCATION, TypeError) \
8788
V(ERR_INVALID_PACKAGE_CONFIG, Error) \
8889
V(ERR_INVALID_OBJECT_DEFINE_PROPERTY, TypeError) \
8990
V(ERR_INVALID_MODULE, Error) \
@@ -201,6 +202,7 @@ ERRORS_WITH_CODE(V)
201202
"Context not associated with Node.js environment") \
202203
V(ERR_ILLEGAL_CONSTRUCTOR, "Illegal constructor") \
203204
V(ERR_INVALID_ADDRESS, "Invalid socket address") \
205+
V(ERR_INVALID_INVOCATION, "Invalid invocation") \
204206
V(ERR_INVALID_MODULE, "No such module") \
205207
V(ERR_INVALID_STATE, "Invalid state") \
206208
V(ERR_INVALID_THIS, "Value of \"this\" is the wrong type") \

src/node_util.cc

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -350,9 +350,24 @@ static void IsInsideNodeModules(const FunctionCallbackInfo<Value>& args) {
350350

351351
static void DefineLazyPropertiesGetter(
352352
Local<v8::Name> name, const v8::PropertyCallbackInfo<Value>& info) {
353-
Realm* realm = Realm::GetCurrent(info);
354-
Isolate* isolate = realm->isolate();
355-
auto context = isolate->GetCurrentContext();
353+
Isolate* isolate = info.GetIsolate();
354+
// This getter is not invoked in the creation context. When this getter
355+
// is invoked in a vm context, the `Realm::GetCurrent(info)` returns a
356+
// nullptr and. Retrieve the creation context via `this` object and
357+
// get the creation Realm.
358+
Local<Value> receiver_val = info.This();
359+
if (!receiver_val->IsObject()) {
360+
THROW_ERR_INVALID_INVOCATION(isolate);
361+
return;
362+
}
363+
Local<Object> receiver = receiver_val.As<Object>();
364+
Local<Context> context;
365+
if (!receiver->GetCreationContext().ToLocal(&context)) {
366+
THROW_ERR_INVALID_INVOCATION(isolate);
367+
return;
368+
}
369+
370+
Realm* realm = Realm::GetCurrent(context);
356371
Local<Value> arg = info.Data();
357372
Local<Value> require_result;
358373
if (!realm->builtin_module_require()
@@ -368,6 +383,7 @@ static void DefineLazyPropertiesGetter(
368383
}
369384
info.GetReturnValue().Set(ret);
370385
}
386+
371387
static void DefineLazyProperties(const FunctionCallbackInfo<Value>& args) {
372388
// target: object, id: string, keys: string[][, enumerable = true]
373389
CHECK_GE(args.Length(), 3);
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
'use strict';
2+
require('../common');
3+
4+
const vm = require('node:vm');
5+
const util = require('node:util');
6+
const assert = require('node:assert');
7+
8+
// This verifies that invoking property getters defined with
9+
// `require('internal/util').defineLazyProperties` does not crash
10+
// the process.
11+
// V8 should invoke native functions in their creation context,
12+
// preventing dynamic context by the caller. However, the lazy getter is
13+
// not invoked in the creation context and leads to a null realm. Before
14+
// V8 fixes this issue, retrieve the creation context via `this`
15+
// argument.
16+
17+
const ctx = vm.createContext();
18+
const getter = vm.runInContext(`
19+
function getter(object, property) {
20+
return object[property];
21+
}
22+
getter;
23+
`, ctx);
24+
25+
// `util.parseArgs` is a lazy property.
26+
const parseArgs = getter(util, 'parseArgs');
27+
assert.strictEqual(parseArgs, util.parseArgs);
28+
29+
// `globalThis.TextEncoder` is a lazy property.
30+
const TextEncoder = getter(globalThis, 'TextEncoder');
31+
assert.strictEqual(TextEncoder, globalThis.TextEncoder);

0 commit comments

Comments
 (0)