Skip to content

Commit 86584f7

Browse files
legendecasRafaelGSS
authored andcommitted
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 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]>
1 parent b79c6e2 commit 86584f7

File tree

3 files changed

+48
-3
lines changed

3 files changed

+48
-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: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -350,9 +350,25 @@ 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 has no JavaScript function representation and is not
355+
// invoked in the creation context.
356+
// When this getter is invoked in a vm context, the `Realm::GetCurrent(info)`
357+
// returns a nullptr and. Retrieve the creation context via `this` object and
358+
// get the creation Realm.
359+
Local<Value> receiver_val = info.This();
360+
if (!receiver_val->IsObject()) {
361+
THROW_ERR_INVALID_INVOCATION(isolate);
362+
return;
363+
}
364+
Local<Object> receiver = receiver_val.As<Object>();
365+
Local<Context> context;
366+
if (!receiver->GetCreationContext().ToLocal(&context)) {
367+
THROW_ERR_INVALID_INVOCATION(isolate);
368+
return;
369+
}
370+
371+
Realm* realm = Realm::GetCurrent(context);
356372
Local<Value> arg = info.Data();
357373
Local<Value> require_result;
358374
if (!realm->builtin_module_require()
@@ -368,6 +384,7 @@ static void DefineLazyPropertiesGetter(
368384
}
369385
info.GetReturnValue().Set(ret);
370386
}
387+
371388
static void DefineLazyProperties(const FunctionCallbackInfo<Value>& args) {
372389
// target: object, id: string, keys: string[][, enumerable = true]
373390
CHECK_GE(args.Length(), 3);
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
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+
12+
const ctx = vm.createContext();
13+
const getter = vm.runInContext(`
14+
function getter(object, property) {
15+
return object[property];
16+
}
17+
getter;
18+
`, ctx);
19+
20+
// `util.parseArgs` is a lazy property.
21+
const parseArgs = getter(util, 'parseArgs');
22+
assert.strictEqual(parseArgs, util.parseArgs);
23+
24+
// `globalThis.TextEncoder` is a lazy property.
25+
const TextEncoder = getter(globalThis, 'TextEncoder');
26+
assert.strictEqual(TextEncoder, globalThis.TextEncoder);

0 commit comments

Comments
 (0)