From 1a8723704b30ac43c3c300223c6132e27b48fc21 Mon Sep 17 00:00:00 2001 From: commenthol Date: Sun, 14 Jul 2019 12:36:19 +0200 Subject: [PATCH] fix: use strict mode recommendation --- README.md | 64 ++++++++++++++++++++++++++++++++---------- package.json | 8 ++++-- src/common.js | 7 +++-- src/index.js | 11 ++++---- test/saferEval.spec.js | 35 +++++++++++++++++++++++ 5 files changed, 99 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index 7ac12c9..3b39c1c 100644 --- a/README.md +++ b/README.md @@ -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 @@ -66,9 +94,11 @@ 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]' ``` @@ -76,9 +106,11 @@ var res = saferEval(code) 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 (..." @@ -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' ``` @@ -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 diff --git a/package.json b/package.json index 35dc49a..ef13bf2 100644 --- a/package.json +++ b/package.json @@ -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" }, diff --git a/src/common.js b/src/common.js index 998e2ba..b4065ba 100644 --- a/src/common.js +++ b/src/common.js @@ -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) @@ -47,7 +49,7 @@ 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) @@ -55,7 +57,7 @@ exports.createContext = function () { 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) { } } @@ -123,7 +125,6 @@ function cloneFunctions (context) { function protectBuiltInObjects (context) { ;[ 'Object', - // 'Object.constructor.constructor', 'Boolean', 'Symbol', 'Error', diff --git a/src/index.js b/src/index.js index 99379c8..8846eee 100644 --- a/src/index.js +++ b/src/index.js @@ -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) } } @@ -72,6 +70,7 @@ class SaferEval { * // => toString.call(res.b) = '[object Buffer]' */ function saferEval (code, context) { + 'use strict' return new SaferEval(context).runInContext(code) } diff --git a/test/saferEval.spec.js b/test/saferEval.spec.js index 22ebd8a..d29597a 100644 --- a/test/saferEval.spec.js +++ b/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('..') @@ -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 () { @@ -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) + }) }) })