Merge pull request #313 from XmiliaH/fix-strict-modules

Add option to run NodeVM modules in strict mode

README.md

@@ -144,6 +144,7 @@ Unlike `VM`, `NodeVM` lets you require modules same way like in regular Node's c
 * `wrapper` - `commonjs` (default) to wrap script into CommonJS wrapper, `none` to retrieve value returned by the script.
 * `argv` - Array to be passed to `process.argv`.
 * `env` - Object to be passed to `process.env`.
+* `strict` - `true` to loaded modules in strict mode (default: `false`).
 
 **IMPORTANT**: Timeout is not effective for NodeVM so it is not immune to `while (true) {}` or similar evil.
 

lib/main.js

@@ -32,6 +32,10 @@ const importModuleDynamically = () => {
 	throw 'Dynamic imports are not allowed.';
 };
 
+const MODULE_PREFIX = '(function (exports, require, module, __filename, __dirname) { ';
+const STRICT_MODULE_PREFIX = MODULE_PREFIX + '"use strict"; ';
+const MODULE_SUFFIX = '\n});';
+
 /**
  * Load a script from a file and compile it.
  * 
@@ -247,6 +251,14 @@ class VMScript {
 	 * @memberOf VMScript#
 	 */
 
+	/**
+	 * The compiled vm.Script for the NodeVM in strict mode or if not compiled <code>null</code>.
+	 * 
+	 * @private
+	 * @member {?vm.Script} _compiledNodeVMStrict
+	 * @memberOf VMScript#
+	 */
+
 	/**
 	 * The resolved compiler to use to get the JavaScript code.
 	 * 
@@ -360,6 +372,10 @@ class VMScript {
 				value: null,
 				writable: true
 			},
+			_compiledNodeVMStrict: {
+				value: null,
+				writable: true
+			},
 			_compiledCode: {
 				value: null,
 				writable: true
@@ -388,6 +404,7 @@ class VMScript {
 		this._suffix = strSuffix;
 		this._compiledVM = null;
 		this._compiledNodeVM = null;
+		this._compiledNodeVMStrict = null;
 		return this;
 	}
 
@@ -461,7 +478,22 @@ class VMScript {
 	_compileNodeVM() {
 		let script = this._compiledNodeVM;
 		if (!script) {
-			this._compiledNodeVM = script = this._compile('(function (exports, require, module, __filename, __dirname) { ', '\n})');
+			this._compiledNodeVM = script = this._compile(MODULE_PREFIX, MODULE_SUFFIX);
+		}
+		return script;
+	}
+
+	/**
+	 * Will return the cached version of the script intended for NodeVM in strict mode or compile it.
+	 * 
+	 * @private
+	 * @return {vm.Script} The compiled script
+	 * @throws {SyntaxError} If there is a syntax error in the script.
+	 */
+	_compileNodeVMStrict() {
+		let script = this._compiledNodeVMStrict;
+		if (!script) {
+			this._compiledNodeVMStrict = script = this._compile(STRICT_MODULE_PREFIX, MODULE_SUFFIX);
 		}
 		return script;
 	}
@@ -1032,6 +1064,7 @@ class NodeVM extends VM {
 	 * This object will not be copied and the script can change this object.
 	 * @param {Object} [options.env={}] - Environment map passed to <code>process.env</code>. 
 	 * This object will not be copied and the script can change this object.
+	 * @param {boolean} [options.strict=false] - If modules should be loaded in strict mode.
 	 * @throws {VMError} If the compiler is unknown.
 	 */
 	constructor(options = {}) {
@@ -1050,7 +1083,8 @@ class NodeVM extends VM {
 			require: options.require || false,
 			nesting: options.nesting || false,
 			wrapper: options.wrapper || 'commonjs',
-			sourceExtensions: options.sourceExtensions || ['js']
+			sourceExtensions: options.sourceExtensions || ['js'],
+			strict: options.strict || false
 		}});
 
 		let sandboxScript = CACHE.sandboxScript;
@@ -1138,7 +1172,7 @@ class NodeVM extends VM {
 		let script;
 
 		if (code instanceof VMScript) {
-			script = code._compileNodeVM();
+			script = this.options.strict ? code._compileNodeVMStrict() : code._compileNodeVM();
 			resolvedFilename = pa.resolve(code.filename);
 			dirname = pa.dirname(resolvedFilename);
 		} else {
@@ -1150,8 +1184,9 @@ class NodeVM extends VM {
 				resolvedFilename = null;
 				dirname = null;
 			}
-			script = new vm.Script('(function (exports, require, module, __filename, __dirname) { ' +
-					this._compiler(code, unresolvedFilename) + '\n})', {
+			const prefix = this.options.strict ? STRICT_MODULE_PREFIX : MODULE_PREFIX;
+			script = new vm.Script(prefix +
+					this._compiler(code, unresolvedFilename) + MODULE_SUFFIX, {
 				filename: unresolvedFilename,
 				displayErrors: false,
 				importModuleDynamically
@@ -1306,7 +1341,10 @@ const HOST = {
 	INSPECT_MAX_BYTES,
 	VM,
 	NodeVM,
-	helpers
+	helpers,
+	MODULE_PREFIX,
+	STRICT_MODULE_PREFIX,
+	MODULE_SUFFIX
 };
 
 exports.VMError = VMError;

lib/sandbox.js

@@ -66,7 +66,7 @@ return ((vm, host) => {
 					let contents = fs.readFileSync(filename, 'utf8');
 					contents = vm._compiler(contents, filename);
 
-					const code = `(function (exports, require, module, __filename, __dirname) { 'use strict'; ${contents} \n});`;
+					const code = host.STRICT_MODULE_PREFIX + contents + host.MODULE_SUFFIX;
 
 					// Precompile script
 					script = new Script(code, {

test/nodevm.js

@@ -56,6 +56,11 @@ describe('NodeVM', () => {
 		assert.doesNotThrow(() => vm.run('#!shebang'));
 	});
 
+	it('strict', () => {
+		assert.doesNotThrow(() => vm.run('newGlobal = 2;'));
+		assert.throws(() => new NodeVM({strict: true}).run('newGlobal = 2;'), /ReferenceError: newGlobal is not defined/);
+	});
+
 	it.skip('timeout (not supported by Node\'s VM)', () => {
 		assert.throws(() => new NodeVM({
 			timeout: 10

test/vm.js

@@ -283,26 +283,25 @@ describe('contextify', () => {
 describe('VM', () => {
 	let vm;
 
-	before(() => {
-		const sandbox = {
-			round(number) {
-				return Math.round(number);
-			},
-			sub: {}
-		};
+	const sandbox = {
+		round(number) {
+			return Math.round(number);
+		},
+		sub: {}
+	};
 
-		Object.defineProperty(sandbox.sub, 'getter', {
-			get() {
-				const results = [];
-				while (true) {
-					results.push(1);
-				}
-				return results;
+	Object.defineProperty(sandbox.sub, 'getter', {
+		get() {
+			const results = [];
+			while (true) {
+				results.push(1);
 			}
-		});
+			return results;
+		}
+	});
 
+	before(() => {
 		vm = new VM({
-			timeout: 10,
 			sandbox
 		});
 	});
@@ -345,7 +344,7 @@ describe('VM', () => {
 		assert.throws(() => new VM({
 			timeout: 10
 		}).run('while (true) {}'), message);
-		assert.throws(() => vm.run('sub.getter'), message);
+		assert.throws(() => new VM({timeout: 10, sandbox}).run('sub.getter'), message);
 	});
 
 	it('timers', () => {