Merge remote-tracking branch 'upstream/master' into resolver-api

CHANGELOG.md

@@ -1,3 +1,7 @@
+v3.9.17 (2023-04-17)
+--------------------
+[fix] Multiple security fixes.
+
 v3.9.16 (2023-04-11)
 --------------------
 [fix] Security fix (see https://github.com/patriksimek/vm2/issues/516).

README.md

@@ -141,7 +141,7 @@ Unlike `VM`, `NodeVM` allows you to require modules in the same way that you wou
 * `require.builtin` - Array of allowed built-in modules, accepts ["\*"] for all (default: none). **WARNING**: "\*" can be dangerous as new built-ins can be added.
 * `require.root` - Restricted path(s) where local modules can be required (default: every path).
 * `require.mock` - Collection of mock modules (both external or built-in).
-* `require.context` - `host` (default) to require modules in the host and proxy them into the sandbox. `sandbox` to load, compile, and require modules in the sandbox. Except for `events`, built-in modules are always required in the host and proxied into the sandbox.
+* `require.context` - `host` (default) to require modules in the host and proxy them into the sandbox. `sandbox` to load, compile, and require modules in the sandbox. `callback(moduleFilename, ext)` to dynamically choose a context per module. The default will be sandbox is nothing is specified. Except for `events`, built-in modules are always required in the host and proxied into the sandbox.
 * `require.import` - An array of modules to be loaded into NodeVM on start.
 * `require.resolve` - An additional lookup function in case a module wasn't found in one of the traditional node lookup paths.
 * `require.customRequire` - Use instead of the `require` function to load modules from the host.

index.d.ts

@@ -72,6 +72,11 @@ export type Builtin = BuiltinLoad | {init: (vm: NodeVM)=>void, load: BuiltinLoad
  */
 export type HostRequire = (id: string) => any;
 
+/**
+ * This callback will be called to specify the context to use "per" module. Defaults to 'sandbox' if no return value provided.
+ */
+export type PathContextCallback = (modulePath: string, extensionType: string) => 'host' | 'sandbox';
+
 /**
  *  Require options for a VM
  */
@@ -83,9 +88,10 @@ export interface VMRequire {
   builtin?: readonly string[];
   /*
    * `host` (default) to require modules in host and proxy them to sandbox. `sandbox` to load, compile and
-   * require modules in sandbox. Built-in modules except `events` always required in host and proxied to sandbox
+   * require modules in sandbox or a callback which chooses the context based on the filename.
+   * Built-in modules except `events` always required in host and proxied to sandbox
    */
-  context?: "host" | "sandbox";
+  context?: "host" | "sandbox" | PathContextCallback;
   /** `true`, an array of allowed external modules or an object with external options (default: `false`) */
   external?: boolean | readonly string[] | { modules: readonly string[], transitive: boolean };
   /** Array of modules to be loaded into NodeVM on start. */

lib/nodevm.js

@@ -17,6 +17,17 @@
  * @return {*} The required module object.
  */
 
+/**
+ * This callback will be called to specify the context to use "per" module. Defaults to 'sandbox' if no return value provided.
+ *
+ * NOTE: many interoperating modules must live in the same context.
+ *
+ * @callback pathContextCallback
+ * @param {string} modulePath - The full path to the module filename being requested.
+ * @param {string} extensionType - The module type (node = native, js = cjs/esm module)
+ * @return {("host"|"sandbox")} The context for this module.
+ */
+
 const fs = require('fs');
 const pa = require('path');
 const {
@@ -208,14 +219,16 @@ class NodeVM extends VM {
 	 * @param {string[]} [options.require.builtin=[]] - Array of allowed built-in modules, accepts ["*"] for all.
 	 * @param {(string|string[])} [options.require.root] - Restricted path(s) where local modules can be required. If omitted every path is allowed.
 	 * @param {Object} [options.require.mock] - Collection of mock modules (both external or built-in).
-	 * @param {("host"|"sandbox")} [options.require.context="host"] - <code>host</code> to require modules in host and proxy them to sandbox.
+	 * @param {("host"|"sandbox"|pathContextCallback)} [options.require.context="host"] -
+	 * <code>host</code> to require modules in host and proxy them to sandbox.
 	 * <code>sandbox</code> to load, compile and require modules in sandbox.
+	 * <code>pathContext(modulePath, ext)</code> to choose a mode per module (full path provided).
 	 * Builtin modules except <code>events</code> always required in host and proxied to sandbox.
 	 * @param {string[]} [options.require.import] - Array of modules to be loaded into NodeVM on start.
 	 * @param {resolveCallback} [options.require.resolve] - An additional lookup function in case a module wasn't
 	 * found in one of the traditional node lookup paths.
 	 * @param {customRequire} [options.require.customRequire=require] - Custom require to require host and built-in modules.
-	 * @param {boolean} [option.require.strict=true] - Load required modules in strict mode.
+	 * @param {boolean} [options.require.strict=true] - Load required modules in strict mode.
 	 * @param {boolean} [options.nesting=false] -
 	 * <b>WARNING: Allowing this is a security risk as scripts can create a NodeVM which can require any host module.</b>
 	 * Allow nesting of VMs.

lib/resolver-compat.js

@@ -40,10 +40,10 @@ function makeExternalMatcher(obj) {
 
 class CustomResolver extends DefaultResolver {
 
-	constructor(fileSystem, globalPaths, builtinModules, rootPaths, context, customResolver, hostRequire, compiler, strict) {
+	constructor(fileSystem, globalPaths, builtinModules, rootPaths, pathContext, customResolver, hostRequire, compiler, strict) {
 		super(fileSystem, globalPaths, builtinModules);
 		this.rootPaths = rootPaths;
-		this.context = context;
+		this.pathContext = pathContext;
 		this.customResolver = customResolver;
 		this.hostRequire = hostRequire;
 		this.compiler = compiler;
@@ -60,13 +60,13 @@ class CustomResolver extends DefaultResolver {
 	}
 
 	loadJS(vm, mod, filename) {
-		if (this.context === 'sandbox') return super.loadJS(vm, mod, filename);
+		if (this.pathContext(filename, 'js') !== 'host') return super.loadJS(vm, mod, filename);
 		const m = this.hostRequire(filename);
 		mod.exports = vm.readonly(m);
 	}
 
 	loadNode(vm, mod, filename) {
-		if (this.context === 'sandbox') return super.loadNode(vm, mod, filename);
+		if (this.pathContext(filename, 'node') !== 'host') return super.loadNode(vm, mod, filename);
 		const m = this.hostRequire(filename);
 		mod.exports = vm.readonly(m);
 	}
@@ -94,8 +94,8 @@ class CustomResolver extends DefaultResolver {
 
 class LegacyResolver extends CustomResolver {
 
-	constructor(fileSystem, globalPaths, builtinModules, rootPaths, context, customResolver, hostRequire, compiler, strict, externals, allowTransitive) {
-		super(fileSystem, globalPaths, builtinModules, rootPaths, context, customResolver, hostRequire, compiler, strict);
+	constructor(fileSystem, globalPaths, builtinModules, rootPaths, pathContext, customResolver, hostRequire, compiler, strict, externals, allowTransitive) {
+		super(fileSystem, globalPaths, builtinModules, rootPaths, pathContext, customResolver, hostRequire, compiler, strict);
 		this.externals = externals.map(makeExternalMatcher);
 		this.externalCache = externals.map(pattern => new RegExp(makeExternalMatcherRegex(pattern)));
 		this.currMod = undefined;
@@ -158,10 +158,10 @@ class LegacyResolver extends CustomResolver {
 	}
 
 	loadJS(vm, mod, filename) {
-		if (this.context === 'sandbox') {
+		if (this.pathContext(filename, 'js') !== 'host') {
 			const trustedMod = this.trustedMods.get(mod);
 			const script = this.readScript(filename);
-			vm.run(script, {filename, strict: true, module: mod, wrapper: 'none', dirname: trustedMod ? trustedMod.path : mod.path});
+			vm.run(script, {filename, strict: this.isStrict(filename), module: mod, wrapper: 'none', dirname: trustedMod ? trustedMod.path : mod.path});
 		} else {
 			const m = this.hostRequire(filename);
 			mod.exports = vm.readonly(m);
@@ -217,8 +217,10 @@ function makeResolverFromLegacyOptions(options, override, compiler) {
 
 	const checkedRootPaths = rootPaths ? (Array.isArray(rootPaths) ? rootPaths : [rootPaths]).map(f => fsOpt.resolve(f)) : undefined;
 
+	const pathContext = typeof context === 'function' ? context : (() => context);
+
 	if (typeof externalOpt !== 'object') {
-		return new CustomResolver(fsOpt, [], builtins, checkedRootPaths, context, customResolver, hostRequire, compiler, strict);
+		return new CustomResolver(fsOpt, [], builtins, checkedRootPaths, pathContext, customResolver, hostRequire, compiler, strict);
 	}
 
 	let transitive = false;
@@ -227,9 +229,9 @@ function makeResolverFromLegacyOptions(options, override, compiler) {
 		external = externalOpt;
 	} else {
 		external = externalOpt.modules;
-		transitive = context === 'sandbox' && externalOpt.transitive;
+		transitive = context !== 'host' && externalOpt.transitive;
 	}
-	return new LegacyResolver(fsOpt, [], builtins, checkedRootPaths, context, customResolver, hostRequire, compiler, strict, external, transitive);
+	return new LegacyResolver(fsOpt, [], builtins, checkedRootPaths, pathContext, customResolver, hostRequire, compiler, strict, external, transitive);
 }
 
 exports.makeResolverFromLegacyOptions = makeResolverFromLegacyOptions;

lib/resolver.js

@@ -206,8 +206,15 @@ class DefaultResolver extends Resolver {
 	}
 
 	loadJS(vm, mod, filename) {
-		const script = this.readScript(filename);
-		vm.run(script, {filename, strict: this.isStrict(filename), module: mod, wrapper: 'none', dirname: mod.path});
+		filename = this.pathResolve(filename);
+		this.checkAccess(mod, filename);
+		if (this.pathContext(filename, 'js') !== 'host') {
+			const script = this.readScript(filename);
+			vm.run(script, {filename, strict: this.isStrict(filename), module: mod, wrapper: 'none', dirname: mod.path});
+		} else {
+			const m = this.hostRequire(filename);
+			mod.exports = vm.readonly(m);
+		}
 	}
 
 	loadJSON(vm, mod, filename) {
@@ -216,7 +223,11 @@ class DefaultResolver extends Resolver {
 	}
 
 	loadNode(vm, mod, filename) {
-		throw new VMError('Native modules can be required only with context set to \'host\'.');
+		filename = this.pathResolve(filename);
+		this.checkAccess(mod, filename);
+		if (this.pathContext(filename, 'node') !== 'host') throw new VMError('Native modules can be required only with context set to \'host\'.');
+		const m = this.hostRequire(filename);
+		mod.exports = vm.readonly(m);
 	}
 
 	customResolve(x, path, extList) {

lib/setup-sandbox.js

@@ -439,23 +439,36 @@ global.eval = new LocalProxy(localEval, EvalHandler);
  * Promise sanitization
  */
 
-if (localPromise && !allowAsync) {
+if (localPromise) {
 
 	const PromisePrototype = localPromise.prototype;
 
-	overrideWithProxy(PromisePrototype, 'then', PromisePrototype.then, AsyncErrorHandler);
-	// This seems not to work, and will produce
-	// UnhandledPromiseRejectionWarning: TypeError: Method Promise.prototype.then called on incompatible receiver [object Object].
-	// This is likely caused since the host.Promise.prototype.then cannot use the VM Proxy object.
-	// Contextify.connect(host.Promise.prototype.then, Promise.prototype.then);
+	if (!allowAsync) {
+
+		overrideWithProxy(PromisePrototype, 'then', PromisePrototype.then, AsyncErrorHandler);
+		// This seems not to work, and will produce
+		// UnhandledPromiseRejectionWarning: TypeError: Method Promise.prototype.then called on incompatible receiver [object Object].
+		// This is likely caused since the host.Promise.prototype.then cannot use the VM Proxy object.
+		// Contextify.connect(host.Promise.prototype.then, Promise.prototype.then);
+
+	} else {
+
+		overrideWithProxy(PromisePrototype, 'then', PromisePrototype.then, {
+			__proto__: null,
+			apply(target, thiz, args) {
+				if (args.length > 1) {
+					const onRejected = args[1];
+					if (typeof onRejected === 'function') {
+						args[1] = function wrapper(error) {
+							error = ensureThis(error);
+							return localReflectApply(onRejected, this, [error]);
+						};
+					}
+				}
+				return localReflectApply(target, thiz, args);
+			}
+		});
 
-	if (PromisePrototype.finally) {
-		overrideWithProxy(PromisePrototype, 'finally', PromisePrototype.finally, AsyncErrorHandler);
-		// Contextify.connect(host.Promise.prototype.finally, Promise.prototype.finally);
-	}
-	if (Promise.prototype.catch) {
-		overrideWithProxy(PromisePrototype, 'catch', PromisePrototype.catch, AsyncErrorHandler);
-		// Contextify.connect(host.Promise.prototype.catch, Promise.prototype.catch);
 	}
 
 }

lib/transformer.js

@@ -113,30 +113,30 @@ function transformer(args, body, isAsync, isGenerator, filename) {
 		if (nodeType === 'CatchClause') {
 			const param = node.param;
 			if (param) {
-				if (param.type === 'ObjectPattern') {
+				if (param.type === 'Identifier') {
+					const name = assertType(param, 'Identifier').name;
+					const cBody = assertType(node.body, 'BlockStatement');
+					if (cBody.body.length > 0) {
+						insertions.push({
+							__proto__: null,
+							pos: cBody.body[0].start,
+							order: TO_LEFT,
+							coder: () => `${name}=${INTERNAL_STATE_NAME}.handleException(${name});`
+						});
+					}
+				} else {
 					insertions.push({
 						__proto__: null,
 						pos: node.start,
 						order: TO_RIGHT,
-						coder: () => `catch(${tmpname}){try{throw(${tmpname}=${INTERNAL_STATE_NAME}.handleException(${tmpname}));}`
+						coder: () => `catch(${tmpname}){${tmpname}=${INTERNAL_STATE_NAME}.handleException(${tmpname});try{throw ${tmpname};}`
 					});
 					insertions.push({
 						__proto__: null,
 						pos: node.body.end,
 						order: TO_LEFT,
 						coder: () => `}`
 					});
-				} else {
-					const name = assertType(param, 'Identifier').name;
-					const cBody = assertType(node.body, 'BlockStatement');
-					if (cBody.body.length > 0) {
-						insertions.push({
-							__proto__: null,
-							pos: cBody.body[0].start,
-							order: TO_LEFT,
-							coder: () => `${name}=${INTERNAL_STATE_NAME}.handleException(${name});`
-						});
-					}
 				}
 			}
 		} else if (nodeType === 'WithStatement') {

package.json

@@ -13,7 +13,7 @@
     "alcatraz",
     "contextify"
   ],
-  "version": "3.9.16",
+  "version": "3.9.17",
   "main": "index.js",
   "sideEffects": false,
   "repository": "github:patriksimek/vm2",

test/nodevm.js

@@ -228,6 +228,32 @@ describe('modules', () => {
 		assert.ok(vm.run("require('module1')", __filename));
 	});
 
+	it('allows choosing a context by path', () => {
+		const vm = new NodeVM({
+			require: {
+				external: {
+					modules: ['mocha', 'module1'],
+					transitive: true,
+				},
+				context(module) {
+					if (module.includes('mocha')) return 'host';
+					return 'sandbox';
+				}
+			}
+		});
+		function isVMProxy(obj) {
+			const key = {};
+			const proto = Object.getPrototypeOf(obj);
+			if (!proto) return undefined;
+			proto.isVMProxy = key;
+			const proxy = obj.isVMProxy !== key;
+			delete proto.isVMProxy;
+			return proxy;
+		}
+		assert.equal(isVMProxy(vm.run("module.exports = require('mocha')", __filename)), false, 'Mocha is a proxy');
+		assert.equal(isVMProxy(vm.run("module.exports = require('module1')", __filename)), true, 'Module1 is not a proxy');
+	});
+
 	it('can resolve paths based on a custom resolver', () => {
 		const vm = new NodeVM({
 			require: {