search

hacksparrow / safe-eval

Safer version of eval()
257 stars 37 forks source link

Security issue: node's VM module doesn't prevent you from accessing the node stdlib #5

Closed odino closed 6 years ago

odino commented 7 years ago

As simple as:

safeEval("this.constructor.constructor('return process')().exit()")

This should be mentioned in the readme, as the VM isn't so safe and people might instead think this prevents any kind of attack :)

Ref: https://github.com/patriksimek/vm2/issues/59

phucluke commented 6 years ago

Hi the author,

The library is great. And is there any plan to fix this issue?

hacksparrow commented 6 years ago

Fixed in 0.4.0.

SuperOP535 commented 6 years ago

this page doesn't say so https://www.npmjs.com/advisories/337

odino commented 6 years ago

Hey @SuperOP535 -- original reporter here :) I tried reproducing the issue with:

~ ᐅ cd /tmp 
/tmp ᐅ npm i safe-eval
npm WARN saveError ENOENT: no such file or directory, open '/tmp/package.json'
npm WARN enoent ENOENT: no such file or directory, open '/tmp/package.json'
npm WARN tmp No description
npm WARN tmp No repository field.
npm WARN tmp No README data
npm WARN tmp No license field.

+ safe-eval@0.4.1
added 1 package from 1 contributor and audited 2 packages in 1.854s
found 1 critical severity vulnerability
  run `npm audit fix` to fix them, or `npm audit` for details
/tmp ᐅ node
> safeEval = require('safe-eval')
[Function: safeEval]
> safeEval("this.constructor.constructor('return process')().exit()")
evalmachine.<anonymous>:11
  SAFE_EVAL_357108=this.constructor.constructor('return process')().exit()
                                    ^

TypeError: this.constructor.constructor is not a function
    at evalmachine.<anonymous>:11:37
    at Script.runInContext (vm.js:102:20)
    at Script.runInNewContext (vm.js:108:17)
    at Object.runInNewContext (vm.js:291:38)
    at safeEval (/tmp/node_modules/safe-eval/index.js:24:6)
    at repl:1:1
    at Script.runInThisContext (vm.js:91:20)
    at REPLServer.defaultEval (repl.js:317:29)
    at bound (domain.js:396:14)
    at REPLServer.runBound [as eval] (domain.js:409:12)

and it looks like safe-eval 0.4 fixes it. Not sure how the NPM advisories work, as it should probably be closed.

cpcallen commented 6 years ago

Unfortunately the sanitisation in 7051474 is not sufficient to close the hole. I have sent a proof-of-concept to @hacksparrow by email.

The new exploit (like the one reported by @odino) is fragile, and there are many trivial ways to break it, but in general any access by the eval-ed code to any object from the caller's realm (e.g., if any object is passed in via context) will allow escaping the sandbox.

I would advise using vm2 instead. (Please note that I have not audited the code of vm2, so can make no representations about its correctness—but at least its author appears to fully understand the issues.)