From ec7fbe82fc7df06bf0ba781cd1531410e274ce94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kau=C3=AA=20Gimenes?= Date: Thu, 15 Nov 2018 18:28:41 -0200 Subject: [PATCH 01/11] make sure context is clear --- index.js | 1 + 1 file changed, 1 insertion(+) diff --git a/index.js b/index.js index bdc5177..29aaae2 100644 --- a/index.js +++ b/index.js @@ -18,6 +18,7 @@ module.exports = function safeEval (code, context, opts) { code = clearContext + resultKey + '=' + code if (context) { Object.keys(context).forEach(function (key) { + if (context[key] === Function) return sandbox[key] = context[key] }) } From fe8a6d31ce84bc06782937adca1ca48b663acaeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kau=C3=AA=20Gimenes?= Date: Thu, 15 Nov 2018 18:30:08 -0200 Subject: [PATCH 02/11] should not have access to Node.js objects --- test/test.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/test.js b/test/test.js index c79027b..edde8b6 100644 --- a/test/test.js +++ b/test/test.js @@ -44,6 +44,16 @@ describe('safe-eval', function () { }) }) + it('should not have access to Node.js objects (@cpcallen report)', function () { + var code = 'test(\'return process\')()' + assert.throws(function () { + safeEval(code, { + // eslint-disable-next-line no-new-func + test: new Function().constructor + }) + }) + }) + it('should not have access to Node.js objects (CWE-265)', function () { var code = 'this.constructor.constructor(\'return process\')()' assert.throws(function () { From 00bbe3c96665b3c86ba85d98e93e4217e7fd33d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kau=C3=AA=20Gimenes?= Date: Thu, 15 Nov 2018 18:36:38 -0200 Subject: [PATCH 03/11] lint --- test/test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test.js b/test/test.js index edde8b6..816426c 100644 --- a/test/test.js +++ b/test/test.js @@ -53,7 +53,7 @@ describe('safe-eval', function () { }) }) }) - + it('should not have access to Node.js objects (CWE-265)', function () { var code = 'this.constructor.constructor(\'return process\')()' assert.throws(function () { From 39d8ef6f3240d693a14e7c88f659c856aa2dc96c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kau=C3=AA=20Gimenes?= Date: Thu, 15 Nov 2018 18:39:31 -0200 Subject: [PATCH 04/11] 0.4.2 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 55327b4..aaacf93 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "safe-eval", - "version": "0.4.1", + "version": "0.4.2", "description": "Safer version of eval()", "main": "index.js", "scripts": { From 8f6233a76c951f31385cc8c1ef16df7132d2ff04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kau=C3=AA=20Gimenes?= Date: Thu, 15 Nov 2018 19:39:23 -0200 Subject: [PATCH 05/11] should not have access to Node.js objects using Object.getPrototypeOf (CWE-265) --- test/test.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/test/test.js b/test/test.js index 816426c..976dfaf 100644 --- a/test/test.js +++ b/test/test.js @@ -44,7 +44,7 @@ describe('safe-eval', function () { }) }) - it('should not have access to Node.js objects (@cpcallen report)', function () { + it('should not have access to Node.js objects using context (CWE-265)', function () { var code = 'test(\'return process\')()' assert.throws(function () { safeEval(code, { @@ -54,7 +54,14 @@ describe('safe-eval', function () { }) }) - it('should not have access to Node.js objects (CWE-265)', function () { + it('should not have access to Node.js objects using Object.getPrototypeOf (CWE-265)', function () { + var code = `Object.getPrototypeOf(Object).constructor('return process')();` + assert.throws(function () { + safeEval(code) + }) + }) + + it('should not have access to Node.js objects using this.constructor (CWE-265)', function () { var code = 'this.constructor.constructor(\'return process\')()' assert.throws(function () { safeEval(code) From fb2c12d1a5b5d9cf0974aed44f95752a9ecee420 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kau=C3=AA=20Gimenes?= Date: Thu, 15 Nov 2018 21:54:06 -0200 Subject: [PATCH 06/11] should not have access to Node.js objects using Object.getPrototypeOf with context (CWE-265) --- test/test.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/test.js b/test/test.js index 976dfaf..fcdbf2a 100644 --- a/test/test.js +++ b/test/test.js @@ -61,6 +61,15 @@ describe('safe-eval', function () { }) }) + it('should not have access to Node.js objects using Object.getPrototypeOf with context (CWE-265)', function () { + var code = `Object.getPrototypeOf(obj).constructor.constructor("return process")();` + assert.throws(function () { + safeEval(code, { + obj: Object + }) + }) + }) + it('should not have access to Node.js objects using this.constructor (CWE-265)', function () { var code = 'this.constructor.constructor(\'return process\')()' assert.throws(function () { From b7ef089137f697a51810686676fdf3eef008c93f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kau=C3=AA=20Gimenes?= Date: Thu, 15 Nov 2018 21:55:25 -0200 Subject: [PATCH 07/11] should check prototype also --- index.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/index.js b/index.js index 29aaae2..5575a57 100644 --- a/index.js +++ b/index.js @@ -10,8 +10,11 @@ module.exports = function safeEval (code, context, opts) { const keys = Object.getOwnPropertyNames(this).concat(['constructor']); keys.forEach((key) => { const item = this[key]; - if (!item || typeof item.constructor !== 'function') return; - this[key].constructor = undefined; + if (!item) return; + if (typeof Object.getPrototypeOf(item).constructor === 'function') + Object.getPrototypeOf(item).constructor = undefined; + if (typeof item.constructor === 'function') + this[key].constructor = undefined; }); })(); ` From d23c1fe99b53136ad61bf0e2287685bb1990f3f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kau=C3=AA=20Gimenes?= Date: Thu, 15 Nov 2018 21:57:52 -0200 Subject: [PATCH 08/11] lint --- test/test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test.js b/test/test.js index fcdbf2a..c036f67 100644 --- a/test/test.js +++ b/test/test.js @@ -69,7 +69,7 @@ describe('safe-eval', function () { }) }) }) - + it('should not have access to Node.js objects using this.constructor (CWE-265)', function () { var code = 'this.constructor.constructor(\'return process\')()' assert.throws(function () { From 399293f863c8d50af32866cb97f47bb2957daf2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kau=C3=AA=20Gimenes?= Date: Thu, 15 Nov 2018 22:01:25 -0200 Subject: [PATCH 09/11] lint --- test/test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test.js b/test/test.js index c036f67..43670da 100644 --- a/test/test.js +++ b/test/test.js @@ -69,7 +69,7 @@ describe('safe-eval', function () { }) }) }) - + it('should not have access to Node.js objects using this.constructor (CWE-265)', function () { var code = 'this.constructor.constructor(\'return process\')()' assert.throws(function () { From 40a159f997f9cf5caa3b41d383377ea5a885ad35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kau=C3=AA=20Gimenes?= Date: Fri, 16 Nov 2018 10:10:43 -0200 Subject: [PATCH 10/11] stop using template string for clearContext function --- index.js | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/index.js b/index.js index 5575a57..f384b3c 100644 --- a/index.js +++ b/index.js @@ -1,24 +1,27 @@ var vm = require('vm') +function clearContext () { + // eslint-disable-next-line no-global-assign + Function = undefined + const keys = Object.getOwnPropertyNames(this).concat(['constructor']) + keys.forEach((key) => { + const item = this[key] + if (!item) return + if (typeof Object.getPrototypeOf(item).constructor === 'function') { + Object.getPrototypeOf(item).constructor = undefined + } + if (typeof item.constructor === 'function') { + this[key].constructor = undefined + } + }) +} + module.exports = function safeEval (code, context, opts) { var sandbox = {} var resultKey = 'SAFE_EVAL_' + Math.floor(Math.random() * 1000000) sandbox[resultKey] = {} - var clearContext = ` - (function() { - Function = undefined; - const keys = Object.getOwnPropertyNames(this).concat(['constructor']); - keys.forEach((key) => { - const item = this[key]; - if (!item) return; - if (typeof Object.getPrototypeOf(item).constructor === 'function') - Object.getPrototypeOf(item).constructor = undefined; - if (typeof item.constructor === 'function') - this[key].constructor = undefined; - }); - })(); - ` - code = clearContext + resultKey + '=' + code + var clearContextCall = `(${clearContext.toString()})();` + code = `${clearContextCall}${resultKey}=${code}` if (context) { Object.keys(context).forEach(function (key) { if (context[key] === Function) return From e34bc167be6add7b48f12b41003e0e914f8a8ac2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kaue=CC=82=20Gimenes?= Date: Sun, 2 Dec 2018 13:55:37 -0200 Subject: [PATCH 11/11] CVE-2017-16088 make sure context is clear should not have access to Node.js objects lint 0.4.2 should not have access to Node.js objects using Object.getPrototypeOf (CWE-265) should not have access to Node.js objects using Object.getPrototypeOf with context (CWE-265) should check prototype also lint lint stop using template string for clearContext function --- index.js | 31 +++++++++++++++++++------------ package.json | 2 +- test/test.js | 28 +++++++++++++++++++++++++++- 3 files changed, 47 insertions(+), 14 deletions(-) diff --git a/index.js b/index.js index bdc5177..f384b3c 100644 --- a/index.js +++ b/index.js @@ -1,23 +1,30 @@ var vm = require('vm') +function clearContext () { + // eslint-disable-next-line no-global-assign + Function = undefined + const keys = Object.getOwnPropertyNames(this).concat(['constructor']) + keys.forEach((key) => { + const item = this[key] + if (!item) return + if (typeof Object.getPrototypeOf(item).constructor === 'function') { + Object.getPrototypeOf(item).constructor = undefined + } + if (typeof item.constructor === 'function') { + this[key].constructor = undefined + } + }) +} + module.exports = function safeEval (code, context, opts) { var sandbox = {} var resultKey = 'SAFE_EVAL_' + Math.floor(Math.random() * 1000000) sandbox[resultKey] = {} - var clearContext = ` - (function() { - Function = undefined; - const keys = Object.getOwnPropertyNames(this).concat(['constructor']); - keys.forEach((key) => { - const item = this[key]; - if (!item || typeof item.constructor !== 'function') return; - this[key].constructor = undefined; - }); - })(); - ` - code = clearContext + resultKey + '=' + code + var clearContextCall = `(${clearContext.toString()})();` + code = `${clearContextCall}${resultKey}=${code}` if (context) { Object.keys(context).forEach(function (key) { + if (context[key] === Function) return sandbox[key] = context[key] }) } diff --git a/package.json b/package.json index 55327b4..aaacf93 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "safe-eval", - "version": "0.4.1", + "version": "0.4.2", "description": "Safer version of eval()", "main": "index.js", "scripts": { diff --git a/test/test.js b/test/test.js index c79027b..43670da 100644 --- a/test/test.js +++ b/test/test.js @@ -44,7 +44,33 @@ describe('safe-eval', function () { }) }) - it('should not have access to Node.js objects (CWE-265)', function () { + it('should not have access to Node.js objects using context (CWE-265)', function () { + var code = 'test(\'return process\')()' + assert.throws(function () { + safeEval(code, { + // eslint-disable-next-line no-new-func + test: new Function().constructor + }) + }) + }) + + it('should not have access to Node.js objects using Object.getPrototypeOf (CWE-265)', function () { + var code = `Object.getPrototypeOf(Object).constructor('return process')();` + assert.throws(function () { + safeEval(code) + }) + }) + + it('should not have access to Node.js objects using Object.getPrototypeOf with context (CWE-265)', function () { + var code = `Object.getPrototypeOf(obj).constructor.constructor("return process")();` + assert.throws(function () { + safeEval(code, { + obj: Object + }) + }) + }) + + it('should not have access to Node.js objects using this.constructor (CWE-265)', function () { var code = 'this.constructor.constructor(\'return process\')()' assert.throws(function () { safeEval(code)