Skip to content

Java: CWE-094 Rhino code injection #5802

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

Merged
merged 10 commits into from
May 18, 2021
Merged

Conversation

luchua-bc
Copy link
Contributor

Rhino is a JavaScript engine written fully in Java and managed by the Mozilla Foundation.
It serves as an embedded scripting engine inside Java applications which allows
Java-to-JavaScript interoperability and provides a seamless integration between the two
languages. If an expression is built using attacker-controlled data, and then evaluated in
a powerful context, it may allow the attacker to run arbitrary code.

Typically an expression is evaluated in the powerful context initialized with
initStandardObjects that allows an expression of arbitrary Java code to
execute in the JVM.

The query detects unsafe usage of Rhino expressions. Please consider to merge the PR. Thanks.

Copy link
Contributor

@Marcono1234 Marcono1234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the following Scripting API methods might be interesting as well:

  • Compilable.compile
    If a script is compiled, it is very likely that the application later executes it. Therefore this might allow remote code execution as well.
  • ScriptEngineFactory.getProgram
    If an application gets the program script, it is likely that it later executes that script as well.
  • ScriptEngineFactory.getMethodCallSyntax
    Not completely sure about this one, but maybe it can be misused for remote code execution as well (possibly by providing malicious args values, or in case the other arguments are not properly validated and allow code injection); with the same rationale as above: If an application uses this method, it is likely that is executes its result later on.

For Rhino maybe the following is interesting as well:

However, I am not a member of this project, so feel free to consider this only as suggestion. The maintainers will probably give you more in depth feedback.

@luchua-bc
Copy link
Contributor Author

Thanks @Marcono1234 for reviewing this PR. I just submitted another PR and will make requested changes to this one in the next two days.

@luchua-bc
Copy link
Contributor Author

All requested changes have been made. Please review.

@luchua-bc luchua-bc force-pushed the java/rhino-injection branch from aa2925d to 5d282d2 Compare May 5, 2021 16:17
wood14
wood14 previously approved these changes May 5, 2021
@luchua-bc
Copy link
Contributor Author

Thanks @smowton for reviewing this PR. I've made all requested changes. Please review again.

@luchua-bc luchua-bc force-pushed the java/rhino-injection branch from 1480696 to d85df6e Compare May 14, 2021 15:30
Copy link
Contributor

@smowton smowton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that's what I meant

smowton
smowton previously approved these changes May 18, 2021
@smowton
Copy link
Contributor

smowton commented May 18, 2021

Needs rebase

@luchua-bc
Copy link
Contributor Author

@smowton I've done the rebase. Please merge again. Thanks.

@smowton smowton merged commit 0c970b5 into github:main May 18, 2021
@luchua-bc luchua-bc deleted the java/rhino-injection branch May 23, 2021 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants