Skip to content

Commit 0c970b5

Browse files
smowtonsmowton
authored
Merge pull request github#5802 from luchua-bc/java/rhino-injection
Java: CWE-094 Rhino code injection
2 parents 9deaace + 02aa9c6 commit 0c970b5

32 files changed

+2249
-133
lines changed

‎java/ql/src/experimental/Security/CWE/CWE-094/RhinoInjection.java

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import org.mozilla.javascript.ClassShutter;
2+
import org.mozilla.javascript.Context;
3+
import org.mozilla.javascript.Scriptable;
4+
5+
public class RhinoInjection extends HttpServlet {
6+
7+
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
8+
response.setContentType("text/plain");
9+
String code = request.getParameter("code");
10+
Context ctx = Context.enter();
11+
try {
12+
{
13+
// BAD: allow arbitrary Java and JavaScript code to be executed
14+
Scriptable scope = ctx.initStandardObjects();
15+
}
16+
17+
{
18+
// GOOD: enable the safe mode
19+
Scriptable scope = ctx.initSafeStandardObjects();
20+
}
21+
22+
{
23+
// GOOD: enforce a constraint on allowed classes
24+
Scriptable scope = ctx.initStandardObjects();
25+
ctx.setClassShutter(new ClassShutter() {
26+
public boolean visibleToScripts(String className) {
27+
return className.startsWith("com.example.");
28+
}
29+
});
30+
}
31+
32+
Object result = ctx.evaluateString(scope, code, "<code>", 1, null);
33+
response.getWriter().print(Context.toString(result));
34+
} catch(RhinoException ex) {
35+
response.getWriter().println(ex.getMessage());
36+
} finally {
37+
Context.exit();
38+
}
39+
}
40+
}

‎java/ql/src/experimental/Security/CWE/CWE-094/ScriptEngine.qhelp

Lines changed: 0 additions & 26 deletions
This file was deleted.

‎java/ql/src/experimental/Security/CWE/CWE-094/ScriptEngine.ql

Lines changed: 0 additions & 51 deletions
This file was deleted.

‎java/ql/src/experimental/Security/CWE/CWE-094/ScriptInjection.qhelp

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>The Java Scripting API has been available since the release of Java 6. It allows
8+
applications to interact with scripts written in languages such as JavaScript. It serves
9+
as an embedded scripting engine inside Java applications which allows Java-to-JavaScript
10+
interoperability and provides a seamless integration between the two languages. If an
11+
expression is built using attacker-controlled data, and then evaluated in a powerful
12+
context, it may allow the attacker to run arbitrary code.</p>
13+
</overview>
14+
15+
<recommendation>
16+
<p>In general, including user input in a Java Script Engine expression should be avoided.
17+
If user input must be included in the expression, it should be then evaluated in a safe
18+
context that doesn't allow arbitrary code invocation. Use "Cloudbees Rhino Sandbox" or
19+
sandboxing with SecurityManager, which will be deprecated in a future release, or use
20+
<a href="https://www.graalvm.org/">GraalVM</a> instead.</p>
21+
</recommendation>
22+
23+
<example>
24+
<p>The following code could execute user-supplied JavaScript code in <code>ScriptEngine</code></p>
25+
<sample src="ScriptEngine.java" />
26+
<sample src="NashornScriptEngine.java" />
27+
28+
<p>The following example shows two ways of using Rhino expression. In the 'BAD' case,
29+
an unsafe context is initialized with <code>initStandardObjects</code> that allows arbitrary
30+
Java code to be executed. In the 'GOOD' case, a safe context is initialized with
31+
<code>initSafeStandardObjects</code> or <code>setClassShutter</code>.</p>
32+
<sample src="RhinoInjection.java" />
33+
</example>
34+
35+
<references>
36+
<li>
37+
CERT coding standard: <a href="https://wiki.sei.cmu.edu/confluence/display/java/IDS52-J.+Prevent+code+injection">ScriptEngine code injection</a>
38+
</li>
39+
<li>
40+
GraalVM: <a href="https://www.graalvm.org/reference-manual/js/NashornMigrationGuide/#secure-by-default">Secure by Default</a>
41+
</li>
42+
<li>
43+
Mozilla Rhino: <a href="https://github.com/mozilla/rhino">Rhino: JavaScript in Java</a>
44+
</li>
45+
<li>
46+
Rhino Sandbox: <a href="https://github.com/javadelight/delight-rhino-sandbox">A sandbox to execute JavaScript code with Rhino in Java</a>
47+
</li>
48+
<li>
49+
GuardRails: <a href="https://docs.guardrails.io/docs/en/vulnerabilities/java/insecure_use_of_dangerous_function#code-injection">Code Injection</a>
50+
</li>
51+
</references>
52+
</qhelp>

‎java/ql/src/experimental/Security/CWE/CWE-094/ScriptInjection.ql

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
/**
2+
* @name Injection in Java Script Engine
3+
* @description Evaluation of user-controlled data using the Java Script Engine may
4+
* lead to remote code execution.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @precision high
8+
* @id java/unsafe-eval
9+
* @tags security
10+
* external/cwe/cwe-094
11+
*/
12+
13+
import java
14+
import semmle.code.java.dataflow.FlowSources
15+
import DataFlow::PathGraph
16+
17+
/** A method of ScriptEngine that allows code injection. */
18+
class ScriptEngineMethod extends Method {
19+
ScriptEngineMethod() {
20+
this.getDeclaringType().getASupertype*().hasQualifiedName("javax.script", "ScriptEngine") and
21+
this.hasName("eval")
22+
or
23+
this.getDeclaringType().getASupertype*().hasQualifiedName("javax.script", "Compilable") and
24+
this.hasName("compile")
25+
or
26+
this.getDeclaringType().getASupertype*().hasQualifiedName("javax.script", "ScriptEngineFactory") and
27+
this.hasName(["getProgram", "getMethodCallSyntax"])
28+
}
29+
}
30+
31+
/** The context class `org.mozilla.javascript.Context` of Rhino Java Script Engine. */
32+
class RhinoContext extends RefType {
33+
RhinoContext() { this.hasQualifiedName("org.mozilla.javascript", "Context") }
34+
}
35+
36+
/** A method that evaluates a Rhino expression with `org.mozilla.javascript.Context`. */
37+
class RhinoEvaluateExpressionMethod extends Method {
38+
RhinoEvaluateExpressionMethod() {
39+
this.getDeclaringType().getAnAncestor*() instanceof RhinoContext and
40+
this.hasName([
41+
"evaluateString", "evaluateReader", "compileFunction", "compileReader", "compileString"
42+
])
43+
}
44+
}
45+
46+
/**
47+
* A method that compiles a Rhino expression with
48+
* `org.mozilla.javascript.optimizer.ClassCompiler`.
49+
*/
50+
class RhinoCompileClassMethod extends Method {
51+
RhinoCompileClassMethod() {
52+
this.getDeclaringType()
53+
.getASupertype*()
54+
.hasQualifiedName("org.mozilla.javascript.optimizer", "ClassCompiler") and
55+
this.hasName("compileToClassFiles")
56+
}
57+
}
58+
59+
/**
60+
* A method that defines a Java class from a Rhino expression with
61+
* `org.mozilla.javascript.GeneratedClassLoader`.
62+
*/
63+
class RhinoDefineClassMethod extends Method {
64+
RhinoDefineClassMethod() {
65+
this.getDeclaringType()
66+
.getASupertype*()
67+
.hasQualifiedName("org.mozilla.javascript", "GeneratedClassLoader") and
68+
this.hasName("defineClass")
69+
}
70+
}
71+
72+
/**
73+
* Holds if `ma` is a call to a `ScriptEngineMethod` and `sink` is an argument that
74+
* will be executed.
75+
*/
76+
predicate isScriptArgument(MethodAccess ma, Expr sink) {
77+
exists(ScriptEngineMethod m |
78+
m = ma.getMethod() and
79+
if m.getDeclaringType().getASupertype*().hasQualifiedName("javax.script", "ScriptEngineFactory")
80+
then sink = ma.getArgument(_) // all arguments allow script injection
81+
else sink = ma.getArgument(0)
82+
)
83+
}
84+
85+
/**
86+
* Holds if a Rhino expression evaluation method is vulnerable to code injection.
87+
*/
88+
predicate evaluatesRhinoExpression(MethodAccess ma, Expr sink) {
89+
exists(RhinoEvaluateExpressionMethod m | m = ma.getMethod() |
90+
(
91+
if ma.getMethod().getName() = "compileReader"
92+
then sink = ma.getArgument(0) // The first argument is the input reader
93+
else sink = ma.getArgument(1) // The second argument is the JavaScript or Java input
94+
) and
95+
not exists(MethodAccess ca |
96+
ca.getMethod().hasName(["initSafeStandardObjects", "setClassShutter"]) and // safe mode or `ClassShutter` constraint is enforced
97+
ma.getQualifier() = ca.getQualifier().(VarAccess).getVariable().getAnAccess()
98+
)
99+
)
100+
}
101+
102+
/**
103+
* Holds if a Rhino expression compilation method is vulnerable to code injection.
104+
*/
105+
predicate compilesScript(MethodAccess ma, Expr sink) {
106+
exists(RhinoCompileClassMethod m | m = ma.getMethod() | sink = ma.getArgument(0))
107+
}
108+
109+
/**
110+
* Holds if a Rhino class loading method is vulnerable to code injection.
111+
*/
112+
predicate definesRhinoClass(MethodAccess ma, Expr sink) {
113+
exists(RhinoDefineClassMethod m | m = ma.getMethod() | sink = ma.getArgument(1))
114+
}
115+
116+
/** A script injection sink. */
117+
class ScriptInjectionSink extends DataFlow::ExprNode {
118+
MethodAccess methodAccess;
119+
120+
ScriptInjectionSink() {
121+
isScriptArgument(methodAccess, this.getExpr()) or
122+
evaluatesRhinoExpression(methodAccess, this.getExpr()) or
123+
compilesScript(methodAccess, this.getExpr()) or
124+
definesRhinoClass(methodAccess, this.getExpr())
125+
}
126+
127+
/** An access to the method associated with this sink. */
128+
MethodAccess getMethodAccess() { result = methodAccess }
129+
}
130+
131+
/**
132+
* A taint tracking configuration that tracks flow from `RemoteFlowSource` to an argument
133+
* of a method call that executes injected script.
134+
*/
135+
class ScriptInjectionConfiguration extends TaintTracking::Configuration {
136+
ScriptInjectionConfiguration() { this = "ScriptInjectionConfiguration" }
137+
138+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
139+
140+
override predicate isSink(DataFlow::Node sink) { sink instanceof ScriptInjectionSink }
141+
}
142+
143+
from DataFlow::PathNode source, DataFlow::PathNode sink, ScriptInjectionConfiguration conf
144+
where conf.hasFlowPath(source, sink)
145+
select sink.getNode().(ScriptInjectionSink).getMethodAccess(), source, sink,
146+
"Java Script Engine evaluate $@.", source.getNode(), "user input"

0 commit comments

Comments
 (0)