hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-20 18:56:32 +01:00
Code Issues Packages Projects Releases Wiki Activity

Refactor the ScriptEngine query and the Rhino code injection query into one

Browse Source
This commit is contained in:
luchua-bc
2021-04-30 03:46:23 +00:00
parent b0b5338359
commit 852bcfb5c7
12 changed files with 151 additions and 214 deletions
Download Patch File Download Diff File Expand all files Collapse all files

51
java/ql/src/experimental/Security/CWE/CWE-094/RhinoInjection.qhelp
View File

@@ -1,51 +0,0 @@
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
<qhelp>
<overview>
<p>
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.
</p>
<p>
Typically an expression is evaluated in the powerful context initialized with
<code>initStandardObjects</code> that allows an expression of arbitrary Java code to
execute in the JVM.
</p>
</overview>
<recommendation>
<p>
In general, including user input in a Rhino expression should be avoided.
If user input must be included in the expression, it should be then evaluated in a safe
context that doesn't allow arbitrary code invocation.
</p>
</recommendation>
<example>
<p>
The following example shows two ways of using Rhino expression. In the 'BAD' case,
an unsafe context is initialized with <code>initStandardObjects</code>. In the 'GOOD' case,
a safe context is initialized with <code>initSafeStandardObjects</code> or
<code>setClassShutter</code>.
</p>
<sample src="RhinoInjection.java" />
</example>
<references>
<li>
Mozilla Rhino:
<a href="https://github.com/mozilla/rhino">Rhino: JavaScript in Java</a>
</li>
<li>
Rhino Sandbox:
<a href="https://github.com/javadelight/delight-rhino-sandbox">A sandbox to execute JavaScript code with Rhino in Java.</a>
</li>
<li>
GuardRails:
<a href="https://docs.guardrails.io/docs/en/vulnerabilities/java/insecure_use_of_dangerous_function">Code Injection</a>
</li>
</references>
</qhelp>

17
java/ql/src/experimental/Security/CWE/CWE-094/RhinoInjection.ql
View File

@@ -1,17 +0,0 @@
/**
* @name Injection in Mozilla Rhino JavaScript Engine
* @description Evaluation of a user-controlled JavaScript or Java expression in Rhino
* JavaScript Engine may lead to remote code execution.
* @kind path-problem
* @id java/rhino-injection
* @tags security
* external/cwe/cwe-094
*/
import java
import RhinoInjection
import DataFlow::PathGraph
from DataFlow::PathNode source, DataFlow::PathNode sink, RhinoInjectionConfig conf
where conf.hasFlowPath(source, sink)
select sink.getNode(), source, sink, "Rhino injection from $@.", source.getNode(), " user input"

56
java/ql/src/experimental/Security/CWE/CWE-094/RhinoInjection.qll
View File

@@ -1,56 +0,0 @@
import java
import semmle.code.java.dataflow.FlowSources
import semmle.code.java.dataflow.TaintTracking
/** The class `org.mozilla.javascript.Context`. */
class Context extends RefType {
Context() { this.hasQualifiedName("org.mozilla.javascript", "Context") }
}
/**
* A method that evaluates a Rhino expression.
*/
class EvaluateExpressionMethod extends Method {
EvaluateExpressionMethod() {
this.getDeclaringType().getAnAncestor*() instanceof Context and
(
hasName("evaluateString") or
hasName("evaluateReader")
)
}
}
/**
* A taint-tracking configuration for unsafe user input that is used to evaluate
* a Rhino expression.
*/
class RhinoInjectionConfig extends TaintTracking::Configuration {
RhinoInjectionConfig() { this = "RhinoInjectionConfig" }
override predicate isSource(DataFlow::Node source) {
source instanceof RemoteFlowSource
or
source instanceof LocalUserInput
}
override predicate isSink(DataFlow::Node sink) { sink instanceof EvaluateExpressionSink }
}
/**
* A sink for Rhino code injection vulnerabilities.
*/
class EvaluateExpressionSink extends DataFlow::ExprNode {
EvaluateExpressionSink() {
exists(MethodAccess ea, EvaluateExpressionMethod m | m = ea.getMethod() |
this.asExpr() = ea.getArgument(1) and // The second argument is the JavaScript or Java input
not exists(MethodAccess ca |
(
ca.getMethod().hasName("initSafeStandardObjects") // safe mode
or
ca.getMethod().hasName("setClassShutter") // `ClassShutter` constraint is enforced
) and
ea.getQualifier() = ca.getQualifier().(VarAccess).getVariable().getAnAccess()
)
)
}
}

26
java/ql/src/experimental/Security/CWE/CWE-094/ScriptEngine.qhelp
View File

@@ -1,26 +0,0 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>The ScriptEngine API has been available since the release of Java 6.
It allows applications to interact with scripts written in languages such as JavaScript.</p>
</overview>
<recommendation>
<p>Use "Cloudbees Rhino Sandbox" or sandboxing with SecurityManager or use <a href="https://www.graalvm.org/">graalvm</a> instead.</p>
</recommendation>
<example>
<p>The following code could execute random JavaScript code</p>
<sample src="ScriptEngine.java" />
<sample src="NashornScriptEngine.java" />
</example>
<references>
<li>
CERT coding standard: <a href="https://wiki.sei.cmu.edu/confluence/display/java/IDS52-J.+Prevent+code+injection">ScriptEngine code injection</a>
</li>
</references>
</qhelp>

51
java/ql/src/experimental/Security/CWE/CWE-094/ScriptEngine.ql
View File

@@ -1,51 +0,0 @@
/**
* @name ScriptEngine evaluation
* @description Malicious Javascript code could cause arbitrary command execution at the OS level
* @kind path-problem
* @problem.severity error
* @precision high
* @id java/unsafe-eval
* @tags security
* external/cwe/cwe-094
*/
import java
import semmle.code.java.dataflow.FlowSources
import DataFlow::PathGraph
class ScriptEngineMethod extends Method {
ScriptEngineMethod() {
this.getDeclaringType().getASupertype*().hasQualifiedName("javax.script", "ScriptEngine") and
this.hasName("eval")
}
}
predicate scriptEngine(MethodAccess ma, Expr sink) {
exists(Method m | m = ma.getMethod() |
m instanceof ScriptEngineMethod and
sink = ma.getArgument(0)
)
}
class ScriptEngineSink extends DataFlow::ExprNode {
ScriptEngineSink() { scriptEngine(_, this.getExpr()) }
MethodAccess getMethodAccess() { scriptEngine(result, this.getExpr()) }
}
class ScriptEngineConfiguration extends TaintTracking::Configuration {
ScriptEngineConfiguration() { this = "ScriptEngineConfiguration" }
override predicate isSource(DataFlow::Node source) {
source instanceof RemoteFlowSource
or
source instanceof LocalUserInput
}
override predicate isSink(DataFlow::Node sink) { sink instanceof ScriptEngineSink }
}
from DataFlow::PathNode source, DataFlow::PathNode sink, ScriptEngineConfiguration conf
where conf.hasFlowPath(source, sink)
select sink.getNode().(ScriptEngineSink).getMethodAccess(), source, sink, "ScriptEngine eval $@.",
source.getNode(), "user input"

49
java/ql/src/experimental/Security/CWE/CWE-094/ScriptInjection.qhelp Normal file
View File

@@ -0,0 +1,49 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>The JavaScript Engine API has been available since the release of Java 6, which allows
applications to interact with scripts written in languages such as JavaScript. 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.</p>
</overview>
<recommendation>
<p>In general, including user input in a JavaScript Engine expression should be avoided.
If user input must be included in the expression, it should be then evaluated in a safe
context that doesn't allow arbitrary code invocation. Use "Cloudbees Rhino Sandbox" or
sandboxing with SecurityManager or use <a href="https://www.graalvm.org/">graalvm</a>
instead.</p>
</recommendation>
<example>
<p>The following code could execute random JavaScript code in <code>ScriptEngine</code></p>
<sample src="ScriptEngine.java" />
<sample src="NashornScriptEngine.java" />
<p>The following example shows two ways of using Rhino expression. In the 'BAD' case,
an unsafe context is initialized with <code>initStandardObjects</code> that allows arbitrary
Java code to be executed. In the 'GOOD' case, a safe context is initialized with
<code>initSafeStandardObjects</code> or <code>setClassShutter</code>.</p>
<sample src="RhinoInjection.java" />
</example>
<references>
<li>
CERT coding standard: <a href="https://wiki.sei.cmu.edu/confluence/display/java/IDS52-J.+Prevent+code+injection">ScriptEngine code injection</a>
</li>
<li>
Mozilla Rhino: <a href="https://github.com/mozilla/rhino">Rhino: JavaScript in Java</a>
</li>
<li>
Rhino Sandbox: <a href="https://github.com/javadelight/delight-rhino-sandbox">A sandbox to execute JavaScript code with Rhino in Java</a>
</li>
<li>
GuardRails: <a href="https://docs.guardrails.io/docs/en/vulnerabilities/java/insecure_use_of_dangerous_function">Code Injection</a>
</li>
</references>
</qhelp>

93
java/ql/src/experimental/Security/CWE/CWE-094/ScriptInjection.ql Normal file
View File

@@ -0,0 +1,93 @@
/**
* @name Injection in JavaScript Engine
* @description Evaluation of a user-controlled malicious JavaScript or Java expression in
* JavaScript Engine may lead to remote code execution.
* @kind path-problem
* @problem.severity error
* @precision high
* @id java/unsafe-eval
* @tags security
* external/cwe/cwe-094
*/
import java
import semmle.code.java.dataflow.FlowSources
import DataFlow::PathGraph
class ScriptEngineMethod extends Method {
ScriptEngineMethod() {
this.getDeclaringType().getASupertype*().hasQualifiedName("javax.script", "ScriptEngine") and
this.hasName("eval")
}
}
/** The context class `org.mozilla.javascript.Context` of Rhino JavaScript Engine. */
class RhinoContext extends RefType {
RhinoContext() { this.hasQualifiedName("org.mozilla.javascript", "Context") }
}
/**
* A method that evaluates a Rhino expression.
*/
class RhinoEvaluateExpressionMethod extends Method {
RhinoEvaluateExpressionMethod() {
this.getDeclaringType().getAnAncestor*() instanceof RhinoContext and
(
hasName("evaluateString") or
hasName("evaluateReader")
)
}
}
predicate scriptEngine(MethodAccess ma, Expr sink) {
exists(Method m | m = ma.getMethod() |
m instanceof ScriptEngineMethod and
sink = ma.getArgument(0)
)
}
/**
* Holds if `ma` has Rhino code injection vulnerabilities.
*/
predicate evaluateRhinoExpression(MethodAccess ma, Expr sink) {
exists(RhinoEvaluateExpressionMethod m | m = ma.getMethod() |
sink = ma.getArgument(1) and // The second argument is the JavaScript or Java input
not exists(MethodAccess ca |
(
ca.getMethod().hasName("initSafeStandardObjects") // safe mode
or
ca.getMethod().hasName("setClassShutter") // `ClassShutter` constraint is enforced
) and
ma.getQualifier() = ca.getQualifier().(VarAccess).getVariable().getAnAccess()
)
)
}
class ScriptInjectionSink extends DataFlow::ExprNode {
ScriptInjectionSink() {
scriptEngine(_, this.getExpr()) or
evaluateRhinoExpression(_, this.getExpr())
}
MethodAccess getMethodAccess() {
scriptEngine(result, this.getExpr()) or
evaluateRhinoExpression(result, this.getExpr())
}
}
class ScriptInjectionConfiguration extends TaintTracking::Configuration {
ScriptInjectionConfiguration() { this = "ScriptInjectionConfiguration" }
override predicate isSource(DataFlow::Node source) {
source instanceof RemoteFlowSource
or
source instanceof LocalUserInput
}
override predicate isSink(DataFlow::Node sink) { sink instanceof ScriptInjectionSink }
}
from DataFlow::PathNode source, DataFlow::PathNode sink, ScriptInjectionConfiguration conf
where conf.hasFlowPath(source, sink)
select sink.getNode().(ScriptInjectionSink).getMethodAccess(), source, sink,
"JavaScript Engine evaluate $@.", source.getNode(), "user input"

7
java/ql/test/experimental/query-tests/security/CWE-094/RhinoInjection.expected
View File

@@ -1,7 +0,0 @@
edges
| RhinoServlet.java:25:23:25:50 | getParameter(...) : String | RhinoServlet.java:29:55:29:58 | code |
nodes
| RhinoServlet.java:25:23:25:50 | getParameter(...) : String | semmle.label | getParameter(...) : String |
| RhinoServlet.java:29:55:29:58 | code | semmle.label | code |
#select
| RhinoServlet.java:29:55:29:58 | code | RhinoServlet.java:25:23:25:50 | getParameter(...) : String | RhinoServlet.java:29:55:29:58 | code | Rhino injection from $@. | RhinoServlet.java:25:23:25:50 | getParameter(...) | user input |

1
java/ql/test/experimental/query-tests/security/CWE-094/RhinoInjection.qlref
View File

@@ -1 +0,0 @@
experimental/Security/CWE/CWE-094/RhinoInjection.ql

1
java/ql/test/experimental/query-tests/security/CWE-094/ScriptEngine.qlref
View File

@@ -1 +0,0 @@
experimental/Security/CWE/CWE-094/ScriptEngine.ql

12
java/ql/test/experimental/query-tests/security/CWE-094/ScriptEngine.expected → java/ql/test/experimental/query-tests/security/CWE-094/ScriptInjection.expected
View File

@@ -1,4 +1,5 @@
edges edges
| RhinoServlet.java:25:23:25:50 | getParameter(...) : String | RhinoServlet.java:29:55:29:58 | code |
| ScriptEngineTest.java:8:44:8:55 | input : String | ScriptEngineTest.java:12:37:12:41 | input | | ScriptEngineTest.java:8:44:8:55 | input : String | ScriptEngineTest.java:12:37:12:41 | input |
| ScriptEngineTest.java:15:51:15:62 | input : String | ScriptEngineTest.java:19:31:19:35 | input | | ScriptEngineTest.java:15:51:15:62 | input : String | ScriptEngineTest.java:19:31:19:35 | input |
| ScriptEngineTest.java:23:58:23:69 | input : String | ScriptEngineTest.java:27:31:27:35 | input | | ScriptEngineTest.java:23:58:23:69 | input : String | ScriptEngineTest.java:27:31:27:35 | input |
@@ -12,6 +13,8 @@ edges
| ScriptEngineTest.java:40:70:40:76 | ...[...] : String | ScriptEngineTest.java:23:58:23:69 | input : String | | ScriptEngineTest.java:40:70:40:76 | ...[...] : String | ScriptEngineTest.java:23:58:23:69 | input : String |
| ScriptEngineTest.java:41:58:41:64 | ...[...] : String | ScriptEngineTest.java:30:46:30:57 | input : String | | ScriptEngineTest.java:41:58:41:64 | ...[...] : String | ScriptEngineTest.java:30:46:30:57 | input : String |
nodes nodes
| RhinoServlet.java:25:23:25:50 | getParameter(...) : String | semmle.label | getParameter(...) : String |
| RhinoServlet.java:29:55:29:58 | code | semmle.label | code |
| ScriptEngineTest.java:8:44:8:55 | input : String | semmle.label | input : String | | ScriptEngineTest.java:8:44:8:55 | input : String | semmle.label | input : String |
| ScriptEngineTest.java:12:37:12:41 | input | semmle.label | input | | ScriptEngineTest.java:12:37:12:41 | input | semmle.label | input |
| ScriptEngineTest.java:15:51:15:62 | input : String | semmle.label | input : String | | ScriptEngineTest.java:15:51:15:62 | input : String | semmle.label | input : String |
@@ -26,7 +29,8 @@ nodes
| ScriptEngineTest.java:40:70:40:76 | ...[...] : String | semmle.label | ...[...] : String | | ScriptEngineTest.java:40:70:40:76 | ...[...] : String | semmle.label | ...[...] : String |
| ScriptEngineTest.java:41:58:41:64 | ...[...] : String | semmle.label | ...[...] : String | | ScriptEngineTest.java:41:58:41:64 | ...[...] : String | semmle.label | ...[...] : String |
#select #select
| ScriptEngineTest.java:12:19:12:42 | eval(...) | ScriptEngineTest.java:37:26:37:38 | args : String[] | ScriptEngineTest.java:12:37:12:41 | input | ScriptEngine eval $@. | ScriptEngineTest.java:37:26:37:38 | args | user input | | RhinoServlet.java:29:29:29:78 | evaluateString(...) | RhinoServlet.java:25:23:25:50 | getParameter(...) : String | RhinoServlet.java:29:55:29:58 | code | JavaScript Engine evaluate $@. | RhinoServlet.java:25:23:25:50 | getParameter(...) | user input |
| ScriptEngineTest.java:19:19:19:36 | eval(...) | ScriptEngineTest.java:37:26:37:38 | args : String[] | ScriptEngineTest.java:19:31:19:35 | input | ScriptEngine eval $@. | ScriptEngineTest.java:37:26:37:38 | args | user input | | ScriptEngineTest.java:12:19:12:42 | eval(...) | ScriptEngineTest.java:37:26:37:38 | args : String[] | ScriptEngineTest.java:12:37:12:41 | input | JavaScript Engine evaluate $@. | ScriptEngineTest.java:37:26:37:38 | args | user input |
| ScriptEngineTest.java:27:19:27:36 | eval(...) | ScriptEngineTest.java:37:26:37:38 | args : String[] | ScriptEngineTest.java:27:31:27:35 | input | ScriptEngine eval $@. | ScriptEngineTest.java:37:26:37:38 | args | user input | | ScriptEngineTest.java:19:19:19:36 | eval(...) | ScriptEngineTest.java:37:26:37:38 | args : String[] | ScriptEngineTest.java:19:31:19:35 | input | JavaScript Engine evaluate $@. | ScriptEngineTest.java:37:26:37:38 | args | user input |
| ScriptEngineTest.java:34:19:34:36 | eval(...) | ScriptEngineTest.java:37:26:37:38 | args : String[] | ScriptEngineTest.java:34:31:34:35 | input | ScriptEngine eval $@. | ScriptEngineTest.java:37:26:37:38 | args | user input | | ScriptEngineTest.java:27:19:27:36 | eval(...) | ScriptEngineTest.java:37:26:37:38 | args : String[] | ScriptEngineTest.java:27:31:27:35 | input | JavaScript Engine evaluate $@. | ScriptEngineTest.java:37:26:37:38 | args | user input |
| ScriptEngineTest.java:34:19:34:36 | eval(...) | ScriptEngineTest.java:37:26:37:38 | args : String[] | ScriptEngineTest.java:34:31:34:35 | input | JavaScript Engine evaluate $@. | ScriptEngineTest.java:37:26:37:38 | args | user input |

1
java/ql/test/experimental/query-tests/security/CWE-094/ScriptInjection.qlref Normal file
View File

@@ -0,0 +1 @@
experimental/Security/CWE/CWE-094/ScriptInjection.ql