Skip to content

Commit 1a87237

Browse files
committed
fix: use strict mode recommendation
1 parent b81dab9 commit 1a87237

File tree

5 files changed

+99
-26
lines changed

5 files changed

+99
-26
lines changed

README.md

+49-15
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,39 @@ npm install --save safer-eval
4141

4242
## Implementation recommendations
4343

44-
Be aware that a `saferEval('function(){while(true){}}()')` may run
44+
**Use strict mode**
45+
46+
Always use `'use strict'` mode in functions/ files calling `saferEval()`.
47+
Otherwise a sandbox breakout may be possible.
48+
49+
```js
50+
51+
'use strict'
52+
const saferEval = require('safer-eval')
53+
54+
function main () {
55+
'use strict' //< alternative within function
56+
const res = saferEval('new Date()')
57+
...
58+
}
59+
60+
```
61+
62+
**Run in worker**
63+
64+
Be aware that a
65+
66+
```js
67+
saferEval('(function () { while (true) {} })()')
68+
```
69+
70+
may run
4571
infinitely. Consider using the module from within a worker thread which is terminated
4672
after timeout.
4773

48-
Avoid passing context props while deserializing data from hostile environments.
74+
**Avoid context props**
75+
76+
Avoid passing `context` props while deserializing data from hostile environments.
4977

5078
## Usage
5179

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

6896
```js
69-
var saferEval = require('safer-eval')
70-
var code = `{d: new Date('1970-01-01'), b: new Buffer('data')}`
71-
var res = saferEval(code)
97+
'use strict' //< NEVER FORGET TO ADD STRICT MODE in file/ function
98+
//< running `saferEval`
99+
const saferEval = require('safer-eval')
100+
const code = `{d: new Date('1970-01-01'), b: new Buffer('data')}`
101+
const res = saferEval(code)
72102
// => toString.call(res.d) = '[object Date]'
73103
// => toString.call(res.b) = '[object Buffer]'
74104
```
75105

76106
in browser:
77107

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

89121
```js
90-
var code = `window.btoa('Hello, world')`
122+
const code = `window.btoa('Hello, world')`
91123

92124
// AVOID passing a GLOBAL context!!!
93-
var res = saferEval(code, {window: window})
125+
const res = saferEval(code, {window: window})
94126

95127
// BETTER - code needs only access to window.btoa
96128
const clones = require('clones')
97-
var context = {
129+
const context = {
98130
window: {
99131
btoa: clones(window.btoa, window)
100132
}
101133
}
102-
var res = saferEval(code ,context)
134+
const res = saferEval(code ,context)
103135
// => res = 'SGVsbG8sIHdvcmxk'
104136
```
105137

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

110142
```js
111-
const {SaferEval} = require('safer-eval')
143+
'use strict' //< NEVER FORGET TO ADD STRICT MODE in file/ function
144+
//< running `saferEval`
145+
const { SaferEval } = require('safer-eval')
112146
const safer = new SaferEval()
113-
var code = `{d: new Date('1970-01-01'), b: new Buffer('data')}`
114-
var res = safer.runInContext(code)
147+
const code = `{d: new Date('1970-01-01'), b: new Buffer('data')}`
148+
const res = safer.runInContext(code)
115149
```
116150

117151
## License

package.json

+6-2
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,17 @@
2323
"test": "test"
2424
},
2525
"scripts": {
26-
"all": "npm run clean && npm run lint && npm run transpile && npm test",
26+
"preall": "npm run clean",
27+
"all": "npm test",
2728
"clean": "rimraf lib",
2829
"coverage": "nyc -r html -r text npm test",
30+
"prekarma": "npm run transpile",
2931
"karma": "karma start",
30-
"lint": "eslint --fix \"**/*.js\"",
32+
"lint": "eslint --fix src test *.js",
3133
"prepublishOnly": "npm run all",
34+
"pretest": "npm run transpile",
3235
"test": "mocha",
36+
"posttest": "npm run lint",
3337
"transpile": "babel -d lib src",
3438
"zuul": "zuul --no-coverage --local 3000 -- test/*.js"
3539
},

src/common.js

+4-3
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ exports.hasWindow = hasWindow
88
const hasGlobal = (typeof global !== 'undefined')
99
exports.hasGlobal = hasGlobal
1010

11+
const FN_NOOP = 'function () {}'
12+
1113
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)$/
1214

1315
const isIdentifier = key => !NON_IDENTIFIER.test(key)
@@ -47,15 +49,15 @@ exports.createContext = function () {
4749
cloneFunctions(context)
4850
context.Buffer = _protect('Buffer')
4951
context.console = clones(console, console) // console needs special treatment
50-
context.console.constructor.constructor = 'function () {}'
52+
context.console.constructor.constructor = FN_NOOP
5153
}
5254
if (hasWindow) {
5355
fillContext(window, true)
5456
cloneFunctions(context)
5557
protectBuiltInObjects(context)
5658
context.console = clones(console, console) // console needs special treatment
5759
try {
58-
context.Object.constructor.constructor = 'function () {}'
60+
context.Object.constructor.constructor = FN_NOOP
5961
} catch (e) {
6062
}
6163
}
@@ -123,7 +125,6 @@ function cloneFunctions (context) {
123125
function protectBuiltInObjects (context) {
124126
;[
125127
'Object',
126-
// 'Object.constructor.constructor',
127128
'Boolean',
128129
'Symbol',
129130
'Error',

src/index.js

+5-6
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,12 @@ class SaferEval {
4040
if (typeof code !== 'string') {
4141
throw new TypeError('not a string')
4242
}
43-
let src = 'Object.constructor = function () {};\n'
43+
let src = '(function () {"use strict";\n'
44+
src += 'Object.constructor = function () {};\n'
4445
src += 'return ' + code + ';\n'
46+
src += '})()'
4547

46-
return vm.runInContext(
47-
'(function () {"use strict"; ' + src + '})()',
48-
this._context,
49-
this._options
50-
)
48+
return vm.runInContext(src, this._context, this._options)
5149
}
5250
}
5351

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

test/saferEval.spec.js

+35
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
/* eslint no-new-func:0 */
22

3+
'use strict'
4+
35
var assert = require('assert')
46
var clones = require('clones')
57
var saferEval = require('..')
@@ -295,6 +297,23 @@ describe('#saferEval', function () {
295297
}
296298
assert.strictEqual(res, undefined)
297299
})
300+
it('should prevent a breakout using function.caller - NEEDS "use strict" being set', function () {
301+
'use strict'
302+
303+
let res
304+
try {
305+
const stmt = `(() => {
306+
function f() {
307+
return f.caller.constructor('return global')();
308+
}
309+
return f.constructor('return ' + f)();
310+
})();
311+
`
312+
res = saferEval(stmt)()
313+
} catch (e) {
314+
}
315+
assert.strictEqual(res, undefined)
316+
})
298317
})
299318

300319
describeBrowser('in browser', function () {
@@ -331,6 +350,22 @@ describe('#saferEval', function () {
331350
}
332351
assert.strictEqual(res, undefined)
333352
})
353+
it('should prevent a breakout using function.caller - NEEDS "use strict" being set', function () {
354+
'use strict'
355+
356+
let res
357+
try {
358+
const stmt = `(() => {
359+
function f() {
360+
return f.caller.constructor('return window')();
361+
}
362+
return f.constructor('return ' + f)();
363+
})()`
364+
res = saferEval(stmt)()
365+
} catch (e) {
366+
}
367+
assert.strictEqual(res, undefined)
368+
})
334369
})
335370
})
336371

0 commit comments

Comments
 (0)