Closed XmiliaH closed 4 years ago
Confirmed. Yikes. Excellent work, @XmiliaH; thanks for bringing this to our attention.
Whoa, that's amazing. I've fixed the bug in getOwnPropertyDescriptor, and added an extra layer of security one level back at the native-pseudo conversion functions. Does this look good to you?
Yes, I think the fix looks good. And the extra checks will make it harder to exploit in the future.
@XmiliaH Can you reach out to me at fraser@google.com, thanks!
Want to note that at some points on pseudo to native conversions one can set the prototype of the native object with __proto__
for example at https://github.com/NeilFraser/JS-Interpreter/blob/00f8069b3a73885ee32a53384dadfc8821d799d9/interpreter.js#L2223.
Want to note that at some points on pseudo to native conversions one can set the prototype of the native object with
__proto__
for example at
That's not ideal. Fixed. Thanks!
That's not ideal. Fixed. Thanks!
I added a comment to that commit, because I think the fix is not as secure as it could be.
Question for @XmiliaH: are there any JS sandboxes you've not found a hole in yet?
I'm quite impressed by @NeilFraser managing to write one that outlasted vm2
(not to mention the laughable safe-eval
), and by the non-trivial amount of work it took to actually exploit this bug.
@cpcallen I would like to find a bug in v8, SpiderMonkey or JavaScriptCore, but I did not find anything there yet. Also there might be other JS sandboxes out there that I am unaware of. I for example didn't know about this one until I stumbled about you in a Issue from safe-eval and looked at your Repositories.
It is possible to break out of the interpreter and run native javascript code.
Following code will change the title when run in the live demo to 'On No':
The bug is in Object.getOwnPropertyDescriptor which will convert the getter and setter from native to pseudo, however it is already pseudo. This can be elevated to native javascript code execution.