diff --git a/CHANGELOG.md b/CHANGELOG.md index a24ec6c..2b54c4f 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/README.md b/README.md index 3625d91..d45e714 100644 --- a/README.md +++ b/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 diff --git a/lib/bridge.js b/lib/bridge.js index 520e5af..fb3d83b 100644 --- a/lib/bridge.js +++ b/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++) { diff --git a/lib/nodevm.js b/lib/nodevm.js index 6192ca9..2931eae 100644 --- a/lib/nodevm.js +++ b/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; diff --git a/lib/resolver-compat.js b/lib/resolver-compat.js index 663a7ce..86453f0 100644 --- a/lib/resolver-compat.js +++ b/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,7 +293,13 @@ 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; @@ -284,8 +307,16 @@ function resolverFromOptions(vm, options, override, compiler) { 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; diff --git a/lib/resolver.js b/lib/resolver.js index 008104c..7575463 100644 --- a/lib/resolver.js +++ b/lib/resolver.js @@ -508,7 +508,7 @@ 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. @@ -516,7 +516,7 @@ class DefaultResolver extends Resolver { 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). diff --git a/package.json b/package.json index 422416d..f5a3b30 100644 --- a/package.json +++ b/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", diff --git a/test/node_modules/with-exports/main.js b/test/node_modules/with-exports/main.js new file mode 100644 index 0000000..e12f1fe --- /dev/null +++ b/test/node_modules/with-exports/main.js @@ -0,0 +1 @@ +exports.ok = true; diff --git a/test/node_modules/with-exports/package.json b/test/node_modules/with-exports/package.json new file mode 100644 index 0000000..2ff222b --- /dev/null +++ b/test/node_modules/with-exports/package.json @@ -0,0 +1,8 @@ +{ + "name": "with-exports", + "exports": { + ".": { + "require": "./main.js" + } + } +} \ No newline at end of file diff --git a/test/nodevm.js b/test/nodevm.js index 5df40ab..2c1e02c 100644 --- a/test/nodevm.js +++ b/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; diff --git a/test/vm.js b/test/vm.js index b7ccd72..e1ac4d6 100644 --- a/test/vm.js +++ b/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; });