Skip to content

Commit

Permalink
Merge pull request #1 from bbeale/master
Browse files Browse the repository at this point in the history
Mitigation for arbitrary code execution / sandbox breakout
  • Loading branch information
JamieSlome committed Aug 3, 2020
2 parents d79adcf + f4368e9 commit f7b56d6
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 4 deletions.
2 changes: 1 addition & 1 deletion src/common.js
Expand Up @@ -52,7 +52,7 @@ exports.createContext = function () {
context.console.constructor.constructor = FN_NOOP
}
if (hasWindow) {
fillContext(window, true)
fillContext(window)
cloneFunctions(context)
protectBuiltInObjects(context)
context.console = clones(console, console) // console needs special treatment
Expand Down
12 changes: 9 additions & 3 deletions src/index.js
Expand Up @@ -34,7 +34,7 @@ class SaferEval {

/**
* @param {String} code - a string containing javascript code
* @return {Any} evaluated code
* @return {String} evaluated code
*/
runInContext (code) {
if (typeof code !== 'string') {
Expand All @@ -45,7 +45,13 @@ class SaferEval {
src += 'return ' + code + ';\n'
src += '})()'

return vm.runInContext(src, this._context, this._options)
const dangerous = src.includes('require(\'child_process\')') || src.includes('require("child_process")') || src.includes('return process')

if (dangerous) {
console.log('potentially unsafe template literal')
} else {
return vm.runInContext(src, this._context, this._options)
}
}
}

Expand All @@ -62,7 +68,7 @@ class SaferEval {
* @throws Error
* @param {String} code - a string containing javascript code
* @param {Object} [context] - define globals, properties for evaluation context
* @return {Any} evaluated code
* @return {String} evaluated code
* @example
* var code = `{d: new Date('1970-01-01'), b: new Buffer('data')}`
* var res = saferEval(code, {Buffer: Buffer})
Expand Down
65 changes: 65 additions & 0 deletions test/saferEval.spec.js
Expand Up @@ -370,6 +370,71 @@ describe('#saferEval', function () {
})

describe('harmful context', function () {
// security mitigation: https://github.com/commenthol/safer-eval/issues/10
it('tries to breakout', function () {
const maliciousFunction = function () {
const f = Buffer.prototype.write
const ft = {
length: 10,
utf8Write () {

}
}
function r (i) {
var x = 0
try {
x = r(i)
} catch (e) {}
if (typeof (x) !== 'number') { return x }
if (x !== i) { return x + 1 }
try {
f.call(ft)
} catch (e) {
return e
}
return null
}
var i = 1
while (1) {
try {
i = r(i).constructor.constructor('return process')()
break
} catch (x) {
i++
}
}
return i.mainModule.require('child_process').execSync('id').toString()
}

const evil = `(${maliciousFunction})()`
const res = saferEval(evil)
assert.strictEqual(res, undefined)
})

it('tries to breakout another way', function () {
const anotherMaliciousFunction = function () {
const process = clearImmediate.constructor('return process;')()
return process.mainModule.require('child_process').execSync('whoami').toString()
}
const evil = `(${anotherMaliciousFunction})()`
const res = saferEval(evil)
assert.strictEqual(res, undefined)
})

it('tries to break out yet another way using setInterval', function () {
const code = "setInterval.constructor('return" +
" process')().mainModule.require('child_process').execSync('whoami').toString();"
const res = saferEval(code)
assert.strictEqual(res, undefined)
})

it('tries to break out yet another way using setInterval', function () {
const code = "Buffer.of.constructor('return" +
" process')().mainModule.require('child_process').execSync('whoami').toString();"
const res = saferEval(code)
assert.strictEqual(res, undefined)
})

describeNode('in node', function () {
it('evaluates global.eval if passing global as context - which is a bad idea', function () {
var res = saferEval('global.eval(9 + 25)', { global: global }) // !!! try to avoid passing global as context this way
Expand Down

0 comments on commit f7b56d6

Please sign in to comment.