Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to run NodeVM modules in strict mode #313

Merged
merged 2 commits into from May 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Expand Up @@ -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.

Expand Down
50 changes: 44 additions & 6 deletions lib/main.js
Expand Up @@ -27,6 +27,10 @@ const {EventEmitter} = require('events');
const {INSPECT_MAX_BYTES} = require('buffer');
const helpers = require('./helpers.js');

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.
*
Expand Down Expand Up @@ -241,6 +245,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.
*
Expand Down Expand Up @@ -354,6 +366,10 @@ class VMScript {
value: null,
writable: true
},
_compiledNodeVMStrict: {
value: null,
writable: true
},
_compiledCode: {
value: null,
writable: true
Expand Down Expand Up @@ -382,6 +398,7 @@ class VMScript {
this._suffix = strSuffix;
this._compiledVM = null;
this._compiledNodeVM = null;
this._compiledNodeVMStrict = null;
return this;
}

Expand Down Expand Up @@ -454,7 +471,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;
}
Expand Down Expand Up @@ -1017,6 +1049,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 = {}) {
Expand All @@ -1035,7 +1068,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;
Expand Down Expand Up @@ -1123,7 +1157,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 {
Expand All @@ -1135,8 +1169,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
});
Expand Down Expand Up @@ -1290,7 +1325,10 @@ const HOST = {
INSPECT_MAX_BYTES,
VM,
NodeVM,
helpers
helpers,
MODULE_PREFIX,
STRICT_MODULE_PREFIX,
MODULE_SUFFIX
};

exports.VMError = VMError;
Expand Down
2 changes: 1 addition & 1 deletion lib/sandbox.js
Expand Up @@ -61,7 +61,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, {
Expand Down
5 changes: 5 additions & 0 deletions test/nodevm.js
Expand Up @@ -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
Expand Down
33 changes: 16 additions & 17 deletions test/vm.js
Expand Up @@ -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
});
});
Expand Down Expand Up @@ -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', () => {
Expand Down