From b4f6e2bd2c4a1ef52fc4483d8e35f28bc4481886 Mon Sep 17 00:00:00 2001 From: XmiliaH Date: Tue, 12 Oct 2021 19:05:01 +0200 Subject: [PATCH] Security Fixes --- lib/contextify.js | 131 ++++++++++++++++++++++++++++++++++++++++++++-- lib/main.js | 87 ++++++++++++++++++++---------- lib/sandbox.js | 4 +- test/vm.js | 30 ++++------- 4 files changed, 201 insertions(+), 51 deletions(-) diff --git a/lib/contextify.js b/lib/contextify.js index 621d4d4..49e404a 100644 --- a/lib/contextify.js +++ b/lib/contextify.js @@ -24,11 +24,11 @@ local.Reflect.isExtensible = Reflect.isExtensible; local.Reflect.preventExtensions = Reflect.preventExtensions; local.Reflect.getOwnPropertyDescriptor = Reflect.getOwnPropertyDescriptor; -function curry(func) { +function uncurryThis(func) { return (thiz, args) => local.Reflect.apply(func, thiz, args); } -const FunctionBind = curry(Function.prototype.bind); +const FunctionBind = uncurryThis(Function.prototype.bind); // global is originally prototype of host.Object so it can be used to climb up from the sandbox. Object.setPrototypeOf(global, Object.prototype); @@ -69,7 +69,7 @@ const Contextified = new host.WeakMap(); const Decontextified = new host.WeakMap(); // We can't use host's hasInstance method -const ObjectHasInstance = curry(local.Object[Symbol.hasInstance]); +const ObjectHasInstance = uncurryThis(local.Object[Symbol.hasInstance]); function instanceOf(value, construct) { try { return ObjectHasInstance(construct, [value]); @@ -1001,6 +1001,131 @@ BufferOverride.inspect = function inspect(recurseTimes, ctx) { }; const LocalBuffer = global.Buffer = Contextify.readonly(host.Buffer, BufferMock); Contextify.connect(host.Buffer.prototype.inspect, BufferOverride.inspect); +Contextify.connect(host.Function.prototype.bind, Function.prototype.bind); + +const oldPrepareStackTraceDesc = Reflect.getOwnPropertyDescriptor(Error, 'prepareStackTrace'); + +let currentPrepareStackTrace = Error.prepareStackTrace; +const wrappedPrepareStackTrace = new host.WeakMap(); +if (typeof currentPrepareStackTrace === 'function') { + wrappedPrepareStackTrace.set(currentPrepareStackTrace, currentPrepareStackTrace); +} + +let OriginalCallSite; +Error.prepareStackTrace = (e, sst) => { + OriginalCallSite = sst[0].constructor; +}; +new Error().stack; +if (typeof OriginalCallSite === 'function') { + Error.prepareStackTrace = undefined; + + function makeCallSiteGetters(list) { + const callSiteGetters = []; + for (let i=0; i { + return local.Reflect.apply(func, thiz, []); + } + }; + } + return callSiteGetters; + } + + function applyCallSiteGetters(callSite, getters) { + const properties = {__proto__: null}; + for (let i=0; i { + if (host.Array.isArray(sst)) { + for (let i=0; i { - if (hook === 'function' || hook === 'generator_function' || hook === 'eval' || hook === 'run') { - const funcConstructor = internal.Function; + if (hook === 'function' || hook === 'generator_function' || hook === 'eval' || hook === 'run' || + (!checkAsync && (hook === 'async_function' || hook === 'async_generator_function'))) { if (hook === 'eval') { const script = args[0]; args = [script]; @@ -563,28 +563,41 @@ function makeCheckAsync(internal) { // Next line throws on Symbol, this is the same behavior as function constructor calls args = args.map(arg => `${arg}`); } - if (args.findIndex(arg => /\basync\b/.test(arg)) === -1) return args; - const asyncMapped = args.map(arg => arg.replace(/async/g, 'a\\u0073ync')); + const hasAsync = checkAsync && args.findIndex(arg => /\basync\b/.test(arg)) !== -1; + const hasImport = checkImport && args.findIndex(arg => /\bimport\b/.test(arg)) !== -1; + if (!hasAsync && !hasImport) return args; + const mapped = args.map(arg => { + if (hasAsync) arg = arg.replace(/async/g, 'a\\u0073ync'); + if (hasImport) arg = arg.replace(/import/g, 'i\\u006dport'); + return arg; + }); try { - // Note: funcConstructor is a Sandbox object, however, asyncMapped are only strings. - funcConstructor(...asyncMapped); + tryCompile(mapped); } catch (u) { - // u is a sandbox object - // Some random syntax error or error because of async. + // Some random syntax error or error because of async or import. // First report real syntax errors - try { - // Note: funcConstructor is a Sandbox object, however, args are only strings. - funcConstructor(...args); - } catch (e) { - throw internal.Decontextify.value(e); + tryCompile(args); + + if (hasAsync && hasImport) { + const mapped2 = args.map(arg => arg.replace(/async/g, 'a\\u0073ync')); + try { + tryCompile(mapped2); + } catch (e) { + throw new VMError('Async not available'); + } + throw new VMError('Dynamic Import not supported'); + } + if (hasAsync) { + // Then async error + throw new VMError('Async not available'); } - // Then async error - throw new VMError('Async not available'); + throw new VMError('Dynamic Import not supported'); } return args; } - throw new VMError('Async not available'); + if (checkAsync) throw new VMError('Async not available'); + return args; }; } @@ -708,7 +721,7 @@ class VM extends EventEmitter { // Create the bridge between the host and the sandbox. const _internal = CACHE.contextifyScript.runInContext(_context, DEFAULT_RUN_OPTIONS).call(_context, require, HOST); - const hook = fixAsync ? makeCheckAsync(_internal) : null; + const hook = makeCheckHook(fixAsync, true); // Define the properties of this object. // Use Object.defineProperties here to be able to @@ -1172,7 +1185,22 @@ class NodeVM extends VM { let script; if (code instanceof VMScript) { - script = this.options.strict ? code._compileNodeVMStrict() : code._compileNodeVM(); + if (this._hook) { + const prefix = this.options.strict ? STRICT_MODULE_PREFIX : MODULE_PREFIX; + const scriptCode = prefix + code.getCompiledCode() + MODULE_SUFFIX; + const changed = this._hook('run', [scriptCode])[0]; + if (changed === scriptCode) { + script = this.options.strict ? code._compileNodeVMStrict() : code._compileNodeVM(); + } else { + script = new vm.Script(changed, { + filename: code.filename, + displayErrors: false, + importModuleDynamically + }); + } + } else { + script = this.options.strict ? code._compileNodeVMStrict() : code._compileNodeVM(); + } resolvedFilename = pa.resolve(code.filename); dirname = pa.dirname(resolvedFilename); } else { @@ -1185,8 +1213,11 @@ class NodeVM extends VM { dirname = null; } const prefix = this.options.strict ? STRICT_MODULE_PREFIX : MODULE_PREFIX; - script = new vm.Script(prefix + - this._compiler(code, unresolvedFilename) + MODULE_SUFFIX, { + let scriptCode = prefix + this._compiler(code, unresolvedFilename) + MODULE_SUFFIX; + if (this._hook) { + scriptCode = this._hook('run', [scriptCode])[0]; + } + script = new vm.Script(scriptCode, { filename: unresolvedFilename, displayErrors: false, importModuleDynamically diff --git a/lib/sandbox.js b/lib/sandbox.js index 391f707..1a60eb5 100644 --- a/lib/sandbox.js +++ b/lib/sandbox.js @@ -68,8 +68,10 @@ return ((vm, host) => { const code = host.STRICT_MODULE_PREFIX + contents + host.MODULE_SUFFIX; + const ccode = vm._hook('run', [code]); + // Precompile script - script = new Script(code, { + script = new Script(ccode, { __proto__: null, filename: filename || 'vm.js', displayErrors: false, diff --git a/test/vm.js b/test/vm.js index d4915b3..2c4738a 100644 --- a/test/vm.js +++ b/test/vm.js @@ -910,31 +910,23 @@ describe('VM', () => { }); if (NODE_VERSION >= 10) { - it('Dynamic import attack', (done) => { - process.once('unhandledRejection', (reason) => { - assert.strictEqual(reason.message, 'process is not defined'); - done(); - }); + it('Dynamic import attack', () => { const vm2 = new VM(); - vm2.run(` - (async () => { - try { - await import('oops!'); - } catch (ex) { - // ex is an instance of NodeError which is not proxied; - const process = ex.constructor.constructor('return process')(); - const require = process.mainModule.require; - const child_process = require('child_process'); - const output = child_process.execSync('id'); - process.stdout.write(output); - } - })(); - `); + assert.throws(()=>vm2.run(` + const process = import('oops!').constructor.constructor('return process')(); + `), /VMError: Dynamic Import not supported/); }); } + it('Error.prepareStackTrace attack', () => { + const vm2 = new VM(); + const sst = vm2.run('Error.prepareStackTrace = (e,sst)=>sst;const sst = new Error().stack;Error.prepareStackTrace = undefined;sst'); + assert.strictEqual(vm2.run('sst=>Object.getPrototypeOf(sst)')(sst), vm2.run('Array.prototype')); + assert.throws(()=>vm2.run('sst=>sst[0].getThis().constructor.constructor')(sst), /TypeError: Cannot read property 'constructor' of undefined/); + }); + after(() => { vm = null; });