Skip to content

Commit

Permalink
Merge pull request #7 from commenthol/strict-mode-recommendation
Browse files Browse the repository at this point in the history
fix: use strict mode recommendation
  • Loading branch information
commenthol authored Jul 14, 2019
2 parents b81dab9 + 1a87237 commit fbbc623
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 26 deletions.
64 changes: 49 additions & 15 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,39 @@ npm install --save safer-eval

## Implementation recommendations

Be aware that a `saferEval('function(){while(true){}}()')` may run
**Use strict mode**

Always use `'use strict'` mode in functions/ files calling `saferEval()`.
Otherwise a sandbox breakout may be possible.

```js

'use strict'
const saferEval = require('safer-eval')

function main () {
'use strict' //< alternative within function
const res = saferEval('new Date()')
...
}

```

**Run in worker**

Be aware that a

```js
saferEval('(function () { while (true) {} })()')
```

may run
infinitely. Consider using the module from within a worker thread which is terminated
after timeout.

Avoid passing context props while deserializing data from hostile environments.
**Avoid context props**

Avoid passing `context` props while deserializing data from hostile environments.

## Usage

Expand All @@ -66,19 +94,23 @@ Check the tests under "harmful context"!
in node:

```js
var saferEval = require('safer-eval')
var code = `{d: new Date('1970-01-01'), b: new Buffer('data')}`
var res = saferEval(code)
'use strict' //< NEVER FORGET TO ADD STRICT MODE in file/ function
//< running `saferEval`
const saferEval = require('safer-eval')
const code = `{d: new Date('1970-01-01'), b: new Buffer('data')}`
const res = saferEval(code)
// => toString.call(res.d) = '[object Date]'
// => toString.call(res.b) = '[object Buffer]'
```

in browser:

```js
var saferEval = require('safer-eval')
var code = `{d: new Date('1970-01-01'), b: function () { return navigator.userAgent }`
var res = saferEval(code, {navigator: window.navigator})
'use strict' //< NEVER FORGET TO ADD STRICT MODE in file/ function
//< running `saferEval`
const saferEval = require('safer-eval')
const code = `{d: new Date('1970-01-01'), b: function () { return navigator.userAgent }`
const res = saferEval(code, {navigator: window.navigator})
// => toString.call(res.d) = '[object Date]'
// => toString.call(res.b) = '[object Function]'
// => res.b() = "Mozilla/5.0 (..."
Expand All @@ -87,19 +119,19 @@ var res = saferEval(code, {navigator: window.navigator})
To minimize any harmful code injection carefully select the methods you allow in `context`

```js
var code = `window.btoa('Hello, world')`
const code = `window.btoa('Hello, world')`

// AVOID passing a GLOBAL context!!!
var res = saferEval(code, {window: window})
const res = saferEval(code, {window: window})

// BETTER - code needs only access to window.btoa
const clones = require('clones')
var context = {
const context = {
window: {
btoa: clones(window.btoa, window)
}
}
var res = saferEval(code ,context)
const res = saferEval(code ,context)
// => res = 'SGVsbG8sIHdvcmxk'
```

Expand All @@ -108,10 +140,12 @@ var res = saferEval(code ,context)
Use `new SaferEval()` to reuse a once created context.

```js
const {SaferEval} = require('safer-eval')
'use strict' //< NEVER FORGET TO ADD STRICT MODE in file/ function
//< running `saferEval`
const { SaferEval } = require('safer-eval')
const safer = new SaferEval()
var code = `{d: new Date('1970-01-01'), b: new Buffer('data')}`
var res = safer.runInContext(code)
const code = `{d: new Date('1970-01-01'), b: new Buffer('data')}`
const res = safer.runInContext(code)
```

## License
Expand Down
8 changes: 6 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,17 @@
"test": "test"
},
"scripts": {
"all": "npm run clean && npm run lint && npm run transpile && npm test",
"preall": "npm run clean",
"all": "npm test",
"clean": "rimraf lib",
"coverage": "nyc -r html -r text npm test",
"prekarma": "npm run transpile",
"karma": "karma start",
"lint": "eslint --fix \"**/*.js\"",
"lint": "eslint --fix src test *.js",
"prepublishOnly": "npm run all",
"pretest": "npm run transpile",
"test": "mocha",
"posttest": "npm run lint",
"transpile": "babel -d lib src",
"zuul": "zuul --no-coverage --local 3000 -- test/*.js"
},
Expand Down
7 changes: 4 additions & 3 deletions src/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ exports.hasWindow = hasWindow
const hasGlobal = (typeof global !== 'undefined')
exports.hasGlobal = hasGlobal

const FN_NOOP = 'function () {}'

const NON_IDENTIFIER = /^\d|-|^(break|case|catch|continue|debugger|default|delete|do|else|finally|for|function|if|in|instanceof|new|return|switch|this|throw|try|typeof|var|void|while|with|class|const|enum|export|extends|import|super|implements|interface|let|package|private|protected|public|static|yield|null|true|false)$/

const isIdentifier = key => !NON_IDENTIFIER.test(key)
Expand Down Expand Up @@ -47,15 +49,15 @@ exports.createContext = function () {
cloneFunctions(context)
context.Buffer = _protect('Buffer')
context.console = clones(console, console) // console needs special treatment
context.console.constructor.constructor = 'function () {}'
context.console.constructor.constructor = FN_NOOP
}
if (hasWindow) {
fillContext(window, true)
cloneFunctions(context)
protectBuiltInObjects(context)
context.console = clones(console, console) // console needs special treatment
try {
context.Object.constructor.constructor = 'function () {}'
context.Object.constructor.constructor = FN_NOOP
} catch (e) {
}
}
Expand Down Expand Up @@ -123,7 +125,6 @@ function cloneFunctions (context) {
function protectBuiltInObjects (context) {
;[
'Object',
// 'Object.constructor.constructor',
'Boolean',
'Symbol',
'Error',
Expand Down
11 changes: 5 additions & 6 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,12 @@ class SaferEval {
if (typeof code !== 'string') {
throw new TypeError('not a string')
}
let src = 'Object.constructor = function () {};\n'
let src = '(function () {"use strict";\n'
src += 'Object.constructor = function () {};\n'
src += 'return ' + code + ';\n'
src += '})()'

return vm.runInContext(
'(function () {"use strict"; ' + src + '})()',
this._context,
this._options
)
return vm.runInContext(src, this._context, this._options)
}
}

Expand All @@ -72,6 +70,7 @@ class SaferEval {
* // => toString.call(res.b) = '[object Buffer]'
*/
function saferEval (code, context) {
'use strict'
return new SaferEval(context).runInContext(code)
}

Expand Down
35 changes: 35 additions & 0 deletions test/saferEval.spec.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
/* eslint no-new-func:0 */

'use strict'

var assert = require('assert')
var clones = require('clones')
var saferEval = require('..')
Expand Down Expand Up @@ -295,6 +297,23 @@ describe('#saferEval', function () {
}
assert.strictEqual(res, undefined)
})
it('should prevent a breakout using function.caller - NEEDS "use strict" being set', function () {
'use strict'

let res
try {
const stmt = `(() => {
function f() {
return f.caller.constructor('return global')();
}
return f.constructor('return ' + f)();
})();
`
res = saferEval(stmt)()
} catch (e) {
}
assert.strictEqual(res, undefined)
})
})

describeBrowser('in browser', function () {
Expand Down Expand Up @@ -331,6 +350,22 @@ describe('#saferEval', function () {
}
assert.strictEqual(res, undefined)
})
it('should prevent a breakout using function.caller - NEEDS "use strict" being set', function () {
'use strict'

let res
try {
const stmt = `(() => {
function f() {
return f.caller.constructor('return window')();
}
return f.constructor('return ' + f)();
})()`
res = saferEval(stmt)()
} catch (e) {
}
assert.strictEqual(res, undefined)
})
})
})

Expand Down

0 comments on commit fbbc623

Please sign in to comment.