From b0c80ab8d8cb6fc9b48f3605f6f240f7e94fc670 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Thu, 24 Oct 2019 16:41:09 +0200 Subject: [PATCH 1/4] disallow accessing __proto__ or constructor Plugs another hole in the not-quite-sandbox. We'll never be 100% airtight but this is low hanging fruit that doesn't impact any legitimate use case. --- index.js | 6 ++++++ test/eval.js | 28 +++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index ea16186..13860f3 100644 --- a/index.js +++ b/index.js @@ -102,10 +102,12 @@ module.exports = function (ast, vars) { return FAIL; } if (node.property.type === 'Identifier') { + if (isUnsafeProperty(node.property.name)) return FAIL; return obj[node.property.name]; } var prop = walk(node.property); if (prop === FAIL) return FAIL; + if (isUnsafeProperty(prop)) return FAIL; return obj[prop]; } else if (node.type === 'ConditionalExpression') { @@ -176,3 +178,7 @@ module.exports = function (ast, vars) { return result === FAIL ? undefined : result; }; + +function isUnsafeProperty(name) { + return name === 'constructor' || name === '__proto__'; +} diff --git a/test/eval.js b/test/eval.js index c8d3d25..6dde4c4 100644 --- a/test/eval.js +++ b/test/eval.js @@ -79,4 +79,30 @@ test('MemberExpressions from Functions unresolved', function(t) { var ast = parse(src).body[0].expression; var res = evaluate(ast, {}); t.equal(res, undefined); -}); \ No newline at end of file +}); + +test('disallow accessing constructor or __proto__', function (t) { + t.plan(4) + + var someValue = {}; + + var src = 'object.constructor'; + var ast = parse(src).body[0].expression; + var res = evaluate(ast, { vars: { object: someValue } }); + t.equal(res, undefined); + + var src = 'object["constructor"]'; + var ast = parse(src).body[0].expression; + var res = evaluate(ast, { vars: { object: someValue } }); + t.equal(res, undefined); + + var src = 'object.__proto__'; + var ast = parse(src).body[0].expression; + var res = evaluate(ast, { vars: { object: someValue } }); + t.equal(res, undefined); + + var src = 'object["__pro"+"t\x6f__"]'; + var ast = parse(src).body[0].expression; + var res = evaluate(ast, { vars: { object: someValue } }); + t.equal(res, undefined); +}); From 551469918259875c5591e5110f7a91781dd30680 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Thu, 24 Oct 2019 17:03:22 +0200 Subject: [PATCH 2/4] Bail on `null` properties (eg. uninitialized arguments) --- index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/index.js b/index.js index 13860f3..7620e5b 100644 --- a/index.js +++ b/index.js @@ -101,12 +101,12 @@ module.exports = function (ast, vars) { if((obj === FAIL) || (typeof obj == 'function')){ return FAIL; } - if (node.property.type === 'Identifier') { + if (node.property.type === 'Identifier' && !node.computed) { if (isUnsafeProperty(node.property.name)) return FAIL; return obj[node.property.name]; } var prop = walk(node.property); - if (prop === FAIL) return FAIL; + if (prop === null || prop === FAIL) return FAIL; if (isUnsafeProperty(prop)) return FAIL; return obj[prop]; } From 6b7b9609d770948ec9e8a8dedeee5e55891459a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Thu, 24 Oct 2019 17:05:39 +0200 Subject: [PATCH 3/4] add runtime-only test case --- test/eval.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/eval.js b/test/eval.js index 6dde4c4..3a0c034 100644 --- a/test/eval.js +++ b/test/eval.js @@ -106,3 +106,18 @@ test('disallow accessing constructor or __proto__', function (t) { var res = evaluate(ast, { vars: { object: someValue } }); t.equal(res, undefined); }); + + +test('constructor at runtime only', function(t) { + t.plan(1) + + var src = '(function myTag(y){return ""[!y?"__proto__":"constructor"][y]})("constructor")("console.log(process.env)")()' + var ast = parse(src).body[0].expression; + var res = evaluate(ast); + t.equal(res, undefined); + + var src = '(function(prop) { return {}[prop ? "benign" : "constructor"][prop] })("constructor")("alert(1)")()' + var ast = parse(src).body[0].expression; + var res = evaluate(ast); + t.equal(res, undefined); +}); From a18a308120ac7d5bc974292a8eefb3dfc0649f61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Wed, 30 Oct 2019 16:12:04 +0100 Subject: [PATCH 4/4] plan fix --- test/eval.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/eval.js b/test/eval.js index 3a0c034..4055b35 100644 --- a/test/eval.js +++ b/test/eval.js @@ -109,7 +109,7 @@ test('disallow accessing constructor or __proto__', function (t) { test('constructor at runtime only', function(t) { - t.plan(1) + t.plan(2) var src = '(function myTag(y){return ""[!y?"__proto__":"constructor"][y]})("constructor")("console.log(process.env)")()' var ast = parse(src).body[0].expression;