From 0ec08ab0ce2c7b14f7e56865a35766a11d28de5a Mon Sep 17 00:00:00 2001 From: Nathan Rajlich Date: Mon, 12 Jul 2021 13:02:37 -0700 Subject: [PATCH 1/4] Use `vm2` module to prevent privilege escalation of untrusted code --- package.json | 3 ++- src/index.ts | 12 ++++++------ test/test.ts | 21 +++++++++++++++++---- 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/package.json b/package.json index 0d85ada..b583a63 100644 --- a/package.json +++ b/package.json @@ -27,7 +27,8 @@ "dependencies": { "ast-types": "^0.13.2", "escodegen": "^1.8.1", - "esprima": "^4.0.0" + "esprima": "^4.0.0", + "vm2": "^3.9.3" }, "devDependencies": { "@types/escodegen": "^0.0.6", diff --git a/src/index.ts b/src/index.ts index 4f3f68b..a5bc746 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,8 +1,9 @@ -import { isRegExp } from 'util'; +import { types } from 'util'; import { generate } from 'escodegen'; import { parseScript } from 'esprima'; import { visit, namedTypes as n, builders as b } from 'ast-types'; -import { Context, RunningScriptOptions, runInNewContext } from 'vm'; +import { Context, RunningScriptOptions } from 'vm'; +import { VM } from 'vm2'; import _supportsAsync from './supports-async'; import generatorToPromiseFn from './generator-to-promise'; @@ -161,10 +162,9 @@ namespace degenerator { ): T { const output = _supportsAsync ? 'async' : 'generator'; const compiled = degenerator(code, names, { ...options, output }); - const fn = runInNewContext( + const vm = new VM(options); + const fn = vm.run( `${compiled};${returnName}`, - options.sandbox, - options ); if (typeof fn !== 'function') { throw new Error( @@ -230,7 +230,7 @@ function checkName(name: string, names: degenerator.DegeneratorNames): boolean { // now that we have the `name`, check if any entries match in the `names` array for (let i = 0; i < names.length; i++) { const n = names[i]; - if (isRegExp(n)) { + if (types.isRegExp(n)) { if (n.test(name)) { return true; } diff --git a/test/test.ts b/test/test.ts index 49a133a..5b04be8 100644 --- a/test/test.ts +++ b/test/test.ts @@ -101,7 +101,7 @@ describe('degenerator()', () => { }); describe('`compile()`', () => { - it('should compile code into an invocable async function', () => { + it('should compile code into an invocable async function', async () => { const a = (v: string) => Promise.resolve(v); const b = () => 'b'; function aPlusB(v: string): string { @@ -115,9 +115,8 @@ describe('degenerator()', () => { sandbox: { a, b } } ); - return fn('c').then((val: string) => { - assert.equal(val, 'cb'); - }); + const val = await fn('c'); + assert.equal(val, 'cb'); }); it('should contain the compiled code in `toString()` output', () => { const a = () => 'a'; @@ -214,5 +213,19 @@ describe('degenerator()', () => { assert.equal(val, 'foo'); }); }); + it('should not allow privilege escalation of untrusted code', async() => { + let err; + try { + const fn = compile<() => Promise>( + `const f = this.constructor.constructor('return process');`, + 'f', + [], + ); + await fn(); + } catch(_err) { + err = _err; + } + assert.equal(err.message,'process is not defined') + }); }); }); From c8c8b6a1b9072a80cd5e8d3f99ff6a2aa1a7438b Mon Sep 17 00:00:00 2001 From: Nathan Rajlich Date: Mon, 12 Jul 2021 13:03:03 -0700 Subject: [PATCH 2/4] Shorter test name --- test/test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test.ts b/test/test.ts index 5b04be8..21ddf30 100644 --- a/test/test.ts +++ b/test/test.ts @@ -213,7 +213,7 @@ describe('degenerator()', () => { assert.equal(val, 'foo'); }); }); - it('should not allow privilege escalation of untrusted code', async() => { + it('should prevent privilege escalation of untrusted code', async() => { let err; try { const fn = compile<() => Promise>( From e045d70771088aab33adaa6be86b6fe150714be4 Mon Sep 17 00:00:00 2001 From: Nathan Rajlich Date: Mon, 12 Jul 2021 13:04:57 -0700 Subject: [PATCH 3/4] Fix --- src/index.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/index.ts b/src/index.ts index a5bc746..50237e6 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,4 +1,4 @@ -import { types } from 'util'; +import { isRegExp } from 'util'; import { generate } from 'escodegen'; import { parseScript } from 'esprima'; import { visit, namedTypes as n, builders as b } from 'ast-types'; @@ -163,9 +163,7 @@ namespace degenerator { const output = _supportsAsync ? 'async' : 'generator'; const compiled = degenerator(code, names, { ...options, output }); const vm = new VM(options); - const fn = vm.run( - `${compiled};${returnName}`, - ); + const fn = vm.run(`${compiled};${returnName}`); if (typeof fn !== 'function') { throw new Error( `Expected a "function" to be returned for \`${returnName}\`, but got "${typeof fn}"` @@ -230,7 +228,7 @@ function checkName(name: string, names: degenerator.DegeneratorNames): boolean { // now that we have the `name`, check if any entries match in the `names` array for (let i = 0; i < names.length; i++) { const n = names[i]; - if (types.isRegExp(n)) { + if (isRegExp(n)) { if (n.test(name)) { return true; } From 66daad4f0e8ef305a2224a1eeb1408bdf424a6c1 Mon Sep 17 00:00:00 2001 From: Nathan Rajlich Date: Mon, 12 Jul 2021 14:02:26 -0700 Subject: [PATCH 4/4] Fix --- test/test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test.ts b/test/test.ts index fdc0fff..2fb9b87 100644 --- a/test/test.ts +++ b/test/test.ts @@ -162,7 +162,7 @@ describe('degenerator()', () => { it('should prevent privilege escalation of untrusted code', async() => { let err; try { - const fn = compile<() => Promise>( + const fn = compile( `const f = this.constructor.constructor('return process');`, 'f', [],