Skip to content

Commit 9e1c229

Browse files
legendecasMikeRalphson
authored and
targos
committed
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 Backport-PR-URL: #53109 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
1 parent 92f3447 commit 9e1c229

File tree

5 files changed

+58
-57
lines changed

5 files changed

+58
-57
lines changed

‎lib/internal/vm/module.js‎

Lines changed: 24 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const {
88
ArrayPrototypeSome,
99
ObjectDefineProperty,
1010
ObjectGetPrototypeOf,
11+
ObjectPrototypeHasOwnProperty,
1112
ObjectSetPrototypeOf,
1213
ReflectApply,
1314
SafePromiseAllReturnVoid,
@@ -43,6 +44,7 @@ const {
4344
validateObject,
4445
validateUint32,
4546
validateString,
47+
validateInternalField,
4648
} = require('internal/validators');
4749

4850
const binding = internalBinding('module_wrap');
@@ -75,6 +77,13 @@ const kLink = Symbol('kLink');
7577

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

80+
function isModule(object) {
81+
if (typeof object !== 'object' || object === null || !ObjectPrototypeHasOwnProperty(object, kWrap)) {
82+
return false;
83+
}
84+
return true;
85+
}
86+
7887
class Module {
7988
constructor(options) {
8089
emitExperimentalWarning('VM Modules');
@@ -147,50 +156,38 @@ class Module {
147156
}
148157

149158
get identifier() {
150-
if (this[kWrap] === undefined) {
151-
throw new ERR_VM_MODULE_NOT_MODULE();
152-
}
159+
validateInternalField(this, kWrap, 'Module');
153160
return this[kWrap].url;
154161
}
155162

156163
get context() {
157-
if (this[kWrap] === undefined) {
158-
throw new ERR_VM_MODULE_NOT_MODULE();
159-
}
164+
validateInternalField(this, kWrap, 'Module');
160165
return this[kContext];
161166
}
162167

163168
get namespace() {
164-
if (this[kWrap] === undefined) {
165-
throw new ERR_VM_MODULE_NOT_MODULE();
166-
}
169+
validateInternalField(this, kWrap, 'Module');
167170
if (this[kWrap].getStatus() < kInstantiated) {
168171
throw new ERR_VM_MODULE_STATUS('must not be unlinked or linking');
169172
}
170173
return this[kWrap].getNamespace();
171174
}
172175

173176
get status() {
174-
if (this[kWrap] === undefined) {
175-
throw new ERR_VM_MODULE_NOT_MODULE();
176-
}
177+
validateInternalField(this, kWrap, 'Module');
177178
return STATUS_MAP[this[kWrap].getStatus()];
178179
}
179180

180181
get error() {
181-
if (this[kWrap] === undefined) {
182-
throw new ERR_VM_MODULE_NOT_MODULE();
183-
}
182+
validateInternalField(this, kWrap, 'Module');
184183
if (this[kWrap].getStatus() !== kErrored) {
185184
throw new ERR_VM_MODULE_STATUS('must be errored');
186185
}
187186
return this[kWrap].getError();
188187
}
189188

190189
async link(linker) {
191-
if (this[kWrap] === undefined) {
192-
throw new ERR_VM_MODULE_NOT_MODULE();
193-
}
190+
validateInternalField(this, kWrap, 'Module');
194191
validateFunction(linker, 'linker');
195192
if (this.status === 'linked') {
196193
throw new ERR_VM_MODULE_ALREADY_LINKED();
@@ -203,10 +200,7 @@ class Module {
203200
}
204201

205202
async evaluate(options = kEmptyObject) {
206-
if (this[kWrap] === undefined) {
207-
throw new ERR_VM_MODULE_NOT_MODULE();
208-
}
209-
203+
validateInternalField(this, kWrap, 'Module');
210204
validateObject(options, 'options');
211205

212206
let timeout = options.timeout;
@@ -229,9 +223,7 @@ class Module {
229223
}
230224

231225
[customInspectSymbol](depth, options) {
232-
if (this[kWrap] === undefined) {
233-
throw new ERR_VM_MODULE_NOT_MODULE();
234-
}
226+
validateInternalField(this, kWrap, 'Module');
235227
if (typeof depth === 'number' && depth < 0)
236228
return this;
237229

@@ -306,7 +298,7 @@ class SourceTextModule extends Module {
306298

307299
const promises = this[kWrap].link(async (identifier, attributes) => {
308300
const module = await linker(identifier, this, { attributes, assert: attributes });
309-
if (module[kWrap] === undefined) {
301+
if (!isModule(module)) {
310302
throw new ERR_VM_MODULE_NOT_MODULE();
311303
}
312304
if (module.context !== this.context) {
@@ -337,19 +329,13 @@ class SourceTextModule extends Module {
337329
}
338330

339331
get dependencySpecifiers() {
340-
if (this[kWrap] === undefined) {
341-
throw new ERR_VM_MODULE_NOT_MODULE();
342-
}
343-
if (this[kDependencySpecifiers] === undefined) {
344-
this[kDependencySpecifiers] = this[kWrap].getStaticDependencySpecifiers();
345-
}
332+
validateInternalField(this, kDependencySpecifiers, 'SourceTextModule');
333+
this[kDependencySpecifiers] ??= this[kWrap].getStaticDependencySpecifiers();
346334
return this[kDependencySpecifiers];
347335
}
348336

349337
get status() {
350-
if (this[kWrap] === undefined) {
351-
throw new ERR_VM_MODULE_NOT_MODULE();
352-
}
338+
validateInternalField(this, kDependencySpecifiers, 'SourceTextModule');
353339
if (this.#error !== kNoError) {
354340
return 'errored';
355341
}
@@ -360,9 +346,7 @@ class SourceTextModule extends Module {
360346
}
361347

362348
get error() {
363-
if (this[kWrap] === undefined) {
364-
throw new ERR_VM_MODULE_NOT_MODULE();
365-
}
349+
validateInternalField(this, kDependencySpecifiers, 'SourceTextModule');
366350
if (this.#error !== kNoError) {
367351
return this.#error;
368352
}
@@ -415,9 +399,7 @@ class SyntheticModule extends Module {
415399
}
416400

417401
setExport(name, value) {
418-
if (this[kWrap] === undefined) {
419-
throw new ERR_VM_MODULE_NOT_MODULE();
420-
}
402+
validateInternalField(this, kWrap, 'SyntheticModule');
421403
validateString(name, 'name');
422404
if (this[kWrap].getStatus() < kInstantiated) {
423405
throw new ERR_VM_MODULE_STATUS('must be linked');
@@ -432,7 +414,7 @@ function importModuleDynamicallyWrap(importModuleDynamically) {
432414
if (isModuleNamespaceObject(m)) {
433415
return m;
434416
}
435-
if (!m || m[kWrap] === undefined) {
417+
if (!isModule(m)) {
436418
throw new ERR_VM_MODULE_NOT_MODULE();
437419
}
438420
if (m.status === 'errored') {

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

Lines changed: 9 additions & 7 deletions
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‎

Lines changed: 4 additions & 4 deletions
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‎

Lines changed: 17 additions & 0 deletions
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‎

Lines changed: 4 additions & 4 deletions
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)