Skip to content
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

Unable to block access to NashornScriptEngine #73

Closed
mxro opened this issue Dec 21, 2018 · 17 comments
Closed

Unable to block access to NashornScriptEngine #73

mxro opened this issue Dec 21, 2018 · 17 comments

Comments

@mxro
Copy link
Collaborator

mxro commented Dec 21, 2018

Nashorn exposes an instance of NashronScriptEngine through the engine property.

This allows executing arbitrary code as follows:

sandbox.eval("delete this.engine; this.engine.factory.scriptEngine.compile('var File = Java.type(\"java.io.File\"); File;').eval()");

The test case for this is defined here: Test Engine

Workaround:

It is suggested that this can be resolved when using a SecurityManager.

@mxro mxro added the security label Dec 21, 2018
@mbechler
Copy link

One addition: Having an active SecurityManager only mitigates the issue when using the most recent Java versions (8u191 etc.)

@amlweems
Copy link

amlweems commented Jan 9, 2019

I've been experimenting with potential solutions that don't involve a security manager or using the jdk.nashorn.internal package.

So far, the best solution I've got relies on JavaScript property descriptors to protect the engine property from being deleted (as shown below).

Object.defineProperty(this, 'engine', {});
Object.defineProperty(this, 'context', {});
delete this.__noSuchProperty__;

The default defineProperty descriptor disable configuration, enumeration, and writing for that property. This leaves the engine property effectively unmodifiable (but, this seems like a fairly weak security boundary).

Overall, I agree with the recommendation to use a SecurityManager.

@mxro
Copy link
Collaborator Author

mxro commented Jan 11, 2019

Hi Anthony, thanks heaps for the info!

So do you think adding this to the initialisation for the script could provide some additional (if limited) protection for scripts and its worthwhile to do this by default?

@joa23
Copy link

joa23 commented Jan 26, 2019

Hi Max, Hi All,
This is, of course, a major concern do you think there is a solution to this issue in the near term?
It seems like your Rhino sandbox does not have such an issue but is not as active under development. Which of the two implementations do you recommend given this issue?
Thanks.

@mxro
Copy link
Collaborator Author

mxro commented Jan 27, 2019

@joa23 Please be welcome to use either project. If you find an issue with the Rhino Sandbox, just report it.

I have committed the fix suggested by Anthony. Would anyone know a way to get around this implemented fix? TestEngine.java tests pass now as expected.

@mxro mxro added the solved label Jan 27, 2019
@joa23
Copy link

joa23 commented Jan 27, 2019

@mxro - thanks for committing the fix.
Are you planning to cut a release and push it to the maven repos anytime soon?
I think I saw a blog post in which you recommend the Rhino Sandbox over the Nashorn one given some memory challenges. Is that still true?
Thanks for putting these very helpful libraries together and sorry misusing the issue to ask all these questions.

@mxro
Copy link
Collaborator Author

mxro commented Jan 27, 2019

@joa23 - new version should have already been deployed to Maven central :)

I think it all depends on your usecase which library will work better. Please in any case note that proper sandboxing is one of the most challenging things to do in terms of security and both libraries should always be seen with a grain of salt!

@analog-hors
Copy link

Sorry: Is this still an issue or have you guys fixed it?

@mxro
Copy link
Collaborator Author

mxro commented Sep 14, 2019

@KSean222 Yes, this issue should be solved!

@wsargent
Copy link

wsargent commented May 3, 2021

Using a security manager is not a reliable solution in itself, as it's fairly easy to disable the security manager itself.

@mxro
Copy link
Collaborator Author

mxro commented May 3, 2021

@wsargent Good point! I think this implementation does not rely on security manager: a51cb57 - however, as with all things sandbox, there may be a way around it. If there is, please raise here and we could see if there is a way to patch things.

@solante1012
Copy link

solante1012 commented Nov 28, 2022

Nashorn exposes an instance of NashronScriptEngine through the engine property.

This allows executing arbitrary code as follows:

sandbox.eval("delete this.engine; this.engine.factory.scriptEngine.compile('var File = Java.type(\"java.io.File\"); File;').eval()");

The test case for this is defined here: Test Engine

Workaround:

It is suggested that this can be resolved when using a SecurityManager.

i am very confused why the POC this.engine.factory.scriptEngine could bypass the sandbox ,Anybody can explain it ?
what is the different between engine.factory.getScriptEngine() and this.engine.factory.scriptEngine , and i don't get it
because the object factory donn't have a property scriptEngine.

@mxro
Copy link
Collaborator Author

mxro commented Dec 1, 2022

Good question! I assume it is some quirk in the Nashorn Engine to expose the engine through the this property.

As per the thread above, I believe this should be fixed now. Does this still occur for you?

@mbechler
Copy link

mbechler commented Dec 1, 2022

Accessing the property on Java objects will call the getter method. Should not matter whether it's .getFactory().getScriptEngine() or .factory.scriptEngine.

@solante1012
Copy link

Good question! I assume it is some quirk in the Nashorn Engine to expose the engine through the this property.

As per the thread above, I believe this should be fixed now. Does this still occur for you?

so what do u mean "quirk" ? :)i am what to know how the bottom logic for the Nashorn Engine ? Anyone can explain?

@fa1c0n1
Copy link

fa1c0n1 commented May 28, 2024

So, this issue can lead to bypass this sandbox to remote code execution? Besides, why this issue is still open?

@mxro
Copy link
Collaborator Author

mxro commented Jul 27, 2024

Thanks for following up @fa1c0n1!

Here is the test case that validates this scenario is now covered by the sandbox: https://github.com/javadelight/delight-nashorn-sandbox/blob/master/src/test/java/delight/nashornsandbox/TestEngine.java

Unless someone provides a different test case that shows this can still be exploited, I will close this issue.

Note that I expect a sandbox like this can be escaped. Please use other sandboxing methods if you want to be sure it is safe for public use.

@mxro mxro closed this as completed Jul 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants