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

5 files changed

Lines changed: 58 additions & 57 deletions

File tree

‎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
constbinding=internalBinding('module_wrap');
@@ -75,6 +77,13 @@ const kLink = Symbol('kLink');
7577

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

80+
functionisModule(object){
81+
if(typeofobject!=='object'||object===null||!ObjectPrototypeHasOwnProperty(object,kWrap)){
82+
returnfalse;
83+
}
84+
returntrue;
85+
}
86+
7887
classModule{
7988
constructor(options){
8089
emitExperimentalWarning('VM Modules');
@@ -147,50 +156,38 @@ class Module {
147156
}
148157

149158
getidentifier(){
150-
if(this[kWrap]===undefined){
151-
thrownewERR_VM_MODULE_NOT_MODULE();
152-
}
159+
validateInternalField(this,kWrap,'Module');
153160
returnthis[kWrap].url;
154161
}
155162

156163
getcontext(){
157-
if(this[kWrap]===undefined){
158-
thrownewERR_VM_MODULE_NOT_MODULE();
159-
}
164+
validateInternalField(this,kWrap,'Module');
160165
returnthis[kContext];
161166
}
162167

163168
getnamespace(){
164-
if(this[kWrap]===undefined){
165-
thrownewERR_VM_MODULE_NOT_MODULE();
166-
}
169+
validateInternalField(this,kWrap,'Module');
167170
if(this[kWrap].getStatus()<kInstantiated){
168171
thrownewERR_VM_MODULE_STATUS('must not be unlinked or linking');
169172
}
170173
returnthis[kWrap].getNamespace();
171174
}
172175

173176
getstatus(){
174-
if(this[kWrap]===undefined){
175-
thrownewERR_VM_MODULE_NOT_MODULE();
176-
}
177+
validateInternalField(this,kWrap,'Module');
177178
returnSTATUS_MAP[this[kWrap].getStatus()];
178179
}
179180

180181
geterror(){
181-
if(this[kWrap]===undefined){
182-
thrownewERR_VM_MODULE_NOT_MODULE();
183-
}
182+
validateInternalField(this,kWrap,'Module');
184183
if(this[kWrap].getStatus()!==kErrored){
185184
thrownewERR_VM_MODULE_STATUS('must be errored');
186185
}
187186
returnthis[kWrap].getError();
188187
}
189188

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

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

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

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

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

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

339331
getdependencySpecifiers(){
340-
if(this[kWrap]===undefined){
341-
thrownewERR_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
returnthis[kDependencySpecifiers];
347335
}
348336

349337
getstatus(){
350-
if(this[kWrap]===undefined){
351-
thrownewERR_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
geterror(){
363-
if(this[kWrap]===undefined){
364-
thrownewERR_VM_MODULE_NOT_MODULE();
365-
}
349+
validateInternalField(this,kDependencySpecifiers,'SourceTextModule');
366350
if(this.#error !==kNoError){
367351
returnthis.#error;
368352
}
@@ -415,9 +399,7 @@ class SyntheticModule extends Module {
415399
}
416400

417401
setExport(name,value){
418-
if(this[kWrap]===undefined){
419-
thrownewERR_VM_MODULE_NOT_MODULE();
420-
}
402+
validateInternalField(this,kWrap,'SyntheticModule');
421403
validateString(name,'name');
422404
if(this[kWrap].getStatus()<kInstantiated){
423405
thrownewERR_VM_MODULE_STATUS('must be linked');
@@ -432,7 +414,7 @@ function importModuleDynamicallyWrap(importModuleDynamically) {
432414
if(isModuleNamespaceObject(m)){
433415
returnm;
434416
}
435-
if(!m||m[kWrap]===undefined){
417+
if(!isModule(m)){
436418
thrownewERR_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(constvalueof[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"argumentmustbeaninstanceofModule/,
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
awaitassert.rejects(async()=>{
217217
awaitModule.prototype[method]();
218218
},{
219-
code: 'ERR_VM_MODULE_NOT_MODULE',
220-
message: /ProvidedmoduleisnotaninstanceofModule/
219+
code: 'ERR_INVALID_ARG_TYPE',
220+
message: /The"this"argumentmustbeaninstanceofModule/
221221
});
222222
});
223223
}
@@ -241,8 +241,8 @@ function checkInvalidCachedData() {
241241

242242
functioncheckGettersErrors(){
243243
constexpectedError={
244-
code: 'ERR_VM_MODULE_NOT_MODULE',
245-
message: /ProvidedmoduleisnotaninstanceofModule/
244+
code: 'ERR_INVALID_ARG_TYPE',
245+
message: /The"this"argumentmustbeaninstanceof(?:Module|SourceTextModule)/,
246246
};
247247
constgetters=['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
deleteglobalThis.fiveResult;
2929
}
3030

31+
asyncfunctioninvalidLinkValue(){
32+
constinvalidValues=[
33+
undefined,
34+
null,
35+
{},
36+
SourceTextModule.prototype,
37+
];
38+
39+
for(constvalueofinvalidValues){
40+
constmodule=newSourceTextModule('import "foo"');
41+
awaitassert.rejects(module.link(()=>value),{
42+
code: 'ERR_VM_MODULE_NOT_MODULE',
43+
});
44+
}
45+
}
46+
3147
asyncfunctiondepth(){
3248
constfoo=newSourceTextModule('export default 5');
3349
awaitfoo.link(common.mustNotCall());
@@ -143,6 +159,7 @@ const finished = common.mustCall();
143159

144160
(asyncfunctionmain(){
145161
awaitsimple();
162+
awaitinvalidLinkValue();
146163
awaitdepth();
147164
awaitcircular();
148165
awaitcircular2();

‎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(constvalueof[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: /ProvidedmoduleisnotaninstanceofModule/
73+
code: 'ERR_INVALID_ARG_TYPE',
74+
message: /The"this"argumentmustbeaninstanceofSyntheticModule/
7575
});
7676
}
7777

0 commit comments

Comments
 (0)