-
Notifications
You must be signed in to change notification settings - Fork 123
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
Sandbox can be broken. #50
Comments
Hi, P.S Thank you for the help. |
@sand1er the code does the following:
Buy you could run commands by replacing You should think I found a very similar thing in another major sandbox (sandcastle) bcoe/sandcastle#70 hopefully all the people who download this modules don't use it for production or something. I basically ended up making my own sandbox... (which I don't want to advertise here) Feel free to ask more questions if you don't understand something. |
I've just used this to escape a hubot's sandbox and list the contents of /etc/passwd and exfiltrate random memory via Buffer. This should definitely be fixed. |
Same issue seems to got found in MathJS by @CapacitorSet ( https://capacitorset.github.io/mathjs/ ) some time ago. I am emailing gf3 (and npm) to try to claim the name so a fixed version can be released. About possible solutions:
I am opening a issue in most of the packages that depend on this one. |
Thanks for the mention, @io4! (I'll also mention @denysvitali for helping find the vulnerability ;) ) I really suggest everyone not to roll your own solution, because it can be really difficult to sanitize JavaScript properly. I do malware analysis on JavaScript files, and so far my solution of choice has been @patriksimek's Patrik, do you think |
Also, pinging @josdejong - seeing that you also had trouble sanitizing JS, I think you may be interested if we do come up with a good solution. |
I think a vm2 port for browsers would not be possible, as no new contexts can be made. The sad part is that Proxy is one of the few ES6 components that can not have a pollyfill. So newer JS engines are needed. For older Node versions I had to use I used the following (boiled down) code:
|
@CapacitorSet yes it's a nightmare to get arbitrary JS code evaluation secured. It looks like we managed to get the expression parser of mathjs secure after months of hard work. I think it was only possible because mathjs has it's own expression parser which has full compile time and runtime control to do security checks and restrictions. I don't think it's possible to secure a JS sandbox as long as there is a regular JS engine running there. You really need to have runtime control on evaluating methods and accessing/assigning properties to prevent exploits getting access to |
There is a workaround, but you cant have any (dynamic) external values.
And then all operations are done with .call to avoid objects with any of .toString .inspect .valueOf |
@CapacitorSet Unfortunately, @io4 is right - without creating a whole new context, there seems to be no other way to prevent things like I'm not sure if there are more places like this but since this is a major issue, I was not looking further. I appreciate any help with this since I'm out of ideas at the moment. I have created a separate issue for future discussion about this feature - patriksimek/vm2#85 |
I found this while searching for other things: https://github.com/browserify/vm-browserify It is too late in the night now, but tomorrow I'll try it and figure out whether it is a suitable alternative. |
@patriksimek @io4 Do you know if |
No, it's not enough. Patrik gives an example on vm2's project page: |
That example only appears to work for `vm.runInNewContext` with its default
sandbox argument (an empty object). `Object.create(null)` as an alternative
for the sandbox argument was provided as a possible mitigation. I can't
seem to get a reference to the process library this way.
On Sep 22, 2017 6:06 AM, "CapacitorSet" <notifications@github.com> wrote:
No, it's not enough. Patrik gives an example on vm2
<https://github.com/patriksimek/vm2>'s project page:
this.constructor.constructor('return process')().exit().
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#50 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AA5FI6iJmtGeCuhjiKe-c0rjvc5Hi4p4ks5sk4ajgaJpZM4JpOba>
.
|
Ohh, I see, it makes sense! Indeed, such a sandbox seems to be safe - I tested it against the most significant tests from |
Then you need to be VERY careful with the operations called in the output... you cant just out.toString() or util.inspect, you need to set right options to avoid running code (toString(), valueOf(), inspect(), toJSON()) |
@io4 I'm calling Node from Python (just running the You can see it here: https://github.com/Anorov/cloudflare-scrape/blob/master/cfscrape/__init__.py#L111 |
Using functions and constructors, its possible to escape the sandbox to get process, which can be used to get require that can be used for evil things like a reverse shell.
Code:
new Function("return (this.constructor.constructor('return (this.process.mainModule.constructor._load)')())")()("util").inspect("hi")
A, I hope, more readable (because of how hacky the thing is its difficult) version:
The text was updated successfully, but these errors were encountered: