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 <[email protected]>
PR-URL: #52162
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
# Conflicts:
#	lib/internal/vm/module.js

Author: legendecas

lib/internal/vm/module.js

@@ -7,7 +7,9 @@ const {
   ArrayPrototypeIndexOf,
   ArrayPrototypeSome,
   ObjectDefineProperty,
+  ObjectFreeze,
   ObjectGetPrototypeOf,
+  ObjectPrototypeHasOwnProperty,
   ObjectSetPrototypeOf,
   ReflectApply,
   SafePromiseAllReturnVoid,
@@ -43,6 +45,7 @@ const {
   validateObject,
   validateUint32,
   validateString,
+  validateInternalField,
 } = require('internal/validators');
 
 const binding = internalBinding('module_wrap');
@@ -75,6 +78,13 @@ const kLink = Symbol('kLink');
 
 const { isContext } = require('internal/vm');
 
+function isModule(object) {
+  if (typeof object !== 'object' || object === null || !ObjectPrototypeHasOwnProperty(object, kWrap)) {
+    return false;
+  }
+  return true;
+}
+
 class Module {
   constructor(options) {
     emitExperimentalWarning('VM Modules');
@@ -148,50 +158,38 @@ class Module {
   }
 
   get identifier() {
-    if (this[kWrap] === undefined) {
-      throw new ERR_VM_MODULE_NOT_MODULE();
-    }
+    validateInternalField(this, kWrap, 'Module');
     return this[kWrap].url;
   }
 
   get context() {
-    if (this[kWrap] === undefined) {
-      throw new ERR_VM_MODULE_NOT_MODULE();
-    }
+    validateInternalField(this, kWrap, 'Module');
     return this[kContext];
   }
 
   get namespace() {
-    if (this[kWrap] === undefined) {
-      throw new ERR_VM_MODULE_NOT_MODULE();
-    }
+    validateInternalField(this, kWrap, 'Module');
     if (this[kWrap].getStatus() < kInstantiated) {
       throw new ERR_VM_MODULE_STATUS('must not be unlinked or linking');
     }
     return this[kWrap].getNamespace();
   }
 
   get status() {
-    if (this[kWrap] === undefined) {
-      throw new ERR_VM_MODULE_NOT_MODULE();
-    }
+    validateInternalField(this, kWrap, 'Module');
     return STATUS_MAP[this[kWrap].getStatus()];
   }
 
   get error() {
-    if (this[kWrap] === undefined) {
-      throw new ERR_VM_MODULE_NOT_MODULE();
-    }
+    validateInternalField(this, kWrap, 'Module');
     if (this[kWrap].getStatus() !== kErrored) {
       throw new ERR_VM_MODULE_STATUS('must be errored');
     }
     return this[kWrap].getError();
   }
 
   async link(linker) {
-    if (this[kWrap] === undefined) {
-      throw new ERR_VM_MODULE_NOT_MODULE();
-    }
+    validateInternalField(this, kWrap, 'Module');
     validateFunction(linker, 'linker');
     if (this.status === 'linked') {
       throw new ERR_VM_MODULE_ALREADY_LINKED();
@@ -204,10 +202,7 @@ class Module {
   }
 
   async evaluate(options = kEmptyObject) {
-    if (this[kWrap] === undefined) {
-      throw new ERR_VM_MODULE_NOT_MODULE();
-    }
-
+    validateInternalField(this, kWrap, 'Module');
     validateObject(options, 'options');
 
     let timeout = options.timeout;
@@ -230,9 +225,7 @@ class Module {
   }
 
   [customInspectSymbol](depth, options) {
-    if (this[kWrap] === undefined) {
-      throw new ERR_VM_MODULE_NOT_MODULE();
-    }
+    validateInternalField(this, kWrap, 'Module');
     if (typeof depth === 'number' && depth < 0)
       return this;
 
@@ -307,7 +300,7 @@ class SourceTextModule extends Module {
 
       const promises = this[kWrap].link(async (identifier, attributes) => {
         const module = await linker(identifier, this, { attributes, assert: attributes });
-        if (module[kWrap] === undefined) {
+        if (!isModule(module)) {
           throw new ERR_VM_MODULE_NOT_MODULE();
         }
         if (module.context !== this.context) {
@@ -338,19 +331,13 @@ class SourceTextModule extends Module {
   }
 
   get dependencySpecifiers() {
-    if (this[kWrap] === undefined) {
-      throw new ERR_VM_MODULE_NOT_MODULE();
-    }
-    if (this[kDependencySpecifiers] === undefined) {
-      this[kDependencySpecifiers] = this[kWrap].getStaticDependencySpecifiers();
-    }
+    validateInternalField(this, kDependencySpecifiers, 'SourceTextModule');
+    this[kDependencySpecifiers] ??= ObjectFreeze(this[kWrap].getStaticDependencySpecifiers());
     return this[kDependencySpecifiers];
   }
 
   get status() {
-    if (this[kWrap] === undefined) {
-      throw new ERR_VM_MODULE_NOT_MODULE();
-    }
+    validateInternalField(this, kDependencySpecifiers, 'SourceTextModule');
     if (this.#error !== kNoError) {
       return 'errored';
     }
@@ -361,9 +348,7 @@ class SourceTextModule extends Module {
   }
 
   get error() {
-    if (this[kWrap] === undefined) {
-      throw new ERR_VM_MODULE_NOT_MODULE();
-    }
+    validateInternalField(this, kDependencySpecifiers, 'SourceTextModule');
     if (this.#error !== kNoError) {
       return this.#error;
     }
@@ -416,9 +401,7 @@ class SyntheticModule extends Module {
   }
 
   setExport(name, value) {
-    if (this[kWrap] === undefined) {
-      throw new ERR_VM_MODULE_NOT_MODULE();
-    }
+    validateInternalField(this, kWrap, 'SyntheticModule');
     validateString(name, 'name');
     if (this[kWrap].getStatus() < kInstantiated) {
       throw new ERR_VM_MODULE_STATUS('must be linked');
@@ -433,7 +416,7 @@ function importModuleDynamicallyWrap(importModuleDynamically) {
     if (isModuleNamespaceObject(m)) {
       return m;
     }
-    if (!m || m[kWrap] === undefined) {
+    if (!isModule(m)) {
       throw new ERR_VM_MODULE_NOT_MODULE();
     }
     if (m.status === 'errored') {

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

@@ -84,13 +84,15 @@ const util = require('util');
 
   assert.strictEqual(util.inspect(m, { depth: -1 }), '[SourceTextModule]');
 
-  assert.throws(
-    () => m[util.inspect.custom].call({ __proto__: null }),
-    {
-      code: 'ERR_VM_MODULE_NOT_MODULE',
-      message: 'Provided module is not an instance of Module'
-    },
-  );
+  for (const value of [null, { __proto__: null }, SourceTextModule.prototype]) {
+    assert.throws(
+      () => m[util.inspect.custom].call(value),
+      {
+        code: 'ERR_INVALID_ARG_TYPE',
+        message: /The "this" argument must be an instance of Module/,
+      },
+    );
+  }
 }
 
 {

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

@@ -216,8 +216,8 @@ async function checkInvalidOptionForEvaluate() {
       await assert.rejects(async () => {
         await Module.prototype[method]();
       }, {
-        code: 'ERR_VM_MODULE_NOT_MODULE',
-        message: /Provided module is not an instance of Module/
+        code: 'ERR_INVALID_ARG_TYPE',
+        message: /The "this" argument must be an instance of Module/
       });
     });
   }
@@ -241,8 +241,8 @@ function checkInvalidCachedData() {
 
 function checkGettersErrors() {
   const expectedError = {
-    code: 'ERR_VM_MODULE_NOT_MODULE',
-    message: /Provided module is not an instance of Module/
+    code: 'ERR_INVALID_ARG_TYPE',
+    message: /The "this" argument must be an instance of (?:Module|SourceTextModule)/,
   };
   const getters = ['identifier', 'context', 'namespace', 'status', 'error'];
   getters.forEach((getter) => {

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

@@ -28,6 +28,22 @@ async function simple() {
   delete globalThis.fiveResult;
 }
 
+async function invalidLinkValue() {
+  const invalidValues = [
+    undefined,
+    null,
+    {},
+    SourceTextModule.prototype,
+  ];
+
+  for (const value of invalidValues) {
+    const module = new SourceTextModule('import "foo"');
+    await assert.rejects(module.link(() => value), {
+      code: 'ERR_VM_MODULE_NOT_MODULE',
+    });
+  }
+}
+
 async function depth() {
   const foo = new SourceTextModule('export default 5');
   await foo.link(common.mustNotCall());
@@ -143,6 +159,7 @@ const finished = common.mustCall();
 
 (async function main() {
   await simple();
+  await invalidLinkValue();
   await depth();
   await circular();
   await circular2();

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

@@ -66,12 +66,12 @@ const assert = require('assert');
     });
   }
 
-  {
+  for (const value of [null, {}, SyntheticModule.prototype]) {
     assert.throws(() => {
-      SyntheticModule.prototype.setExport.call({}, 'foo');
+      SyntheticModule.prototype.setExport.call(value, 'foo');
     }, {
-      code: 'ERR_VM_MODULE_NOT_MODULE',
-      message: /Provided module is not an instance of Module/
+      code: 'ERR_INVALID_ARG_TYPE',
+      message: /The "this" argument must be an instance of SyntheticModule/
     });
   }