diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
index 0d723ac05b6..01338eaeff4 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
@@ -735,7 +735,12 @@ private module FieldFlow {
private class FieldConfiguration extends Configuration {
FieldConfiguration() { this = "FieldConfiguration" }
- override predicate isSource(Node source) { storeStep(source, _, _) }
+ override predicate isSource(Node source) {
+ storeStep(source, _, _)
+ or
+ // Also mark `foo(a.b);` as a source when `a.b` may be overwritten by `foo`.
+ readStep(_, _, any(Node node | node.asExpr() = source.asDefiningArgument()))
+ }
override predicate isSink(Node sink) { readStep(_, _, sink) }
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected
index 75cdaee69ac..bb14729a12b 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected
@@ -70,3 +70,8 @@
| test.cpp:391:11:391:13 | tmp | test.cpp:391:10:391:13 | & ... |
| test.cpp:391:17:391:23 | source1 | test.cpp:391:10:391:13 | ref arg & ... |
| test.cpp:391:17:391:23 | source1 | test.cpp:391:16:391:23 | & ... |
+| test.cpp:480:67:480:67 | s | test.cpp:481:21:481:21 | s |
+| test.cpp:480:67:480:67 | s | test.cpp:482:20:482:20 | s |
+| test.cpp:481:21:481:21 | s [post update] | test.cpp:482:20:482:20 | s |
+| test.cpp:481:24:481:30 | ref arg content | test.cpp:482:23:482:29 | content |
+| test.cpp:482:23:482:29 | content | test.cpp:483:9:483:17 | p_content |
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp
index f59552aa2dd..afceddfabd0 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp
@@ -470,3 +470,15 @@ void viaOutparam() {
intOutparamSource(&x);
sink(x); // $ ast,ir
}
+
+void writes_to_content(void*);
+
+struct MyStruct {
+ int* content;
+};
+
+void local_field_flow_def_by_ref_steps_with_local_flow(MyStruct * s) {
+ writes_to_content(s->content);
+ int* p_content = s->content;
+ sink(*p_content);
+}
\ No newline at end of file
diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
index ce939661c92..27cfbb6430a 100644
--- a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
+++ b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
@@ -496,9 +496,13 @@
| map.cpp:49:7:49:7 | f [post update] | map.cpp:51:7:51:7 | f | |
| map.cpp:49:7:49:7 | f [post update] | map.cpp:53:30:53:30 | f | |
| map.cpp:49:7:49:7 | f [post update] | map.cpp:59:6:59:6 | f | |
+| map.cpp:49:9:49:13 | ref arg first | map.cpp:54:9:54:13 | first | |
+| map.cpp:49:9:49:13 | ref arg first | map.cpp:60:9:60:13 | first | |
| map.cpp:50:7:50:7 | f [post update] | map.cpp:51:7:51:7 | f | |
| map.cpp:50:7:50:7 | f [post update] | map.cpp:53:30:53:30 | f | |
| map.cpp:50:7:50:7 | f [post update] | map.cpp:59:6:59:6 | f | |
+| map.cpp:50:9:50:14 | ref arg second | map.cpp:55:9:55:14 | second | |
+| map.cpp:50:9:50:14 | ref arg second | map.cpp:61:9:61:14 | second | |
| map.cpp:53:30:53:30 | f | map.cpp:54:7:54:7 | g | |
| map.cpp:53:30:53:30 | f | map.cpp:55:7:55:7 | g | |
| map.cpp:53:30:53:30 | f | map.cpp:56:7:56:7 | g | |
@@ -3395,6 +3399,7 @@
| smart_pointer.cpp:125:20:125:20 | call to operator-> [post update] | smart_pointer.cpp:125:18:125:19 | ref arg p1 | TAINT |
| smart_pointer.cpp:125:22:125:22 | q | smart_pointer.cpp:125:18:125:22 | call to shared_ptr | |
| smart_pointer.cpp:125:22:125:22 | ref arg q | smart_pointer.cpp:125:22:125:22 | q [inner post update] | |
+| smart_pointer.cpp:125:22:125:22 | ref arg q | smart_pointer.cpp:126:12:126:12 | q | |
| smart_pointer.cpp:126:8:126:9 | p1 | smart_pointer.cpp:126:10:126:10 | call to operator-> | |
| smart_pointer.cpp:126:8:126:9 | ref arg p1 | smart_pointer.cpp:124:48:124:49 | p1 | |
| smart_pointer.cpp:126:10:126:10 | call to operator-> [post update] | smart_pointer.cpp:126:8:126:9 | ref arg p1 | TAINT |
@@ -3432,6 +3437,7 @@
| smart_pointer.cpp:133:23:133:24 | ref arg p1 | smart_pointer.cpp:132:53:132:54 | p1 | |
| smart_pointer.cpp:133:23:133:24 | ref arg p1 | smart_pointer.cpp:134:8:134:9 | p1 | |
| smart_pointer.cpp:133:25:133:25 | call to operator-> [post update] | smart_pointer.cpp:133:23:133:24 | ref arg p1 | TAINT |
+| smart_pointer.cpp:133:27:133:27 | ref arg q | smart_pointer.cpp:134:12:134:12 | q | |
| smart_pointer.cpp:134:8:134:9 | p1 | smart_pointer.cpp:134:10:134:10 | call to operator-> | |
| smart_pointer.cpp:134:8:134:9 | ref arg p1 | smart_pointer.cpp:132:53:132:54 | p1 | |
| smart_pointer.cpp:134:10:134:10 | call to operator-> [post update] | smart_pointer.cpp:134:8:134:9 | ref arg p1 | TAINT |
@@ -6435,6 +6441,7 @@
| taint.cpp:669:18:669:18 | s [post update] | taint.cpp:671:7:671:7 | s | |
| taint.cpp:669:18:669:18 | s [post update] | taint.cpp:672:7:672:7 | s | |
| taint.cpp:669:18:669:18 | s [post update] | taint.cpp:673:7:673:7 | s | |
+| taint.cpp:669:20:669:20 | ref arg x | taint.cpp:672:9:672:9 | x | |
| taint.cpp:672:7:672:7 | s [post update] | taint.cpp:673:7:673:7 | s | |
| vector.cpp:16:43:16:49 | source1 | vector.cpp:17:26:17:32 | source1 | |
| vector.cpp:16:43:16:49 | source1 | vector.cpp:31:38:31:44 | source1 | |
@@ -7076,14 +7083,20 @@
| vector.cpp:198:3:198:4 | ee [post update] | vector.cpp:200:3:200:4 | ee | |
| vector.cpp:198:3:198:4 | ee [post update] | vector.cpp:201:8:201:9 | ee | |
| vector.cpp:198:3:198:4 | ee [post update] | vector.cpp:202:2:202:2 | ee | |
+| vector.cpp:198:6:198:7 | ref arg vs | vector.cpp:199:11:199:12 | vs | |
+| vector.cpp:198:6:198:7 | ref arg vs | vector.cpp:200:6:200:7 | vs | |
+| vector.cpp:198:6:198:7 | ref arg vs | vector.cpp:201:11:201:12 | vs | |
| vector.cpp:198:19:198:19 | 0 | vector.cpp:198:6:198:7 | ref arg vs | TAINT |
| vector.cpp:199:8:199:9 | ee [post update] | vector.cpp:200:3:200:4 | ee | |
| vector.cpp:199:8:199:9 | ee [post update] | vector.cpp:201:8:201:9 | ee | |
| vector.cpp:199:8:199:9 | ee [post update] | vector.cpp:202:2:202:2 | ee | |
+| vector.cpp:199:11:199:12 | ref arg vs | vector.cpp:200:6:200:7 | vs | |
+| vector.cpp:199:11:199:12 | ref arg vs | vector.cpp:201:11:201:12 | vs | |
| vector.cpp:199:11:199:12 | vs | vector.cpp:199:13:199:13 | call to operator[] | TAINT |
| vector.cpp:200:3:200:4 | ee [post update] | vector.cpp:201:8:201:9 | ee | |
| vector.cpp:200:3:200:4 | ee [post update] | vector.cpp:202:2:202:2 | ee | |
| vector.cpp:200:3:200:21 | ... = ... | vector.cpp:200:8:200:8 | call to operator[] [post update] | |
+| vector.cpp:200:6:200:7 | ref arg vs | vector.cpp:201:11:201:12 | vs | |
| vector.cpp:200:6:200:7 | vs | vector.cpp:200:8:200:8 | call to operator[] | TAINT |
| vector.cpp:200:8:200:8 | call to operator[] [post update] | vector.cpp:200:6:200:7 | ref arg vs | TAINT |
| vector.cpp:200:14:200:19 | call to source | vector.cpp:200:3:200:21 | ... = ... | |
diff --git a/java/change-notes/2021-06-02-mvel-injection-query.md b/java/change-notes/2021-06-02-mvel-injection-query.md
new file mode 100644
index 00000000000..53b4634e9fe
--- /dev/null
+++ b/java/change-notes/2021-06-02-mvel-injection-query.md
@@ -0,0 +1,2 @@
+lgtm,codescanning
+* The query "Expression language injection (MVEL) (`java/mvel-expression-injection`) has been promoted from experimental to the main query pack. Its results will now appear by default. This query was originally [submitted as an experimental query by @artem-smotrakov](https://github.com/github/codeql/pull/3329)
\ No newline at end of file
diff --git a/java/ql/src/Security/CWE/CWE-094/MvelExpressionEvaluation.java b/java/ql/src/Security/CWE/CWE-094/MvelExpressionEvaluation.java
new file mode 100644
index 00000000000..b8ec0a4db4d
--- /dev/null
+++ b/java/ql/src/Security/CWE/CWE-094/MvelExpressionEvaluation.java
@@ -0,0 +1,25 @@
+public void evaluate(Socket socket) throws IOException {
+ try (BufferedReader reader = new BufferedReader(
+ new InputStreamReader(socket.getInputStream()))) {
+
+ String expression = reader.readLine();
+ // BAD: the user-provided expression is directly evaluated
+ MVEL.eval(expression);
+ }
+}
+
+public void safeEvaluate(Socket socket) throws IOException {
+ try (BufferedReader reader = new BufferedReader(
+ new InputStreamReader(socket.getInputStream()))) {
+
+ String expression = reader.readLine();
+ // GOOD: the user-provided expression is validated before evaluation
+ validateExpression(expression);
+ MVEL.eval(expression);
+ }
+}
+
+private void validateExpression(String expression) {
+ // Validate that the expression does not contain unexpected code.
+ // For instance, this can be done with allow-lists or deny-lists of code patterns.
+}
\ No newline at end of file
diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjection.qhelp b/java/ql/src/Security/CWE/CWE-094/MvelInjection.qhelp
similarity index 55%
rename from java/ql/src/experimental/Security/CWE/CWE-094/MvelInjection.qhelp
rename to java/ql/src/Security/CWE/CWE-094/MvelInjection.qhelp
index d68d298b5f5..ad303285920 100644
--- a/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjection.qhelp
+++ b/java/ql/src/Security/CWE/CWE-094/MvelInjection.qhelp
@@ -3,11 +3,11 @@
-MVEL is an expression language based on Java-syntax.
-The language offers many features
+MVEL is an expression language based on Java-syntax,
+which offers many features
including invocation of methods available in the JVM.
If a MVEL expression is built using attacker-controlled data,
-and then evaluated, then it may allow the attacker to run arbitrary code.
+and then evaluated, then it may allow attackers to run arbitrary code.
@@ -19,10 +19,12 @@ Including user input in a MVEL expression should be avoided.
-The following example uses untrusted data to build a MVEL expression
-and then runs it in the default powerfull context.
+In the following sample, the first example uses untrusted data to build a MVEL expression
+and then runs it in the default context. In the second example, the untrusted data is
+validated with a custom method that checks that the expression does not contain unexpected code
+before evaluating it.
-
+
@@ -35,4 +37,4 @@ and then runs it in the default powerfull context.
Expression Language Injection.
-
\ No newline at end of file
+
diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjection.ql b/java/ql/src/Security/CWE/CWE-094/MvelInjection.ql
similarity index 88%
rename from java/ql/src/experimental/Security/CWE/CWE-094/MvelInjection.ql
rename to java/ql/src/Security/CWE/CWE-094/MvelInjection.ql
index d32c33c343c..9a65bb5e481 100644
--- a/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjection.ql
+++ b/java/ql/src/Security/CWE/CWE-094/MvelInjection.ql
@@ -11,9 +11,9 @@
*/
import java
-import MvelInjectionLib
+import semmle.code.java.security.MvelInjectionQuery
import DataFlow::PathGraph
-from DataFlow::PathNode source, DataFlow::PathNode sink, MvelInjectionConfig conf
+from DataFlow::PathNode source, DataFlow::PathNode sink, MvelInjectionFlowConfig conf
where conf.hasFlowPath(source, sink)
select sink.getNode(), source, sink, "MVEL injection from $@.", source.getNode(), "this user input"
diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll b/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
deleted file mode 100644
index a6cf891330f..00000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
+++ /dev/null
@@ -1,367 +0,0 @@
-import java
-import semmle.code.java.dataflow.FlowSources
-import semmle.code.java.dataflow.TaintTracking
-
-/**
- * A taint-tracking configuration for unsafe user input
- * that is used to construct and evaluate a MVEL expression.
- */
-class MvelInjectionConfig extends TaintTracking::Configuration {
- MvelInjectionConfig() { this = "MvelInjectionConfig" }
-
- override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
-
- override predicate isSink(DataFlow::Node sink) { sink instanceof MvelEvaluationSink }
-
- override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
- expressionCompilationStep(node1, node2) or
- createExpressionCompilerStep(node1, node2) or
- expressionCompilerCompileStep(node1, node2) or
- createCompiledAccExpressionStep(node1, node2) or
- scriptCompileStep(node1, node2) or
- createMvelCompiledScriptStep(node1, node2) or
- templateCompileStep(node1, node2) or
- createTemplateCompilerStep(node1, node2)
- }
-}
-
-/**
- * A sink for EL injection vulnerabilities via MVEL,
- * i.e. methods that run evaluation of a MVEL expression.
- */
-class MvelEvaluationSink extends DataFlow::ExprNode {
- MvelEvaluationSink() {
- exists(StaticMethodAccess ma, Method m | m = ma.getMethod() |
- (
- m instanceof MvelEvalMethod or
- m instanceof TemplateRuntimeEvaluationMethod
- ) and
- ma.getArgument(0) = asExpr()
- )
- or
- exists(MethodAccess ma, Method m | m = ma.getMethod() |
- m instanceof MvelScriptEngineEvaluationMethod and
- ma.getArgument(0) = asExpr()
- )
- or
- exists(MethodAccess ma, Method m | m = ma.getMethod() |
- (
- m instanceof ExecutableStatementEvaluationMethod or
- m instanceof CompiledExpressionEvaluationMethod or
- m instanceof CompiledAccExpressionEvaluationMethod or
- m instanceof AccessorEvaluationMethod or
- m instanceof CompiledScriptEvaluationMethod or
- m instanceof MvelCompiledScriptEvaluationMethod
- ) and
- ma.getQualifier() = asExpr()
- )
- or
- exists(StaticMethodAccess ma, Method m | m = ma.getMethod() |
- m instanceof MvelRuntimeEvaluationMethod and
- ma.getArgument(1) = asExpr()
- )
- }
-}
-
-/**
- * Holds if `node1` to `node2` is a dataflow step that compiles a MVEL expression
- * by callilng `MVEL.compileExpression(tainted)`.
- */
-predicate expressionCompilationStep(DataFlow::Node node1, DataFlow::Node node2) {
- exists(StaticMethodAccess ma, Method m | ma.getMethod() = m |
- m.getDeclaringType() instanceof MVEL and
- m.hasName("compileExpression") and
- ma.getAnArgument() = node1.asExpr() and
- node2.asExpr() = ma
- )
-}
-
-/**
- * Holds if `node1` to `node2` is a dataflow step creates `ExpressionCompiler`,
- * i.e. `new ExpressionCompiler(tainted)`.
- */
-predicate createExpressionCompilerStep(DataFlow::Node node1, DataFlow::Node node2) {
- exists(ConstructorCall cc |
- cc.getConstructedType() instanceof ExpressionCompiler and
- cc = node2.asExpr() and
- cc.getArgument(0) = node1.asExpr()
- )
-}
-
-/**
- * Holds if `node1` to `node2` is a dataflow step creates `CompiledAccExpression`,
- * i.e. `new CompiledAccExpression(tainted, ...)`.
- */
-predicate createCompiledAccExpressionStep(DataFlow::Node node1, DataFlow::Node node2) {
- exists(ConstructorCall cc |
- cc.getConstructedType() instanceof CompiledAccExpression and
- cc = node2.asExpr() and
- cc.getArgument(0) = node1.asExpr()
- )
-}
-
-/**
- * Holds if `node1` to `node2` is a dataflow step that compiles a MVEL expression
- * by calling `ExpressionCompiler.compile()`.
- */
-predicate expressionCompilerCompileStep(DataFlow::Node node1, DataFlow::Node node2) {
- exists(MethodAccess ma, Method m | ma.getMethod() = m |
- m.getDeclaringType() instanceof ExpressionCompiler and
- m.hasName("compile") and
- ma = node2.asExpr() and
- ma.getQualifier() = node1.asExpr()
- )
-}
-
-/**
- * Holds if `node1` to `node2` is a dataflow step that compiles a script via `MvelScriptEngine`,
- * i.e. `engine.compile(tainted)` or `engine.compiledScript(tainted)`.
- */
-predicate scriptCompileStep(DataFlow::Node node1, DataFlow::Node node2) {
- exists(MethodAccess ma, Method m | ma.getMethod() = m |
- m instanceof MvelScriptEngineCompilationMethod and
- ma = node2.asExpr() and
- ma.getArgument(0) = node1.asExpr()
- )
-}
-
-/**
- * Holds if `node1` to `node2` is a dataflow step creates `MvelCompiledScript`,
- * i.e. `new MvelCompiledScript(engine, tainted)`.
- */
-predicate createMvelCompiledScriptStep(DataFlow::Node node1, DataFlow::Node node2) {
- exists(ConstructorCall cc |
- cc.getConstructedType() instanceof MvelCompiledScript and
- cc = node2.asExpr() and
- cc.getArgument(1) = node1.asExpr()
- )
-}
-
-/**
- * Holds if `node1` to `node2` is a dataflow step creates `TemplateCompiler`,
- * i.e. `new TemplateCompiler(tainted)`.
- */
-predicate createTemplateCompilerStep(DataFlow::Node node1, DataFlow::Node node2) {
- exists(ConstructorCall cc |
- cc.getConstructedType() instanceof TemplateCompiler and
- cc = node2.asExpr() and
- cc.getArgument(0) = node1.asExpr()
- )
-}
-
-/**
- * Holds if `node1` to `node2` is a dataflow step that compiles a script via `TemplateCompiler`,
- * i.e. `compiler.compile()` or `TemplateCompiler.compileTemplate(tainted)`.
- */
-predicate templateCompileStep(DataFlow::Node node1, DataFlow::Node node2) {
- exists(MethodAccess ma, Method m | ma.getMethod() = m |
- m instanceof TemplateCompilerCompileMethod and
- ma.getQualifier() = node1.asExpr() and
- ma = node2.asExpr()
- )
- or
- exists(StaticMethodAccess ma, Method m | ma.getMethod() = m |
- m instanceof TemplateCompilerCompileTemplateMethod and
- ma = node2.asExpr() and
- ma.getArgument(0) = node1.asExpr()
- )
-}
-
-/**
- * Methods in the MVEL class that evaluate a MVEL expression.
- */
-class MvelEvalMethod extends Method {
- MvelEvalMethod() {
- getDeclaringType() instanceof MVEL and
- (
- hasName("eval") or
- hasName("executeExpression") or
- hasName("evalToBoolean") or
- hasName("evalToString") or
- hasName("executeAllExpression") or
- hasName("executeSetExpression")
- )
- }
-}
-
-/**
- * Methods in `MVEL` class that compile a MVEL expression.
- */
-class MvelCompileExpressionMethod extends Method {
- MvelCompileExpressionMethod() {
- getDeclaringType() instanceof MVEL and
- (
- hasName("compileExpression") or
- hasName("compileGetExpression") or
- hasName("compileSetExpression")
- )
- }
-}
-
-/**
- * Methods in `ExecutableStatement` that evaluate a MVEL expression.
- */
-class ExecutableStatementEvaluationMethod extends Method {
- ExecutableStatementEvaluationMethod() {
- getDeclaringType() instanceof ExecutableStatement and
- hasName("getValue")
- }
-}
-
-/**
- * Methods in `CompiledExpression` that evaluate a MVEL expression.
- */
-class CompiledExpressionEvaluationMethod extends Method {
- CompiledExpressionEvaluationMethod() {
- getDeclaringType() instanceof CompiledExpression and
- hasName("getDirectValue")
- }
-}
-
-/**
- * Methods in `CompiledAccExpression` that evaluate a MVEL expression.
- */
-class CompiledAccExpressionEvaluationMethod extends Method {
- CompiledAccExpressionEvaluationMethod() {
- getDeclaringType() instanceof CompiledAccExpression and
- hasName("getValue")
- }
-}
-
-/**
- * Methods in `Accessor` that evaluate a MVEL expression.
- */
-class AccessorEvaluationMethod extends Method {
- AccessorEvaluationMethod() {
- getDeclaringType() instanceof Accessor and
- hasName("getValue")
- }
-}
-
-/**
- * Methods in `MvelScriptEngine` that evaluate a MVEL expression.
- */
-class MvelScriptEngineEvaluationMethod extends Method {
- MvelScriptEngineEvaluationMethod() {
- getDeclaringType() instanceof MvelScriptEngine and
- (hasName("eval") or hasName("evaluate"))
- }
-}
-
-/**
- * Methods in `MvelScriptEngine` that compile a MVEL expression.
- */
-class MvelScriptEngineCompilationMethod extends Method {
- MvelScriptEngineCompilationMethod() {
- getDeclaringType() instanceof MvelScriptEngine and
- (hasName("compile") or hasName("compiledScript"))
- }
-}
-
-/**
- * Methods in `CompiledScript` that evaluate a MVEL expression.
- */
-class CompiledScriptEvaluationMethod extends Method {
- CompiledScriptEvaluationMethod() {
- getDeclaringType() instanceof CompiledScript and
- hasName("eval")
- }
-}
-
-/**
- * Methods in `TemplateRuntime` that evaluate a MVEL template.
- */
-class TemplateRuntimeEvaluationMethod extends Method {
- TemplateRuntimeEvaluationMethod() {
- getDeclaringType() instanceof TemplateRuntime and
- (hasName("eval") or hasName("execute"))
- }
-}
-
-/**
- * `TemplateCompiler.compile()` method compiles a MVEL template.
- */
-class TemplateCompilerCompileMethod extends Method {
- TemplateCompilerCompileMethod() {
- getDeclaringType() instanceof TemplateCompiler and
- hasName("compile")
- }
-}
-
-/**
- * `TemplateCompiler.compileTemplate(tainted)` static method compiles a MVEL template.
- */
-class TemplateCompilerCompileTemplateMethod extends Method {
- TemplateCompilerCompileTemplateMethod() {
- getDeclaringType() instanceof TemplateCompiler and
- hasName("compileTemplate")
- }
-}
-
-/**
- * Methods in `MvelCompiledScript` that evaluate a MVEL expression.
- */
-class MvelCompiledScriptEvaluationMethod extends Method {
- MvelCompiledScriptEvaluationMethod() {
- getDeclaringType() instanceof MvelCompiledScript and
- hasName("eval")
- }
-}
-
-/**
- * Methods in `MVELRuntime` that evaluate a MVEL expression.
- */
-class MvelRuntimeEvaluationMethod extends Method {
- MvelRuntimeEvaluationMethod() {
- getDeclaringType() instanceof MVELRuntime and
- hasName("execute")
- }
-}
-
-class MVEL extends RefType {
- MVEL() { hasQualifiedName("org.mvel2", "MVEL") }
-}
-
-class ExpressionCompiler extends RefType {
- ExpressionCompiler() { hasQualifiedName("org.mvel2.compiler", "ExpressionCompiler") }
-}
-
-class ExecutableStatement extends RefType {
- ExecutableStatement() { hasQualifiedName("org.mvel2.compiler", "ExecutableStatement") }
-}
-
-class CompiledExpression extends RefType {
- CompiledExpression() { hasQualifiedName("org.mvel2.compiler", "CompiledExpression") }
-}
-
-class CompiledAccExpression extends RefType {
- CompiledAccExpression() { hasQualifiedName("org.mvel2.compiler", "CompiledAccExpression") }
-}
-
-class Accessor extends RefType {
- Accessor() { hasQualifiedName("org.mvel2.compiler", "Accessor") }
-}
-
-class CompiledScript extends RefType {
- CompiledScript() { hasQualifiedName("javax.script", "CompiledScript") }
-}
-
-class MvelScriptEngine extends RefType {
- MvelScriptEngine() { hasQualifiedName("org.mvel2.jsr223", "MvelScriptEngine") }
-}
-
-class MvelCompiledScript extends RefType {
- MvelCompiledScript() { hasQualifiedName("org.mvel2.jsr223", "MvelCompiledScript") }
-}
-
-class TemplateRuntime extends RefType {
- TemplateRuntime() { hasQualifiedName("org.mvel2.templates", "TemplateRuntime") }
-}
-
-class TemplateCompiler extends RefType {
- TemplateCompiler() { hasQualifiedName("org.mvel2.templates", "TemplateCompiler") }
-}
-
-class MVELRuntime extends RefType {
- MVELRuntime() { hasQualifiedName("org.mvel2", "MVELRuntime") }
-}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/UnsafeMvelExpressionEvaluation.java b/java/ql/src/experimental/Security/CWE/CWE-094/UnsafeMvelExpressionEvaluation.java
deleted file mode 100644
index 4942bee79f6..00000000000
--- a/java/ql/src/experimental/Security/CWE/CWE-094/UnsafeMvelExpressionEvaluation.java
+++ /dev/null
@@ -1,8 +0,0 @@
-public void evaluate(Socket socket) throws IOException {
- try (BufferedReader reader = new BufferedReader(
- new InputStreamReader(socket.getInputStream()))) {
-
- String expression = reader.readLine();
- MVEL.eval(expression);
- }
-}
\ No newline at end of file
diff --git a/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll b/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll
index 1c8c33c23a1..ccffceee1f5 100644
--- a/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll
+++ b/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll
@@ -98,6 +98,7 @@ private module Frameworks {
private import semmle.code.java.security.InformationLeak
private import semmle.code.java.security.JexlInjectionSinkModels
private import semmle.code.java.security.LdapInjection
+ private import semmle.code.java.security.MvelInjection
private import semmle.code.java.security.OgnlInjection
private import semmle.code.java.security.XPath
private import semmle.code.java.frameworks.android.SQLite
diff --git a/java/ql/src/semmle/code/java/security/MvelInjection.qll b/java/ql/src/semmle/code/java/security/MvelInjection.qll
new file mode 100644
index 00000000000..984641fbd18
--- /dev/null
+++ b/java/ql/src/semmle/code/java/security/MvelInjection.qll
@@ -0,0 +1,233 @@
+/** Provides classes to reason about MVEL injection attacks. */
+
+import java
+private import semmle.code.java.dataflow.DataFlow
+private import semmle.code.java.dataflow.ExternalFlow
+
+/** A data flow sink for unvalidated user input that is used to construct MVEL expressions. */
+abstract class MvelEvaluationSink extends DataFlow::Node { }
+
+/** A sanitizer that prevents MVEL injection attacks. */
+abstract class MvelInjectionSanitizer extends DataFlow::Node { }
+
+/**
+ * A unit class for adding additional taint steps.
+ *
+ * Extend this class to add additional taint steps that should apply to the `MvelInjectionFlowConfig`.
+ */
+class MvelInjectionAdditionalTaintStep extends Unit {
+ /**
+ * Holds if the step from `node1` to `node2` should be considered a taint
+ * step for the `MvelInjectionFlowConfig` configuration.
+ */
+ abstract predicate step(DataFlow::Node n1, DataFlow::Node n2);
+}
+
+/** Default sink for MVEL injection vulnerabilities. */
+private class DefaultMvelEvaluationSink extends MvelEvaluationSink {
+ DefaultMvelEvaluationSink() { sinkNode(this, "mvel") }
+}
+
+private class DefaulMvelEvaluationSinkModel extends SinkModelCsv {
+ override predicate row(string row) {
+ row =
+ [
+ "javax.script;CompiledScript;false;eval;;;Argument[-1];mvel",
+ "org.mvel2;MVEL;false;eval;;;Argument[0];mvel",
+ "org.mvel2;MVEL;false;executeExpression;;;Argument[0];mvel",
+ "org.mvel2;MVEL;false;evalToBoolean;;;Argument[0];mvel",
+ "org.mvel2;MVEL;false;evalToString;;;Argument[0];mvel",
+ "org.mvel2;MVEL;false;executeAllExpression;;;Argument[0];mvel",
+ "org.mvel2;MVEL;false;executeSetExpression;;;Argument[0];mvel",
+ "org.mvel2;MVELRuntime;false;execute;;;Argument[1];mvel",
+ "org.mvel2.templates;TemplateRuntime;false;eval;;;Argument[0];mvel",
+ "org.mvel2.templates;TemplateRuntime;false;execute;;;Argument[0];mvel",
+ "org.mvel2.jsr223;MvelScriptEngine;false;eval;;;Argument[0];mvel",
+ "org.mvel2.jsr223;MvelScriptEngine;false;evaluate;;;Argument[0];mvel",
+ "org.mvel2.jsr223;MvelCompiledScript;false;eval;;;Argument[-1];mvel",
+ "org.mvel2.compiler;ExecutableStatement;false;getValue;;;Argument[-1];mvel",
+ "org.mvel2.compiler;CompiledExpression;false;getDirectValue;;;Argument[-1];mvel",
+ "org.mvel2.compiler;CompiledAccExpression;false;getValue;;;Argument[-1];mvel",
+ "org.mvel2.compiler;Accessor;false;getValue;;;Argument[-1];mvel"
+ ]
+ }
+}
+
+/** A default sanitizer that considers numeric and boolean typed data safe for building MVEL expressions */
+private class DefaultMvelInjectionSanitizer extends MvelInjectionSanitizer {
+ DefaultMvelInjectionSanitizer() {
+ this.getType() instanceof NumericType or this.getType() instanceof BooleanType
+ }
+}
+
+/** A set of additional taint steps to consider when taint tracking MVEL related data flows. */
+private class DefaultMvelInjectionAdditionalTaintStep extends MvelInjectionAdditionalTaintStep {
+ override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
+ expressionCompilationStep(node1, node2) or
+ createExpressionCompilerStep(node1, node2) or
+ expressionCompilerCompileStep(node1, node2) or
+ createCompiledAccExpressionStep(node1, node2) or
+ scriptCompileStep(node1, node2) or
+ createMvelCompiledScriptStep(node1, node2) or
+ templateCompileStep(node1, node2) or
+ createTemplateCompilerStep(node1, node2)
+ }
+}
+
+/**
+ * Holds if `node1` to `node2` is a dataflow step that compiles a MVEL expression
+ * by callilng `MVEL.compileExpression(tainted)`.
+ */
+private predicate expressionCompilationStep(DataFlow::Node node1, DataFlow::Node node2) {
+ exists(StaticMethodAccess ma, Method m | ma.getMethod() = m |
+ m.getDeclaringType() instanceof MVEL and
+ m.hasName("compileExpression") and
+ ma.getAnArgument() = node1.asExpr() and
+ node2.asExpr() = ma
+ )
+}
+
+/**
+ * Holds if `node1` to `node2` is a dataflow step that creates `ExpressionCompiler`
+ * by calling `new ExpressionCompiler(tainted)`.
+ */
+private predicate createExpressionCompilerStep(DataFlow::Node node1, DataFlow::Node node2) {
+ exists(ConstructorCall cc |
+ cc.getConstructedType() instanceof ExpressionCompiler and
+ cc = node2.asExpr() and
+ cc.getArgument(0) = node1.asExpr()
+ )
+}
+
+/**
+ * Holds if `node1` to `node2` is a dataflow step creates `CompiledAccExpression`
+ * by calling `new CompiledAccExpression(tainted, ...)`.
+ */
+private predicate createCompiledAccExpressionStep(DataFlow::Node node1, DataFlow::Node node2) {
+ exists(ConstructorCall cc |
+ cc.getConstructedType() instanceof CompiledAccExpression and
+ cc = node2.asExpr() and
+ cc.getArgument(0) = node1.asExpr()
+ )
+}
+
+/**
+ * Holds if `node1` to `node2` is a dataflow step that compiles a MVEL expression
+ * by calling `ExpressionCompiler.compile()`.
+ */
+private predicate expressionCompilerCompileStep(DataFlow::Node node1, DataFlow::Node node2) {
+ exists(MethodAccess ma, Method m | ma.getMethod() = m |
+ m.getDeclaringType() instanceof ExpressionCompiler and
+ m.hasName("compile") and
+ ma = node2.asExpr() and
+ ma.getQualifier() = node1.asExpr()
+ )
+}
+
+/**
+ * Holds if `node1` to `node2` is a dataflow step that compiles a script via `MvelScriptEngine`
+ * by calling `engine.compile(tainted)` or `engine.compiledScript(tainted)`.
+ */
+private predicate scriptCompileStep(DataFlow::Node node1, DataFlow::Node node2) {
+ exists(MethodAccess ma, Method m | ma.getMethod() = m |
+ m instanceof MvelScriptEngineCompilationMethod and
+ ma = node2.asExpr() and
+ ma.getArgument(0) = node1.asExpr()
+ )
+}
+
+/**
+ * Holds if `node1` to `node2` is a dataflow step that creates `MvelCompiledScript`
+ * by calling `new MvelCompiledScript(engine, tainted)`.
+ */
+private predicate createMvelCompiledScriptStep(DataFlow::Node node1, DataFlow::Node node2) {
+ exists(ConstructorCall cc |
+ cc.getConstructedType() instanceof MvelCompiledScript and
+ cc = node2.asExpr() and
+ cc.getArgument(1) = node1.asExpr()
+ )
+}
+
+/**
+ * Holds if `node1` to `node2` is a dataflow step creates `TemplateCompiler`
+ * by calling `new TemplateCompiler(tainted)`.
+ */
+private predicate createTemplateCompilerStep(DataFlow::Node node1, DataFlow::Node node2) {
+ exists(ConstructorCall cc |
+ cc.getConstructedType() instanceof TemplateCompiler and
+ cc = node2.asExpr() and
+ cc.getArgument(0) = node1.asExpr()
+ )
+}
+
+/**
+ * Holds if `node1` to `node2` is a dataflow step that compiles a script via `TemplateCompiler`
+ * by calling `compiler.compile()` or `TemplateCompiler.compileTemplate(tainted)`.
+ */
+private predicate templateCompileStep(DataFlow::Node node1, DataFlow::Node node2) {
+ exists(MethodAccess ma, Method m | ma.getMethod() = m |
+ m instanceof TemplateCompilerCompileMethod and
+ ma.getQualifier() = node1.asExpr() and
+ ma = node2.asExpr()
+ )
+ or
+ exists(StaticMethodAccess ma, Method m | ma.getMethod() = m |
+ m instanceof TemplateCompilerCompileTemplateMethod and
+ ma = node2.asExpr() and
+ ma.getArgument(0) = node1.asExpr()
+ )
+}
+
+/**
+ * Methods in `MvelScriptEngine` that compile a MVEL expression.
+ */
+private class MvelScriptEngineCompilationMethod extends Method {
+ MvelScriptEngineCompilationMethod() {
+ getDeclaringType() instanceof MvelScriptEngine and
+ hasName(["compile", "compiledScript"])
+ }
+}
+
+/**
+ * `TemplateCompiler.compile()` method that compiles a MVEL template.
+ */
+private class TemplateCompilerCompileMethod extends Method {
+ TemplateCompilerCompileMethod() {
+ getDeclaringType() instanceof TemplateCompiler and
+ hasName("compile")
+ }
+}
+
+/**
+ * `TemplateCompiler.compileTemplate(tainted)` static method that compiles a MVEL template.
+ */
+private class TemplateCompilerCompileTemplateMethod extends Method {
+ TemplateCompilerCompileTemplateMethod() {
+ getDeclaringType() instanceof TemplateCompiler and
+ hasName("compileTemplate")
+ }
+}
+
+private class MVEL extends RefType {
+ MVEL() { hasQualifiedName("org.mvel2", "MVEL") }
+}
+
+private class ExpressionCompiler extends RefType {
+ ExpressionCompiler() { hasQualifiedName("org.mvel2.compiler", "ExpressionCompiler") }
+}
+
+private class CompiledAccExpression extends RefType {
+ CompiledAccExpression() { hasQualifiedName("org.mvel2.compiler", "CompiledAccExpression") }
+}
+
+private class MvelScriptEngine extends RefType {
+ MvelScriptEngine() { hasQualifiedName("org.mvel2.jsr223", "MvelScriptEngine") }
+}
+
+private class MvelCompiledScript extends RefType {
+ MvelCompiledScript() { hasQualifiedName("org.mvel2.jsr223", "MvelCompiledScript") }
+}
+
+private class TemplateCompiler extends RefType {
+ TemplateCompiler() { hasQualifiedName("org.mvel2.templates", "TemplateCompiler") }
+}
diff --git a/java/ql/src/semmle/code/java/security/MvelInjectionQuery.qll b/java/ql/src/semmle/code/java/security/MvelInjectionQuery.qll
new file mode 100644
index 00000000000..332ea387e6b
--- /dev/null
+++ b/java/ql/src/semmle/code/java/security/MvelInjectionQuery.qll
@@ -0,0 +1,26 @@
+/** Provides taint tracking configurations to be used in MVEL injection related queries. */
+
+import java
+import semmle.code.java.dataflow.FlowSources
+import semmle.code.java.dataflow.TaintTracking
+import semmle.code.java.security.MvelInjection
+
+/**
+ * A taint-tracking configuration for unsafe user input
+ * that is used to construct and evaluate a MVEL expression.
+ */
+class MvelInjectionFlowConfig extends TaintTracking::Configuration {
+ MvelInjectionFlowConfig() { this = "MvelInjectionFlowConfig" }
+
+ override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+ override predicate isSink(DataFlow::Node sink) { sink instanceof MvelEvaluationSink }
+
+ override predicate isSanitizer(DataFlow::Node sanitizer) {
+ sanitizer instanceof MvelInjectionSanitizer
+ }
+
+ override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
+ any(MvelInjectionAdditionalTaintStep c).step(node1, node2)
+ }
+}
diff --git a/java/ql/test/experimental/query-tests/security/CWE-094/MvelInjection.expected b/java/ql/test/experimental/query-tests/security/CWE-094/MvelInjection.expected
deleted file mode 100644
index 545da956f8e..00000000000
--- a/java/ql/test/experimental/query-tests/security/CWE-094/MvelInjection.expected
+++ /dev/null
@@ -1,67 +0,0 @@
-edges
-| MvelInjection.java:29:54:29:65 | read(...) : String | MvelInjection.java:30:28:30:37 | expression |
-| MvelInjection.java:34:58:34:69 | read(...) : String | MvelInjection.java:36:5:36:13 | statement |
-| MvelInjection.java:34:58:34:69 | read(...) : String | MvelInjection.java:37:5:37:13 | statement |
-| MvelInjection.java:41:58:41:69 | read(...) : String | MvelInjection.java:43:5:43:14 | expression |
-| MvelInjection.java:48:7:48:18 | read(...) : String | MvelInjection.java:49:5:49:14 | expression |
-| MvelInjection.java:53:20:53:31 | read(...) : String | MvelInjection.java:57:5:57:18 | compiledScript |
-| MvelInjection.java:53:20:53:31 | read(...) : String | MvelInjection.java:60:21:60:26 | script |
-| MvelInjection.java:65:58:65:69 | read(...) : String | MvelInjection.java:68:5:68:10 | script |
-| MvelInjection.java:77:40:77:51 | read(...) : String | MvelInjection.java:77:7:77:52 | compileTemplate(...) |
-| MvelInjection.java:81:54:81:65 | read(...) : String | MvelInjection.java:82:29:82:46 | compile(...) |
-| MvelInjection.java:86:58:86:69 | read(...) : String | MvelInjection.java:88:32:88:41 | expression |
-| MvelInjection.java:92:27:92:49 | getInputStream(...) : InputStream | MvelInjection.java:94:15:94:16 | is : InputStream |
-| MvelInjection.java:94:15:94:16 | is : InputStream | MvelInjection.java:94:23:94:27 | bytes [post update] : byte[] |
-| MvelInjection.java:94:23:94:27 | bytes [post update] : byte[] | MvelInjection.java:95:14:95:36 | new String(...) : String |
-| MvelInjection.java:95:14:95:36 | new String(...) : String | MvelInjection.java:25:15:25:26 | read(...) |
-| MvelInjection.java:95:14:95:36 | new String(...) : String | MvelInjection.java:29:54:29:65 | read(...) : String |
-| MvelInjection.java:95:14:95:36 | new String(...) : String | MvelInjection.java:34:58:34:69 | read(...) : String |
-| MvelInjection.java:95:14:95:36 | new String(...) : String | MvelInjection.java:41:58:41:69 | read(...) : String |
-| MvelInjection.java:95:14:95:36 | new String(...) : String | MvelInjection.java:48:7:48:18 | read(...) : String |
-| MvelInjection.java:95:14:95:36 | new String(...) : String | MvelInjection.java:53:20:53:31 | read(...) : String |
-| MvelInjection.java:95:14:95:36 | new String(...) : String | MvelInjection.java:65:58:65:69 | read(...) : String |
-| MvelInjection.java:95:14:95:36 | new String(...) : String | MvelInjection.java:72:26:72:37 | read(...) |
-| MvelInjection.java:95:14:95:36 | new String(...) : String | MvelInjection.java:77:40:77:51 | read(...) : String |
-| MvelInjection.java:95:14:95:36 | new String(...) : String | MvelInjection.java:81:54:81:65 | read(...) : String |
-| MvelInjection.java:95:14:95:36 | new String(...) : String | MvelInjection.java:86:58:86:69 | read(...) : String |
-nodes
-| MvelInjection.java:25:15:25:26 | read(...) | semmle.label | read(...) |
-| MvelInjection.java:29:54:29:65 | read(...) : String | semmle.label | read(...) : String |
-| MvelInjection.java:30:28:30:37 | expression | semmle.label | expression |
-| MvelInjection.java:34:58:34:69 | read(...) : String | semmle.label | read(...) : String |
-| MvelInjection.java:36:5:36:13 | statement | semmle.label | statement |
-| MvelInjection.java:37:5:37:13 | statement | semmle.label | statement |
-| MvelInjection.java:41:58:41:69 | read(...) : String | semmle.label | read(...) : String |
-| MvelInjection.java:43:5:43:14 | expression | semmle.label | expression |
-| MvelInjection.java:48:7:48:18 | read(...) : String | semmle.label | read(...) : String |
-| MvelInjection.java:49:5:49:14 | expression | semmle.label | expression |
-| MvelInjection.java:53:20:53:31 | read(...) : String | semmle.label | read(...) : String |
-| MvelInjection.java:57:5:57:18 | compiledScript | semmle.label | compiledScript |
-| MvelInjection.java:60:21:60:26 | script | semmle.label | script |
-| MvelInjection.java:65:58:65:69 | read(...) : String | semmle.label | read(...) : String |
-| MvelInjection.java:68:5:68:10 | script | semmle.label | script |
-| MvelInjection.java:72:26:72:37 | read(...) | semmle.label | read(...) |
-| MvelInjection.java:77:7:77:52 | compileTemplate(...) | semmle.label | compileTemplate(...) |
-| MvelInjection.java:77:40:77:51 | read(...) : String | semmle.label | read(...) : String |
-| MvelInjection.java:81:54:81:65 | read(...) : String | semmle.label | read(...) : String |
-| MvelInjection.java:82:29:82:46 | compile(...) | semmle.label | compile(...) |
-| MvelInjection.java:86:58:86:69 | read(...) : String | semmle.label | read(...) : String |
-| MvelInjection.java:88:32:88:41 | expression | semmle.label | expression |
-| MvelInjection.java:92:27:92:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:94:15:94:16 | is : InputStream | semmle.label | is : InputStream |
-| MvelInjection.java:94:23:94:27 | bytes [post update] : byte[] | semmle.label | bytes [post update] : byte[] |
-| MvelInjection.java:95:14:95:36 | new String(...) : String | semmle.label | new String(...) : String |
-#select
-| MvelInjection.java:25:15:25:26 | read(...) | MvelInjection.java:92:27:92:49 | getInputStream(...) : InputStream | MvelInjection.java:25:15:25:26 | read(...) | MVEL injection from $@. | MvelInjection.java:92:27:92:49 | getInputStream(...) | this user input |
-| MvelInjection.java:30:28:30:37 | expression | MvelInjection.java:92:27:92:49 | getInputStream(...) : InputStream | MvelInjection.java:30:28:30:37 | expression | MVEL injection from $@. | MvelInjection.java:92:27:92:49 | getInputStream(...) | this user input |
-| MvelInjection.java:36:5:36:13 | statement | MvelInjection.java:92:27:92:49 | getInputStream(...) : InputStream | MvelInjection.java:36:5:36:13 | statement | MVEL injection from $@. | MvelInjection.java:92:27:92:49 | getInputStream(...) | this user input |
-| MvelInjection.java:37:5:37:13 | statement | MvelInjection.java:92:27:92:49 | getInputStream(...) : InputStream | MvelInjection.java:37:5:37:13 | statement | MVEL injection from $@. | MvelInjection.java:92:27:92:49 | getInputStream(...) | this user input |
-| MvelInjection.java:43:5:43:14 | expression | MvelInjection.java:92:27:92:49 | getInputStream(...) : InputStream | MvelInjection.java:43:5:43:14 | expression | MVEL injection from $@. | MvelInjection.java:92:27:92:49 | getInputStream(...) | this user input |
-| MvelInjection.java:49:5:49:14 | expression | MvelInjection.java:92:27:92:49 | getInputStream(...) : InputStream | MvelInjection.java:49:5:49:14 | expression | MVEL injection from $@. | MvelInjection.java:92:27:92:49 | getInputStream(...) | this user input |
-| MvelInjection.java:57:5:57:18 | compiledScript | MvelInjection.java:92:27:92:49 | getInputStream(...) : InputStream | MvelInjection.java:57:5:57:18 | compiledScript | MVEL injection from $@. | MvelInjection.java:92:27:92:49 | getInputStream(...) | this user input |
-| MvelInjection.java:60:21:60:26 | script | MvelInjection.java:92:27:92:49 | getInputStream(...) : InputStream | MvelInjection.java:60:21:60:26 | script | MVEL injection from $@. | MvelInjection.java:92:27:92:49 | getInputStream(...) | this user input |
-| MvelInjection.java:68:5:68:10 | script | MvelInjection.java:92:27:92:49 | getInputStream(...) : InputStream | MvelInjection.java:68:5:68:10 | script | MVEL injection from $@. | MvelInjection.java:92:27:92:49 | getInputStream(...) | this user input |
-| MvelInjection.java:72:26:72:37 | read(...) | MvelInjection.java:92:27:92:49 | getInputStream(...) : InputStream | MvelInjection.java:72:26:72:37 | read(...) | MVEL injection from $@. | MvelInjection.java:92:27:92:49 | getInputStream(...) | this user input |
-| MvelInjection.java:77:7:77:52 | compileTemplate(...) | MvelInjection.java:92:27:92:49 | getInputStream(...) : InputStream | MvelInjection.java:77:7:77:52 | compileTemplate(...) | MVEL injection from $@. | MvelInjection.java:92:27:92:49 | getInputStream(...) | this user input |
-| MvelInjection.java:82:29:82:46 | compile(...) | MvelInjection.java:92:27:92:49 | getInputStream(...) : InputStream | MvelInjection.java:82:29:82:46 | compile(...) | MVEL injection from $@. | MvelInjection.java:92:27:92:49 | getInputStream(...) | this user input |
-| MvelInjection.java:88:32:88:41 | expression | MvelInjection.java:92:27:92:49 | getInputStream(...) : InputStream | MvelInjection.java:88:32:88:41 | expression | MVEL injection from $@. | MvelInjection.java:92:27:92:49 | getInputStream(...) | this user input |
diff --git a/java/ql/test/experimental/query-tests/security/CWE-094/MvelInjection.qlref b/java/ql/test/experimental/query-tests/security/CWE-094/MvelInjection.qlref
deleted file mode 100644
index 13d7cbd2295..00000000000
--- a/java/ql/test/experimental/query-tests/security/CWE-094/MvelInjection.qlref
+++ /dev/null
@@ -1 +0,0 @@
-experimental/Security/CWE/CWE-094/MvelInjection.ql
\ No newline at end of file
diff --git a/java/ql/test/experimental/query-tests/security/CWE-094/options b/java/ql/test/experimental/query-tests/security/CWE-094/options
index e376236f8a8..647a038cd2f 100644
--- a/java/ql/test/experimental/query-tests/security/CWE-094/options
+++ b/java/ql/test/experimental/query-tests/security/CWE-094/options
@@ -1 +1 @@
-//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.3.8:${testdir}/../../../../stubs/mvel2-2.4.7:${testdir}/../../../../stubs/jsr223-api:${testdir}/../../../../stubs/scriptengine:${testdir}/../../../../stubs/java-ee-el:${testdir}/../../../../stubs/juel-2.2:${testdir}/../../../stubs/groovy-all-3.0.7:${testdir}/../../../../stubs/servlet-api-2.4:${testdir}/../../../../stubs/jython-2.7.2:${testdir}/../../../../experimental/stubs/rhino-1.7.13:${testdir}/../../../../stubs/bsh-2.0b5:${testdir}/../../../../experimental/stubs/jshell
+//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.3.8:${testdir}/../../../../stubs/jsr223-api:${testdir}/../../../../stubs/scriptengine:${testdir}/../../../../stubs/java-ee-el:${testdir}/../../../../stubs/juel-2.2:${testdir}/../../../stubs/groovy-all-3.0.7:${testdir}/../../../../stubs/servlet-api-2.4:${testdir}/../../../../stubs/jython-2.7.2:${testdir}/../../../../experimental/stubs/rhino-1.7.13:${testdir}/../../../../stubs/bsh-2.0b5:${testdir}/../../../../experimental/stubs/jshell
diff --git a/java/ql/test/query-tests/security/CWE-094/MvelInjectionTest.expected b/java/ql/test/query-tests/security/CWE-094/MvelInjectionTest.expected
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/java/ql/test/experimental/query-tests/security/CWE-094/MvelInjection.java b/java/ql/test/query-tests/security/CWE-094/MvelInjectionTest.java
similarity index 78%
rename from java/ql/test/experimental/query-tests/security/CWE-094/MvelInjection.java
rename to java/ql/test/query-tests/security/CWE-094/MvelInjectionTest.java
index 88f9b861cae..4013246eecd 100644
--- a/java/ql/test/experimental/query-tests/security/CWE-094/MvelInjection.java
+++ b/java/ql/test/query-tests/security/CWE-094/MvelInjectionTest.java
@@ -15,38 +15,37 @@ import org.mvel2.compiler.ExpressionCompiler;
import org.mvel2.integration.impl.ImmutableDefaultFactory;
import org.mvel2.jsr223.MvelCompiledScript;
import org.mvel2.jsr223.MvelScriptEngine;
-import org.mvel2.templates.CompiledTemplate;
import org.mvel2.templates.TemplateCompiler;
import org.mvel2.templates.TemplateRuntime;
-public class MvelInjection {
+public class MvelInjectionTest {
public static void testWithMvelEval(Socket socket) throws IOException {
- MVEL.eval(read(socket));
+ MVEL.eval(read(socket)); // $hasMvelInjection
}
public static void testWithMvelCompileAndExecute(Socket socket) throws IOException {
Serializable expression = MVEL.compileExpression(read(socket));
- MVEL.executeExpression(expression);
+ MVEL.executeExpression(expression); // $hasMvelInjection
}
public static void testWithExpressionCompiler(Socket socket) throws IOException {
ExpressionCompiler compiler = new ExpressionCompiler(read(socket));
ExecutableStatement statement = compiler.compile();
- statement.getValue(new Object(), new ImmutableDefaultFactory());
- statement.getValue(new Object(), new Object(), new ImmutableDefaultFactory());
+ statement.getValue(new Object(), new ImmutableDefaultFactory()); // $hasMvelInjection
+ statement.getValue(new Object(), new Object(), new ImmutableDefaultFactory()); // $hasMvelInjection
}
public static void testWithCompiledExpressionGetDirectValue(Socket socket) throws IOException {
ExpressionCompiler compiler = new ExpressionCompiler(read(socket));
CompiledExpression expression = compiler.compile();
- expression.getDirectValue(new Object(), new ImmutableDefaultFactory());
+ expression.getDirectValue(new Object(), new ImmutableDefaultFactory()); // $hasMvelInjection
}
public static void testCompiledAccExpressionGetValue(Socket socket) throws IOException {
- CompiledAccExpression expression = new CompiledAccExpression(
- read(socket).toCharArray(), Object.class, new ParserContext());
- expression.getValue(new Object(), new ImmutableDefaultFactory());
+ CompiledAccExpression expression =
+ new CompiledAccExpression(read(socket).toCharArray(), Object.class, new ParserContext());
+ expression.getValue(new Object(), new ImmutableDefaultFactory()); // $hasMvelInjection
}
public static void testMvelScriptEngineCompileAndEvaluate(Socket socket) throws Exception {
@@ -54,10 +53,10 @@ public class MvelInjection {
MvelScriptEngine engine = new MvelScriptEngine();
CompiledScript compiledScript = engine.compile(input);
- compiledScript.eval();
+ compiledScript.eval(); // $hasMvelInjection
Serializable script = engine.compiledScript(input);
- engine.evaluate(script, new SimpleScriptContext());
+ engine.evaluate(script, new SimpleScriptContext()); // $hasMvelInjection
}
public static void testMvelCompiledScriptCompileAndEvaluate(Socket socket) throws Exception {
@@ -65,27 +64,26 @@ public class MvelInjection {
ExpressionCompiler compiler = new ExpressionCompiler(read(socket));
ExecutableStatement statement = compiler.compile();
MvelCompiledScript script = new MvelCompiledScript(engine, statement);
- script.eval(new SimpleScriptContext());
+ script.eval(new SimpleScriptContext()); // $hasMvelInjection
}
public static void testTemplateRuntimeEval(Socket socket) throws Exception {
- TemplateRuntime.eval(read(socket), new HashMap());
+ TemplateRuntime.eval(read(socket), new HashMap()); // $hasMvelInjection
}
public static void testTemplateRuntimeCompileTemplateAndExecute(Socket socket) throws Exception {
- TemplateRuntime.execute(
- TemplateCompiler.compileTemplate(read(socket)), new HashMap());
+ TemplateRuntime.execute(TemplateCompiler.compileTemplate(read(socket)), new HashMap()); // $hasMvelInjection
}
public static void testTemplateRuntimeCompileAndExecute(Socket socket) throws Exception {
TemplateCompiler compiler = new TemplateCompiler(read(socket));
- TemplateRuntime.execute(compiler.compile(), new HashMap());
+ TemplateRuntime.execute(compiler.compile(), new HashMap()); // $hasMvelInjection
}
public static void testMvelRuntimeExecute(Socket socket) throws Exception {
ExpressionCompiler compiler = new ExpressionCompiler(read(socket));
CompiledExpression expression = compiler.compile();
- MVELRuntime.execute(false, expression, new Object(), new ImmutableDefaultFactory());
+ MVELRuntime.execute(false, expression, new Object(), new ImmutableDefaultFactory()); // $hasMvelInjection
}
public static String read(Socket socket) throws IOException {
diff --git a/java/ql/test/query-tests/security/CWE-094/MvelInjectionTest.ql b/java/ql/test/query-tests/security/CWE-094/MvelInjectionTest.ql
new file mode 100644
index 00000000000..71146acfcf9
--- /dev/null
+++ b/java/ql/test/query-tests/security/CWE-094/MvelInjectionTest.ql
@@ -0,0 +1,22 @@
+import java
+import semmle.code.java.dataflow.TaintTracking
+import semmle.code.java.dataflow.FlowSources
+import semmle.code.java.security.MvelInjectionQuery
+import TestUtilities.InlineExpectationsTest
+
+class HasMvelInjectionTest extends InlineExpectationsTest {
+ HasMvelInjectionTest() { this = "HasMvelInjectionTest" }
+
+ override string getARelevantTag() { result = "hasMvelInjection" }
+
+ override predicate hasActualResult(Location location, string element, string tag, string value) {
+ tag = "hasMvelInjection" and
+ exists(DataFlow::Node src, DataFlow::Node sink, MvelInjectionFlowConfig conf |
+ conf.hasFlow(src, sink)
+ |
+ sink.getLocation() = location and
+ element = sink.toString() and
+ value = ""
+ )
+ }
+}
diff --git a/java/ql/test/query-tests/security/CWE-094/options b/java/ql/test/query-tests/security/CWE-094/options
index d050cd75232..9e890133d0f 100644
--- a/java/ql/test/query-tests/security/CWE-094/options
+++ b/java/ql/test/query-tests/security/CWE-094/options
@@ -1 +1 @@
-//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/validation-api-2.0.1.Final:${testdir}/../../../stubs/springframework-5.3.8:${testdir}/../../../stubs/apache-commons-jexl-2.1.1:${testdir}/../../../stubs/apache-commons-jexl-3.1:${testdir}/../../../stubs/apache-commons-logging-1.2
+//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/validation-api-2.0.1.Final:${testdir}/../../../stubs/springframework-5.3.8:${testdir}/../../../stubs/apache-commons-jexl-2.1.1:${testdir}/../../../stubs/apache-commons-jexl-3.1:${testdir}/../../../stubs/apache-commons-logging-1.2:${testdir}/../../../stubs/mvel2-2.4.7:${testdir}/../../../stubs/scriptengine:${testdir}/../../../stubs/jsr223-api
diff --git a/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/jsr223/MvelCompiledScript.java b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/jsr223/MvelCompiledScript.java
index a4be37ada32..771f8b83345 100644
--- a/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/jsr223/MvelCompiledScript.java
+++ b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/jsr223/MvelCompiledScript.java
@@ -3,9 +3,17 @@ package org.mvel2.jsr223;
import java.io.Serializable;
import javax.script.CompiledScript;
import javax.script.ScriptContext;
+import javax.script.ScriptEngine;
import javax.script.ScriptException;
public class MvelCompiledScript extends CompiledScript {
public MvelCompiledScript(MvelScriptEngine engine, Serializable compiledScript) {}
- public Object eval(ScriptContext context) throws ScriptException { return null; }
-}
\ No newline at end of file
+
+ public Object eval(ScriptContext context) throws ScriptException {
+ return null;
+ }
+
+ public ScriptEngine getEngine() {
+ return null;
+ }
+}