Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CVE-2017-16088] Sandbox Breakout (Critical Security Fix) - context clear #13

Merged
merged 12 commits into from
Dec 14, 2018
31 changes: 19 additions & 12 deletions index.js
Original file line number Diff line number Diff line change
@@ -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]
})
}
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "safe-eval",
"version": "0.4.1",
"version": "0.4.2",
"description": "Safer version of eval()",
"main": "index.js",
"scripts": {
Expand Down
28 changes: 27 additions & 1 deletion test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down