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

fix: use strict mode recommendation #7

Merged
merged 1 commit into from Jul 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
64 changes: 49 additions & 15 deletions README.md
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
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
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
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
@@ -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