Merge branch 'main' into atorralba/promote-ognl-injection

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) }
 

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 |

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);
+}

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 | ... = ... |  |

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)

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.
+}

java/ql/src/Security/CWE/CWE-094/MvelInjection.qhelp

@@ -3,11 +3,11 @@
 
 <overview>
 <p>
-MVEL is an expression language based on Java-syntax. 
+MVEL is an expression language based on Java-syntax, 
-The language offers many features
+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.
 </p>
 </overview>
 
@@ -19,10 +19,12 @@ Including user input in a MVEL expression should be avoided.
 
 <example>
 <p>
-The following example uses untrusted data to build a MVEL expression
+In the following sample, the first example uses untrusted data to build a MVEL expression
-and then runs it in the default powerfull context.
+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.
 </p>
-<sample src="UnsafeMvelExpressionEvaluation.java" />
+<sample src="MvelExpressionEvaluation.java" />
 </example>
 
 <references>
@@ -35,4 +37,4 @@ and then runs it in the default powerfull context.
   <a href="https://owasp.org/www-community/vulnerabilities/Expression_Language_Injection">Expression Language Injection</a>.
 </li>
 </references>
-</qhelp>
+</qhelp>

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"

java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll

@@ -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") }
-}

java/ql/src/experimental/Security/CWE/CWE-094/UnsafeMvelExpressionEvaluation.java

@@ -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);
-  }
-}

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

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") }
+}

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)
+  }
+}

java/ql/test/experimental/query-tests/security/CWE-094/MvelInjection.expected

@@ -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 |

java/ql/test/experimental/query-tests/security/CWE-094/MvelInjection.qlref

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

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

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 ImmutableDefaultFactory()); // $hasMvelInjection
-    statement.getValue(new Object(), new Object(), new ImmutableDefaultFactory());
+    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(
+    CompiledAccExpression expression =
-      read(socket).toCharArray(), Object.class, new ParserContext());
+        new CompiledAccExpression(read(socket).toCharArray(), Object.class, new ParserContext());
-    expression.getValue(new Object(), new ImmutableDefaultFactory());
+    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(
+    TemplateRuntime.execute(TemplateCompiler.compileTemplate(read(socket)), new HashMap()); // $hasMvelInjection
-      TemplateCompiler.compileTemplate(read(socket)), new HashMap());
   }
 
   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 {

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 = ""
+    )
+  }
+}

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

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; }
+
-}
+  public Object eval(ScriptContext context) throws ScriptException {
+    return null;
+  }
+
+  public ScriptEngine getEngine() {
+    return null;
+  }
+}