diff --git a/CHANGELOG.md b/CHANGELOG.md index 0fbc31b..2a28da1 100644 --- a/CHANGELOG.md +++ b/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). diff --git a/README.md b/README.md index fe72773..c25e063 100644 --- a/README.md +++ b/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. diff --git a/index.d.ts b/index.d.ts index 4ad0508..dfbf62c 100644 --- a/index.d.ts +++ b/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. */ diff --git a/lib/nodevm.js b/lib/nodevm.js index c224758..d666c70 100644 --- a/lib/nodevm.js +++ b/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"] - host to require modules in host and proxy them to sandbox. + * @param {("host"|"sandbox"|pathContextCallback)} [options.require.context="host"] - + * host to require modules in host and proxy them to sandbox. * sandbox to load, compile and require modules in sandbox. + * pathContext(modulePath, ext) to choose a mode per module (full path provided). * Builtin modules except events 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] - * WARNING: Allowing this is a security risk as scripts can create a NodeVM which can require any host module. * Allow nesting of VMs. diff --git a/lib/resolver-compat.js b/lib/resolver-compat.js index ce6b4cf..60ff571 100644 --- a/lib/resolver-compat.js +++ b/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; diff --git a/lib/resolver.js b/lib/resolver.js index fd86aff..b05bd39 100644 --- a/lib/resolver.js +++ b/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) { diff --git a/lib/setup-sandbox.js b/lib/setup-sandbox.js index 539ce04..71afa52 100644 --- a/lib/setup-sandbox.js +++ b/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); } } diff --git a/lib/transformer.js b/lib/transformer.js index 9919453..47c5ac9 100644 --- a/lib/transformer.js +++ b/lib/transformer.js @@ -113,12 +113,23 @@ 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, @@ -126,17 +137,6 @@ function transformer(args, body, isAsync, isGenerator, filename) { 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') { diff --git a/package-lock.json b/package-lock.json index 8ed2787..f4e8dd5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "vm2", - "version": "3.9.15", + "version": "3.9.16", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "vm2", - "version": "3.9.15", + "version": "3.9.16", "license": "MIT", "dependencies": { "acorn": "^8.7.0", diff --git a/package.json b/package.json index 685d41b..600dee8 100644 --- a/package.json +++ b/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", diff --git a/test/nodevm.js b/test/nodevm.js index 843ce82..41669eb 100644 --- a/test/nodevm.js +++ b/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: {