Skip to content

Commit d1d5da2

Browse files
vm: harden module type checks
Check if the value returned from user linker function is a null-ish value. `validateInternalField` should be preferred when checking `this` argument to guard against null-ish `this`. Co-authored-by: Mike Ralphson <mike.ralphson@gmail.com> PR-URL: #52162 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
1 parent 0b67673 commit d1d5da2

File tree

5 files changed

+57
-54
lines changed

5 files changed

+57
-54
lines changed

lib/internal/vm/module.js

+23-39
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const {
99
ObjectDefineProperty,
1010
ObjectFreeze,
1111
ObjectGetPrototypeOf,
12+
ObjectPrototypeHasOwnProperty,
1213
ObjectSetPrototypeOf,
1314
ReflectApply,
1415
SafePromiseAllReturnVoid,
@@ -44,6 +45,7 @@ const {
4445
validateObject,
4546
validateUint32,
4647
validateString,
48+
validateInternalField,
4749
} = require('internal/validators');
4850

4951
const binding = internalBinding('module_wrap');
@@ -76,6 +78,13 @@ const kLink = Symbol('kLink');
7678

7779
const { isContext } = require('internal/vm');
7880

81+
function isModule(object) {
82+
if (typeof object !== 'object' || object === null || !ObjectPrototypeHasOwnProperty(object, kWrap)) {
83+
return false;
84+
}
85+
return true;
86+
}
87+
7988
class Module {
8089
constructor(options) {
8190
emitExperimentalWarning('VM Modules');
@@ -149,50 +158,38 @@ class Module {
149158
}
150159

151160
get identifier() {
152-
if (this[kWrap] === undefined) {
153-
throw new ERR_VM_MODULE_NOT_MODULE();
154-
}
161+
validateInternalField(this, kWrap, 'Module');
155162
return this[kWrap].url;
156163
}
157164

158165
get context() {
159-
if (this[kWrap] === undefined) {
160-
throw new ERR_VM_MODULE_NOT_MODULE();
161-
}
166+
validateInternalField(this, kWrap, 'Module');
162167
return this[kContext];
163168
}
164169

165170
get namespace() {
166-
if (this[kWrap] === undefined) {
167-
throw new ERR_VM_MODULE_NOT_MODULE();
168-
}
171+
validateInternalField(this, kWrap, 'Module');
169172
if (this[kWrap].getStatus() < kInstantiated) {
170173
throw new ERR_VM_MODULE_STATUS('must not be unlinked or linking');
171174
}
172175
return this[kWrap].getNamespace();
173176
}
174177

175178
get status() {
176-
if (this[kWrap] === undefined) {
177-
throw new ERR_VM_MODULE_NOT_MODULE();
178-
}
179+
validateInternalField(this, kWrap, 'Module');
179180
return STATUS_MAP[this[kWrap].getStatus()];
180181
}
181182

182183
get error() {
183-
if (this[kWrap] === undefined) {
184-
throw new ERR_VM_MODULE_NOT_MODULE();
185-
}
184+
validateInternalField(this, kWrap, 'Module');
186185
if (this[kWrap].getStatus() !== kErrored) {
187186
throw new ERR_VM_MODULE_STATUS('must be errored');
188187
}
189188
return this[kWrap].getError();
190189
}
191190

192191
async link(linker) {
193-
if (this[kWrap] === undefined) {
194-
throw new ERR_VM_MODULE_NOT_MODULE();
195-
}
192+
validateInternalField(this, kWrap, 'Module');
196193
validateFunction(linker, 'linker');
197194
if (this.status === 'linked') {
198195
throw new ERR_VM_MODULE_ALREADY_LINKED();
@@ -205,10 +202,7 @@ class Module {
205202
}
206203

207204
async evaluate(options = kEmptyObject) {
208-
if (this[kWrap] === undefined) {
209-
throw new ERR_VM_MODULE_NOT_MODULE();
210-
}
211-
205+
validateInternalField(this, kWrap, 'Module');
212206
validateObject(options, 'options');
213207

214208
let timeout = options.timeout;
@@ -231,9 +225,7 @@ class Module {
231225
}
232226

233227
[customInspectSymbol](depth, options) {
234-
if (this[kWrap] === undefined) {
235-
throw new ERR_VM_MODULE_NOT_MODULE();
236-
}
228+
validateInternalField(this, kWrap, 'Module');
237229
if (typeof depth === 'number' && depth < 0)
238230
return this;
239231

@@ -308,7 +300,7 @@ class SourceTextModule extends Module {
308300

309301
const promises = this[kWrap].link(async (identifier, attributes) => {
310302
const module = await linker(identifier, this, { attributes, assert: attributes });
311-
if (module[kWrap] === undefined) {
303+
if (!isModule(module)) {
312304
throw new ERR_VM_MODULE_NOT_MODULE();
313305
}
314306
if (module.context !== this.context) {
@@ -339,17 +331,13 @@ class SourceTextModule extends Module {
339331
}
340332

341333
get dependencySpecifiers() {
342-
if (this[kWrap] === undefined) {
343-
throw new ERR_VM_MODULE_NOT_MODULE();
344-
}
334+
validateInternalField(this, kDependencySpecifiers, 'SourceTextModule');
345335
this[kDependencySpecifiers] ??= ObjectFreeze(this[kWrap].getStaticDependencySpecifiers());
346336
return this[kDependencySpecifiers];
347337
}
348338

349339
get status() {
350-
if (this[kWrap] === undefined) {
351-
throw new ERR_VM_MODULE_NOT_MODULE();
352-
}
340+
validateInternalField(this, kDependencySpecifiers, 'SourceTextModule');
353341
if (this.#error !== kNoError) {
354342
return 'errored';
355343
}
@@ -360,9 +348,7 @@ class SourceTextModule extends Module {
360348
}
361349

362350
get error() {
363-
if (this[kWrap] === undefined) {
364-
throw new ERR_VM_MODULE_NOT_MODULE();
365-
}
351+
validateInternalField(this, kDependencySpecifiers, 'SourceTextModule');
366352
if (this.#error !== kNoError) {
367353
return this.#error;
368354
}
@@ -415,9 +401,7 @@ class SyntheticModule extends Module {
415401
}
416402

417403
setExport(name, value) {
418-
if (this[kWrap] === undefined) {
419-
throw new ERR_VM_MODULE_NOT_MODULE();
420-
}
404+
validateInternalField(this, kWrap, 'SyntheticModule');
421405
validateString(name, 'name');
422406
if (this[kWrap].getStatus() < kInstantiated) {
423407
throw new ERR_VM_MODULE_STATUS('must be linked');
@@ -432,7 +416,7 @@ function importModuleDynamicallyWrap(importModuleDynamically) {
432416
if (isModuleNamespaceObject(m)) {
433417
return m;
434418
}
435-
if (!m || m[kWrap] === undefined) {
419+
if (!isModule(m)) {
436420
throw new ERR_VM_MODULE_NOT_MODULE();
437421
}
438422
if (m.status === 'errored') {

test/parallel/test-vm-module-basic.js

+9-7
Original file line numberDiff line numberDiff line change
@@ -84,13 +84,15 @@ const util = require('util');
8484

8585
assert.strictEqual(util.inspect(m, { depth: -1 }), '[SourceTextModule]');
8686

87-
assert.throws(
88-
() => m[util.inspect.custom].call({ __proto__: null }),
89-
{
90-
code: 'ERR_VM_MODULE_NOT_MODULE',
91-
message: 'Provided module is not an instance of Module'
92-
},
93-
);
87+
for (const value of [null, { __proto__: null }, SourceTextModule.prototype]) {
88+
assert.throws(
89+
() => m[util.inspect.custom].call(value),
90+
{
91+
code: 'ERR_INVALID_ARG_TYPE',
92+
message: /The "this" argument must be an instance of Module/,
93+
},
94+
);
95+
}
9496
}
9597

9698
{

test/parallel/test-vm-module-errors.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -216,8 +216,8 @@ async function checkInvalidOptionForEvaluate() {
216216
await assert.rejects(async () => {
217217
await Module.prototype[method]();
218218
}, {
219-
code: 'ERR_VM_MODULE_NOT_MODULE',
220-
message: /Provided module is not an instance of Module/
219+
code: 'ERR_INVALID_ARG_TYPE',
220+
message: /The "this" argument must be an instance of Module/
221221
});
222222
});
223223
}
@@ -241,8 +241,8 @@ function checkInvalidCachedData() {
241241

242242
function checkGettersErrors() {
243243
const expectedError = {
244-
code: 'ERR_VM_MODULE_NOT_MODULE',
245-
message: /Provided module is not an instance of Module/
244+
code: 'ERR_INVALID_ARG_TYPE',
245+
message: /The "this" argument must be an instance of (?:Module|SourceTextModule)/,
246246
};
247247
const getters = ['identifier', 'context', 'namespace', 'status', 'error'];
248248
getters.forEach((getter) => {

test/parallel/test-vm-module-link.js

+17
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,22 @@ async function simple() {
2828
delete globalThis.fiveResult;
2929
}
3030

31+
async function invalidLinkValue() {
32+
const invalidValues = [
33+
undefined,
34+
null,
35+
{},
36+
SourceTextModule.prototype,
37+
];
38+
39+
for (const value of invalidValues) {
40+
const module = new SourceTextModule('import "foo"');
41+
await assert.rejects(module.link(() => value), {
42+
code: 'ERR_VM_MODULE_NOT_MODULE',
43+
});
44+
}
45+
}
46+
3147
async function depth() {
3248
const foo = new SourceTextModule('export default 5');
3349
await foo.link(common.mustNotCall());
@@ -143,6 +159,7 @@ const finished = common.mustCall();
143159

144160
(async function main() {
145161
await simple();
162+
await invalidLinkValue();
146163
await depth();
147164
await circular();
148165
await circular2();

test/parallel/test-vm-module-synthetic.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,12 @@ const assert = require('assert');
6666
});
6767
}
6868

69-
{
69+
for (const value of [null, {}, SyntheticModule.prototype]) {
7070
assert.throws(() => {
71-
SyntheticModule.prototype.setExport.call({}, 'foo');
71+
SyntheticModule.prototype.setExport.call(value, 'foo');
7272
}, {
73-
code: 'ERR_VM_MODULE_NOT_MODULE',
74-
message: /Provided module is not an instance of Module/
73+
code: 'ERR_INVALID_ARG_TYPE',
74+
message: /The "this" argument must be an instance of SyntheticModule/
7575
});
7676
}
7777

0 commit comments

Comments
 (0)