Merge pull request #400 from XmiliaH/bugfixes

Some bugfixes for v3.9.6

CHANGELOG.md

@@ -1,3 +1,11 @@
+v3.9.7 (2022-02-10)
+-------------------
+[fix] Allow relative require from base script  
+[fix] Fix issue with modules with exports clause in package json  
+[fix] Added missing whitelist check before custom require  
+[fix] Revert plain object toString behavior  
+[fix] Root path check improved  
+
 v3.9.6 (2022-02-08)
 -------------------
 [fix] Security fixes (XmiliaH)  

README.md

@@ -377,6 +377,9 @@ vm2 ./script.js
 ## Known Issues
 
 * It is not possible to define a class that extends a proxied class.
+* Direct eval does not work.
+* Logging sandbox arrays will repeat the array part in the properties.
+* Source code transformations can result a different source string for a function.
 
 ## Deployment
 

lib/bridge.js

@@ -30,7 +30,6 @@ const globalsList = [
 	'Set',
 	'WeakSet',
 	'Promise',
-	'Object',
 	'Function'
 ];
 
@@ -48,6 +47,7 @@ const OPNA = 'Operation not allowed on contextified object.';
 
 const thisGlobalPrototypes = {
 	__proto__: null,
+	Object: Object.prototype,
 	Array: Array.prototype
 };
 
@@ -927,6 +927,7 @@ function createBridge(otherInit, registerProxy) {
 		thisReflectApply(thisWeakMapSet, mappingOtherToThis, [other, obj]);
 	}
 
+	thisAddProtoMapping(thisGlobalPrototypes.Object, otherGlobalPrototypes.Object);
 	thisAddProtoMapping(thisGlobalPrototypes.Array, otherGlobalPrototypes.Array);
 
 	for (let i = 0; i < globalsList.length; i++) {

lib/nodevm.js

@@ -389,6 +389,7 @@ class NodeVM extends VM {
 				const resolvedFilename = this._resolver.pathResolve(code.filename);
 				dirname = this._resolver.pathDirname(resolvedFilename);
 				sandboxModule = new (this._Module)(resolvedFilename, dirname);
+				this._resolver.registerModule(sandboxModule, resolvedFilename, dirname, null, false);
 			}
 		} else {
 			const unresolvedFilename = filename || 'vm.js';
@@ -397,6 +398,7 @@ class NodeVM extends VM {
 					const resolvedFilename = this._resolver.pathResolve(filename);
 					dirname = this._resolver.pathDirname(resolvedFilename);
 					sandboxModule = new (this._Module)(resolvedFilename, dirname);
+					this._resolver.registerModule(sandboxModule, resolvedFilename, dirname, null, false);
 				} else {
 					sandboxModule = new (this._Module)(null, null);
 					sandboxModule.id = unresolvedFilename;

lib/resolver-compat.js

@@ -34,19 +34,24 @@ function escapeRegExp(string) {
 	return string.replace(/[.*+\-?^${}()|[\]\\]/g, '\\$&'); // $& means the whole matched string
 }
 
-function makeExternalMatcher(obj) {
-	const regexString = escapeRegExp(obj).replace(/\\\\|\//g, '[\\\\/]')
+function makeExternalMatcherRegex(obj) {
+	return escapeRegExp(obj).replace(/\\\\|\//g, '[\\\\/]')
 		.replace(/\\\*\\\*/g, '.*').replace(/\\\*/g, '[^\\\\/]*').replace(/\\\?/g, '[^\\\\/]');
+}
+
+function makeExternalMatcher(obj) {
+	const regexString = makeExternalMatcherRegex(obj);
 	return new RegExp(`[\\\\/]node_modules[\\\\/]${regexString}(?:[\\\\/](?!(?:.*[\\\\/])?node_modules[\\\\/]).*)?$`);
 }
 
-class TransitiveResolver extends DefaultResolver {
+class LegacyResolver extends DefaultResolver {
 
-	constructor(builtinModules, checkPath, globalPaths, pathContext, customResolver, hostRequire, compiler, externals) {
+	constructor(builtinModules, checkPath, globalPaths, pathContext, customResolver, hostRequire, compiler, externals, allowTransitive) {
 		super(builtinModules, checkPath, globalPaths, pathContext, customResolver, hostRequire, compiler);
 		this.externals = externals;
 		this.currMod = undefined;
 		this.trustedMods = new WeakMap();
+		this.allowTransitive = allowTransitive;
 	}
 
 	isPathAllowed(path) {
@@ -55,7 +60,13 @@ class TransitiveResolver extends DefaultResolver {
 
 	isPathAllowedForModule(path, mod) {
 		if (!super.isPathAllowed(path)) return false;
-		if (mod && (mod.allowTransitive || path.startsWith(mod.path))) return true;
+		if (mod) {
+			if (mod.allowTransitive) return true;
+			if (path.startsWith(mod.path)) {
+				const rem = path.slice(mod.path.length);
+				if (!/(?:^|[\\\\/])node_modules(?:$|[\\\\/])/.test(rem)) return true;
+			}
+		}
 		return this.externals.some(regex => regex.test(path));
 	}
 
@@ -65,7 +76,8 @@ class TransitiveResolver extends DefaultResolver {
 			filename,
 			path,
 			paths: this.genLookupPaths(path),
-			allowTransitive: (direct && trustedParent && trustedParent.allowTransitive) || this.externals.some(regex => regex.test(filename))
+			allowTransitive: this.allowTransitive &&
+				((direct && trustedParent && trustedParent.allowTransitive) || this.externals.some(regex => regex.test(filename)))
 		});
 	}
 
@@ -101,9 +113,14 @@ class TransitiveResolver extends DefaultResolver {
 	loadJS(vm, mod, filename) {
 		filename = this.pathResolve(filename);
 		this.checkAccess(mod, filename);
-		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});
+		if (this.pathContext(filename, 'js') === 'sandbox') {
+			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});
+		} else {
+			const m = this.hostRequire(filename);
+			mod.exports = vm.readonly(m);
+		}
 	}
 
 }
@@ -276,16 +293,30 @@ function resolverFromOptions(vm, options, override, compiler) {
 	if (rootPaths) {
 		const checkedRootPaths = (Array.isArray(rootPaths) ? rootPaths : [rootPaths]).map(f => pa.resolve(f));
 		checkPath = (filename) => {
-			return checkedRootPaths.some(path => filename.startsWith(path));
+			return checkedRootPaths.some(path => {
+				if (!filename.startsWith(path)) return false;
+				const len = path.length;
+				if (filename.length === len) return true;
+				const sep = filename[len];
+				return sep === '/' || sep === pa.sep;
+			});
 		};
 	} else {
 		checkPath = () => true;
 	}
 
 	let newCustomResolver = defaultCustomResolver;
 	let externals = undefined;
+	let external = undefined;
 	if (customResolver) {
+		let externalCache;
 		newCustomResolver = (resolver, x, path, extList) => {
+			if (external && !(resolver.pathIsAbsolute(x) || resolver.pathIsRelative(x))) {
+				if (!externalCache) {
+					externalCache = external.map(ext => new RegExp(makeExternalMatcherRegex(ext)));
+				}
+				if (!externalCache.some(regex => regex.test(x))) return undefined;
+			}
 			const resolved = customResolver(x, path);
 			if (!resolved) return undefined;
 			if (externals) externals.push(new RegExp('^' + escapeRegExp(resolved)));
@@ -297,7 +328,6 @@ function resolverFromOptions(vm, options, override, compiler) {
 		return new DefaultResolver(builtins, checkPath, [], () => context, newCustomResolver, hostRequire, compiler);
 	}
 
-	let external;
 	let transitive = false;
 	if (Array.isArray(externalOpt)) {
 		external = externalOpt;
@@ -306,12 +336,7 @@ function resolverFromOptions(vm, options, override, compiler) {
 		transitive = context === 'sandbox' && externalOpt.transitive;
 	}
 	externals = external.map(makeExternalMatcher);
-	if (transitive) return new TransitiveResolver(builtins, checkPath, [], () => context, newCustomResolver, hostRequire, compiler, externals);
-	const nextCheckPath = checkPath;
-	checkPath = (filename) => {
-		return nextCheckPath(filename) && externals.some(regex => regex.test(filename));
-	};
-	return new DefaultResolver(builtins, checkPath, [], () => context, newCustomResolver, hostRequire, compiler);
+	return new LegacyResolver(builtins, checkPath, [], () => context, newCustomResolver, hostRequire, compiler, externals, transitive);
 }
 
 exports.resolverFromOptions = resolverFromOptions;

lib/resolver.js

@@ -508,15 +508,15 @@ class DefaultResolver extends Resolver {
 		// 2. If X does not match this pattern or DIR/NAME/package.json is not a file,
 		//    return.
 		if (!res) return undefined;
-		const scope = this.pathConcat(dir, res[0]);
+		const scope = this.pathConcat(dir, res[1]);
 		const pack = this.readPackage(scope);
 		if (!pack) return undefined;
 		// 3. Parse DIR/NAME/package.json, and look for "exports" field.
 		// 4. If "exports" is null or undefined, return.
 		if (!pack.exports) return undefined;
 		// 5. let MATCH = PACKAGE_EXPORTS_RESOLVE(pathToFileURL(DIR/NAME), "." + SUBPATH,
 		//    `package.json` "exports", ["node", "require"]) defined in the ESM resolver.
-		const match = this.packageExportsResolve(scope, '.' + res[1], pack.exports, ['node', 'require'], extList);
+		const match = this.packageExportsResolve(scope, '.' + (res[2] || ''), pack.exports, ['node', 'require'], extList);
 		// 6. RESOLVE_ESM_MATCH(MATCH)
 		return this.resolveEsmMatch(match, x, extList);
 	}
@@ -777,7 +777,7 @@ class DefaultResolver extends Resolver {
 			for (let i = 0; i < keys.length; i++) {
 				const p = keys[i];
 				// 1. If p equals "default" or conditions contains an entry for p, then
-				if (p === 'default' || conditions.contains(p)) {
+				if (p === 'default' || conditions.includes(p)) {
 					// a. Let targetValue be the value of the p property in target.
 					const targetValue = target[p];
 					// b. Let resolved be the result of PACKAGE_TARGET_RESOLVE( packageURL, targetValue, subpath, pattern, internal, conditions).

package.json

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

test/nodevm.js

@@ -272,6 +272,54 @@ describe('modules', () => {
 		assert.equal(vm.run("module.exports = require('module-main-without-extension').bar()", __filename), 1);
 	});
 
+	it('module with exports', () => {
+		const vm = new NodeVM({
+			require: {
+				external: [
+					'with-exports'
+				]
+			}
+		});
+
+		assert.strictEqual(vm.run("module.exports = require('with-exports')", __filename).ok, true);
+
+	});
+
+	it('whitelist check before custom resolver', () => {
+		const vm = new NodeVM({
+			require: {
+				external: [],
+				resolve: () => {
+					throw new Error('Unexpected');
+				},
+			},
+		});
+
+		assert.throws(() => vm.run("require('mocha')", __filename), /Cannot find module 'mocha'/);
+	});
+
+	it('root path checking', () => {
+		const vm = new NodeVM({
+			require: {
+				external: true,
+				root: `${__dirname}/node_modules/module`
+			},
+		});
+
+		assert.throws(() => vm.run("require('module2')", __filename), /Cannot find module 'module2'/);
+	});
+
+	it('relative require not allowed to enter node modules', () => {
+		const vm = new NodeVM({
+			require: {
+				external: ['mocha'],
+				root: `${__dirname}`
+			},
+		});
+
+		assert.throws(() => vm.run("require('./node_modules/module2')", __filename), /Cannot find module '\.\/node_modules\/module2'/);
+	});
+
 	it('arguments attack', () => {
 		let vm = new NodeVM;
 

test/vm.js

@@ -350,6 +350,36 @@ describe('contextify', () => {
 		assert.strictEqual(vm.run('test.error.constructor.constructor === Function;'), true);
 	});
 
+	it('tostring', () => {
+		const list = [
+			'Object',
+			'Array',
+			'Number',
+			'String',
+			'Boolean',
+			'Date',
+			'RegExp',
+			'Map',
+			'WeakMap',
+			'Set',
+			'WeakSet',
+			'Function',
+			'RangeError',
+			'ReferenceError',
+			'SyntaxError',
+			'TypeError',
+			'EvalError',
+			'URIError',
+			'Error'
+		];
+		const gen = vm.run('name => new (global[name])()');
+		const oToString = Object.prototype.toString;
+		for (let i = 0; i < list.length; i++) {
+			const obj = list[i];
+			assert.strictEqual(oToString.call(gen(obj)), oToString.call(new (global[obj])()));
+		}
+	});
+
 	after(() => {
 		vm = null;
 	});