From c6c4c2c99b91e0d04b2b00acd24a28665c7154c3 Mon Sep 17 00:00:00 2001
From: Artem Smotrakov <artem.smotrakov@gmail.com>
Date: Thu, 23 Apr 2020 09:09:28 +0200
Subject: [PATCH 01/10] Java: Add a query for MVEL injections

- Added experimental/Security/CWE/CWE-094/MvelInjection.ql
- Added experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
- Added a qhelp file with an example of vulnerable code
- Added tests and stubs for mvel2-2.4.7
---
 .../Security/CWE/CWE-094/MvelInjection.qhelp  |  37 +++++
 .../Security/CWE/CWE-094/MvelInjection.ql     |  19 +++
 .../Security/CWE/CWE-094/MvelInjectionLib.qll | 131 ++++++++++++++++++
 .../UnsafeMvelExpressionEvaluation.java       |   8 ++
 .../CWE/CWE-094/MvelInjection.expected        |  15 ++
 .../Security/CWE/CWE-094/MvelInjection.java   |  41 ++++++
 .../Security/CWE/CWE-094/MvelInjection.qlref  |   1 +
 .../experimental/Security/CWE/CWE-094/options |   2 +-
 .../stubs/mvel2-2.4.7/org/mvel2/MVEL.java     |   9 ++
 .../mvel2/compiler/ExecutableStatement.java   |   7 +
 .../mvel2/compiler/ExpressionCompiler.java    |   6 +
 .../integration/VariableResolverFactory.java  |   5 +
 .../impl/ImmutableDefaultFactory.java         |   5 +
 13 files changed, 285 insertions(+), 1 deletion(-)
 create mode 100644 java/ql/src/experimental/Security/CWE/CWE-094/MvelInjection.qhelp
 create mode 100644 java/ql/src/experimental/Security/CWE/CWE-094/MvelInjection.ql
 create mode 100644 java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
 create mode 100644 java/ql/src/experimental/Security/CWE/CWE-094/UnsafeMvelExpressionEvaluation.java
 create mode 100644 java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected
 create mode 100644 java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java
 create mode 100644 java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.qlref
 create mode 100644 java/ql/test/stubs/mvel2-2.4.7/org/mvel2/MVEL.java
 create mode 100644 java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/ExecutableStatement.java
 create mode 100644 java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/ExpressionCompiler.java
 create mode 100644 java/ql/test/stubs/mvel2-2.4.7/org/mvel2/integration/VariableResolverFactory.java
 create mode 100644 java/ql/test/stubs/mvel2-2.4.7/org/mvel2/integration/impl/ImmutableDefaultFactory.java

diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjection.qhelp b/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjection.qhelp
new file mode 100644
index 000000000000..1055847020f6
--- /dev/null
+++ b/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjection.qhelp
@@ -0,0 +1,37 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+MVEL is an expression language based on Java-syntax. 
+The language 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.
+</p>
+</overview>
+
+<recommendation>
+<p>
+Including user input in a MVEL expression should be avoided.
+</p>
+</recommendation>
+
+<example>
+<p>
+The following example uses untrusted data to build a MVEL expression
+and then runs it in the default powerfull context.
+</p>
+<sample src="UnsafeMvelExpressionEvaluation.java" />
+
+<references>
+<li>
+  MVEL Documentation:
+  <a href="http://mvel.documentnode.com/">Language Guide for 2.0</a>.
+</li>
+<li>
+  OWASP:
+  <a href="https://owasp.org/www-community/vulnerabilities/Expression_Language_Injection">Expression Language Injection</a>.
+</li>
+</references>
+</qhelp>
\ No newline at end of file
diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjection.ql b/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjection.ql
new file mode 100644
index 000000000000..d32c33c343c3
--- /dev/null
+++ b/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjection.ql
@@ -0,0 +1,19 @@
+/**
+ * @name Expression language injection (MVEL)
+ * @description Evaluation of a user-controlled MVEL expression
+ *              may lead to remote code execution.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id java/mvel-expression-injection
+ * @tags security
+ *       external/cwe/cwe-094
+ */
+
+import java
+import MvelInjectionLib
+import DataFlow::PathGraph
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, MvelInjectionConfig 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
new file mode 100644
index 000000000000..be457d9534ec
--- /dev/null
+++ b/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
@@ -0,0 +1,131 @@
+import java
+import semmle.code.java.dataflow.FlowSources
+import semmle.code.java.dataflow.TaintTracking
+import DataFlow::PathGraph
+
+/**
+ * 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)
+  }
+}
+
+/**
+ * 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 and
+      ma.getAnArgument() = asExpr()
+    )
+    or
+    exists(MethodAccess ma, Method m | m = ma.getMethod() |
+      m instanceof ExecutableStatementEvaluationMethod and
+      (ma = asExpr() or ma.getQualifier() = 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.getQualifier() or 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() or cc.getQualifier() = 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()
+  )
+}
+
+/**
+ * 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 trigger evaluating a MVEL expression.
+ */
+class ExecutableStatementEvaluationMethod extends Method {
+  ExecutableStatementEvaluationMethod() {
+    getDeclaringType() instanceof ExecutableStatement and
+    hasName("getValue")
+  }
+}
+
+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") }
+}
diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/UnsafeMvelExpressionEvaluation.java b/java/ql/src/experimental/Security/CWE/CWE-094/UnsafeMvelExpressionEvaluation.java
new file mode 100644
index 000000000000..4942bee79f6d
--- /dev/null
+++ b/java/ql/src/experimental/Security/CWE/CWE-094/UnsafeMvelExpressionEvaluation.java
@@ -0,0 +1,8 @@
+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/test/experimental/Security/CWE/CWE-094/MvelInjection.expected b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected
new file mode 100644
index 000000000000..837cee1e3a1e
--- /dev/null
+++ b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected
@@ -0,0 +1,15 @@
+edges
+| MvelInjection.java:13:27:13:49 | getInputStream(...) : InputStream | MvelInjection.java:17:17:17:21 | input |
+| MvelInjection.java:22:27:22:49 | getInputStream(...) : InputStream | MvelInjection.java:27:30:27:39 | expression |
+| MvelInjection.java:32:27:32:49 | getInputStream(...) : InputStream | MvelInjection.java:38:7:38:15 | statement |
+nodes
+| MvelInjection.java:13:27:13:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:17:17:17:21 | input | semmle.label | input |
+| MvelInjection.java:22:27:22:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:27:30:27:39 | expression | semmle.label | expression |
+| MvelInjection.java:32:27:32:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:38:7:38:15 | statement | semmle.label | statement |
+#select
+| MvelInjection.java:17:17:17:21 | input | MvelInjection.java:13:27:13:49 | getInputStream(...) : InputStream | MvelInjection.java:17:17:17:21 | input | MVEL injection from $@. | MvelInjection.java:13:27:13:49 | getInputStream(...) | this user input |
+| MvelInjection.java:27:30:27:39 | expression | MvelInjection.java:22:27:22:49 | getInputStream(...) : InputStream | MvelInjection.java:27:30:27:39 | expression | MVEL injection from $@. | MvelInjection.java:22:27:22:49 | getInputStream(...) | this user input |
+| MvelInjection.java:38:7:38:15 | statement | MvelInjection.java:32:27:32:49 | getInputStream(...) : InputStream | MvelInjection.java:38:7:38:15 | statement | MVEL injection from $@. | MvelInjection.java:32:27:32:49 | getInputStream(...) | this user input |
\ No newline at end of file
diff --git a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java
new file mode 100644
index 000000000000..6c5ceb957b66
--- /dev/null
+++ b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java
@@ -0,0 +1,41 @@
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.Serializable;
+import java.net.Socket;
+import org.mvel2.MVEL;
+import org.mvel2.compiler.ExecutableStatement;
+import org.mvel2.compiler.ExpressionCompiler;
+import org.mvel2.integration.impl.ImmutableDefaultFactory;
+
+public class MvelInjection {
+
+  public static void testWithMvelEval(Socket socket) throws IOException {
+    try (InputStream in = socket.getInputStream()) {
+      byte[] bytes = new byte[1024];
+      int n = in.read(bytes);
+      String input = new String(bytes, 0, n);
+      MVEL.eval(input);
+    }
+  }
+
+  public static void testWithMvelCompileAndExecute(Socket socket) throws IOException {
+    try (InputStream in = socket.getInputStream()) {
+      byte[] bytes = new byte[1024];
+      int n = in.read(bytes);
+      String input = new String(bytes, 0, n);
+      Serializable expression = MVEL.compileExpression(input);
+      MVEL.executeExpression(expression);
+    }
+  }
+
+  public static void testWithExpressionCompiler(Socket socket) throws IOException {
+    try (InputStream in = socket.getInputStream()) {
+      byte[] bytes = new byte[1024];
+      int n = in.read(bytes);
+      String input = new String(bytes, 0, n);
+      ExpressionCompiler compiler = new ExpressionCompiler(input);
+      ExecutableStatement statement = compiler.compile();
+      statement.getValue(new Object(), new ImmutableDefaultFactory());
+    }
+  }
+}
diff --git a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.qlref b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.qlref
new file mode 100644
index 000000000000..13d7cbd22950
--- /dev/null
+++ b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.qlref
@@ -0,0 +1 @@
+experimental/Security/CWE/CWE-094/MvelInjection.ql
\ No newline at end of file
diff --git a/java/ql/test/experimental/Security/CWE/CWE-094/options b/java/ql/test/experimental/Security/CWE/CWE-094/options
index 31b8e3f69351..2048ced65b40 100644
--- a/java/ql/test/experimental/Security/CWE/CWE-094/options
+++ b/java/ql/test/experimental/Security/CWE/CWE-094/options
@@ -1 +1 @@
-//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.2.3
\ No newline at end of file
+//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.2.3:${testdir}/../../../../stubs/mvel2-2.4.7
diff --git a/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/MVEL.java b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/MVEL.java
new file mode 100644
index 000000000000..3d0c39e724ff
--- /dev/null
+++ b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/MVEL.java
@@ -0,0 +1,9 @@
+package org.mvel2;
+
+import java.io.Serializable;
+
+public class MVEL {
+  public static Object eval(String expression) { return null; }
+  public static Serializable compileExpression(String expression) { return null; }
+  public static Object executeExpression(Object compiledExpression) { return null; }
+}
\ No newline at end of file
diff --git a/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/ExecutableStatement.java b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/ExecutableStatement.java
new file mode 100644
index 000000000000..cdd510035fe5
--- /dev/null
+++ b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/ExecutableStatement.java
@@ -0,0 +1,7 @@
+package org.mvel2.compiler;
+
+import org.mvel2.integration.VariableResolverFactory;
+
+public interface ExecutableStatement {
+    public Object getValue(Object staticContext, VariableResolverFactory factory);
+}
\ No newline at end of file
diff --git a/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/ExpressionCompiler.java b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/ExpressionCompiler.java
new file mode 100644
index 000000000000..204bd5e283fd
--- /dev/null
+++ b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/ExpressionCompiler.java
@@ -0,0 +1,6 @@
+package org.mvel2.compiler;
+
+public class ExpressionCompiler {
+  public ExpressionCompiler(String expression) {}
+  public ExecutableStatement compile() { return null; }
+}
\ No newline at end of file
diff --git a/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/integration/VariableResolverFactory.java b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/integration/VariableResolverFactory.java
new file mode 100644
index 000000000000..1fb643374b68
--- /dev/null
+++ b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/integration/VariableResolverFactory.java
@@ -0,0 +1,5 @@
+package org.mvel2.integration;
+
+import java.io.Serializable;
+
+public interface VariableResolverFactory extends Serializable {}
\ No newline at end of file
diff --git a/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/integration/impl/ImmutableDefaultFactory.java b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/integration/impl/ImmutableDefaultFactory.java
new file mode 100644
index 000000000000..97463a9e9d3d
--- /dev/null
+++ b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/integration/impl/ImmutableDefaultFactory.java
@@ -0,0 +1,5 @@
+package org.mvel2.integration.impl;
+
+import org.mvel2.integration.VariableResolverFactory;
+
+public class ImmutableDefaultFactory implements VariableResolverFactory {}
\ No newline at end of file

From 32ff5ad4967efdfffc48be28181fbe75355daeaa Mon Sep 17 00:00:00 2001
From: Artem Smotrakov <artem.smotrakov@gmail.com>
Date: Fri, 24 Apr 2020 22:06:20 +0200
Subject: [PATCH 02/10] Java: Added CompiledExpression sink for MVEL injections

---
 .../Security/CWE/CWE-094/MvelInjectionLib.qll | 19 +++++++++++++
 .../CWE/CWE-094/MvelInjection.expected        | 28 +++++++++++--------
 .../Security/CWE/CWE-094/MvelInjection.java   | 12 ++++++++
 .../mvel2/compiler/CompiledExpression.java    |  8 ++++++
 .../mvel2/compiler/ExecutableStatement.java   |  2 +-
 .../mvel2/compiler/ExpressionCompiler.java    |  2 +-
 6 files changed, 57 insertions(+), 14 deletions(-)
 create mode 100644 java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/CompiledExpression.java

diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll b/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
index be457d9534ec..04833b8537e8 100644
--- a/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
+++ b/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
@@ -36,6 +36,11 @@ class MvelEvaluationSink extends DataFlow::ExprNode {
       m instanceof ExecutableStatementEvaluationMethod and
       (ma = asExpr() or ma.getQualifier() = asExpr())
     )
+    or
+    exists(MethodAccess ma, Method m | m = ma.getMethod() |
+      m instanceof CompiledExpressionEvaluationMethod and
+      (ma = asExpr() or ma.getQualifier() = asExpr())
+    )
   }
 }
 
@@ -118,6 +123,16 @@ class ExecutableStatementEvaluationMethod extends Method {
   }
 }
 
+/**
+ * Methods in `CompiledExpression` that trigger evaluating a MVEL expression.
+ */
+class CompiledExpressionEvaluationMethod extends Method {
+  CompiledExpressionEvaluationMethod() {
+    getDeclaringType() instanceof CompiledExpression and
+    hasName("getDirectValue")
+  }
+}
+
 class MVEL extends RefType {
   MVEL() { hasQualifiedName("org.mvel2", "MVEL") }
 }
@@ -129,3 +144,7 @@ class ExpressionCompiler extends RefType {
 class ExecutableStatement extends RefType {
   ExecutableStatement() { hasQualifiedName("org.mvel2.compiler", "ExecutableStatement") }
 }
+
+class CompiledExpression extends RefType {
+    CompiledExpression() { hasQualifiedName("org.mvel2.compiler", "CompiledExpression") }
+}
diff --git a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected
index 837cee1e3a1e..399ab3fe6acf 100644
--- a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected
+++ b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected
@@ -1,15 +1,19 @@
 edges
-| MvelInjection.java:13:27:13:49 | getInputStream(...) : InputStream | MvelInjection.java:17:17:17:21 | input |
-| MvelInjection.java:22:27:22:49 | getInputStream(...) : InputStream | MvelInjection.java:27:30:27:39 | expression |
-| MvelInjection.java:32:27:32:49 | getInputStream(...) : InputStream | MvelInjection.java:38:7:38:15 | statement |
+| MvelInjection.java:14:27:14:49 | getInputStream(...) : InputStream | MvelInjection.java:18:17:18:21 | input |
+| MvelInjection.java:23:27:23:49 | getInputStream(...) : InputStream | MvelInjection.java:28:30:28:39 | expression |
+| MvelInjection.java:33:27:33:49 | getInputStream(...) : InputStream | MvelInjection.java:39:7:39:15 | statement |
+| MvelInjection.java:44:27:44:49 | getInputStream(...) : InputStream | MvelInjection.java:50:7:50:16 | expression |
 nodes
-| MvelInjection.java:13:27:13:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:17:17:17:21 | input | semmle.label | input |
-| MvelInjection.java:22:27:22:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:27:30:27:39 | expression | semmle.label | expression |
-| MvelInjection.java:32:27:32:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:38:7:38:15 | statement | semmle.label | statement |
+| MvelInjection.java:14:27:14:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:18:17:18:21 | input | semmle.label | input |
+| MvelInjection.java:23:27:23:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:28:30:28:39 | expression | semmle.label | expression |
+| MvelInjection.java:33:27:33:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:39:7:39:15 | statement | semmle.label | statement |
+| MvelInjection.java:44:27:44:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:50:7:50:16 | expression | semmle.label | expression |
 #select
-| MvelInjection.java:17:17:17:21 | input | MvelInjection.java:13:27:13:49 | getInputStream(...) : InputStream | MvelInjection.java:17:17:17:21 | input | MVEL injection from $@. | MvelInjection.java:13:27:13:49 | getInputStream(...) | this user input |
-| MvelInjection.java:27:30:27:39 | expression | MvelInjection.java:22:27:22:49 | getInputStream(...) : InputStream | MvelInjection.java:27:30:27:39 | expression | MVEL injection from $@. | MvelInjection.java:22:27:22:49 | getInputStream(...) | this user input |
-| MvelInjection.java:38:7:38:15 | statement | MvelInjection.java:32:27:32:49 | getInputStream(...) : InputStream | MvelInjection.java:38:7:38:15 | statement | MVEL injection from $@. | MvelInjection.java:32:27:32:49 | getInputStream(...) | this user input |
\ No newline at end of file
+| MvelInjection.java:18:17:18:21 | input | MvelInjection.java:14:27:14:49 | getInputStream(...) : InputStream | MvelInjection.java:18:17:18:21 | input | MVEL injection from $@. | MvelInjection.java:14:27:14:49 | getInputStream(...) | this user input |
+| MvelInjection.java:28:30:28:39 | expression | MvelInjection.java:23:27:23:49 | getInputStream(...) : InputStream | MvelInjection.java:28:30:28:39 | expression | MVEL injection from $@. | MvelInjection.java:23:27:23:49 | getInputStream(...) | this user input |
+| MvelInjection.java:39:7:39:15 | statement | MvelInjection.java:33:27:33:49 | getInputStream(...) : InputStream | MvelInjection.java:39:7:39:15 | statement | MVEL injection from $@. | MvelInjection.java:33:27:33:49 | getInputStream(...) | this user input |
+| MvelInjection.java:50:7:50:16 | expression | MvelInjection.java:44:27:44:49 | getInputStream(...) : InputStream | MvelInjection.java:50:7:50:16 | expression | MVEL injection from $@. | MvelInjection.java:44:27:44:49 | getInputStream(...) | this user input |
diff --git a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java
index 6c5ceb957b66..c5199b272c46 100644
--- a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java
+++ b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java
@@ -3,6 +3,7 @@
 import java.io.Serializable;
 import java.net.Socket;
 import org.mvel2.MVEL;
+import org.mvel2.compiler.CompiledExpression;
 import org.mvel2.compiler.ExecutableStatement;
 import org.mvel2.compiler.ExpressionCompiler;
 import org.mvel2.integration.impl.ImmutableDefaultFactory;
@@ -38,4 +39,15 @@ public static void testWithExpressionCompiler(Socket socket) throws IOException
       statement.getValue(new Object(), new ImmutableDefaultFactory());
     }
   }
+
+  public static void testWithCompiledExpressionGetDirectValue(Socket socket) throws IOException {
+    try (InputStream in = socket.getInputStream()) {
+      byte[] bytes = new byte[1024];
+      int n = in.read(bytes);
+      String input = new String(bytes, 0, n);
+      ExpressionCompiler compiler = new ExpressionCompiler(input);
+      CompiledExpression expression = compiler.compile();
+      expression.getDirectValue(new Object(), new ImmutableDefaultFactory());
+    }
+  }
 }
diff --git a/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/CompiledExpression.java b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/CompiledExpression.java
new file mode 100644
index 000000000000..234900b0fa32
--- /dev/null
+++ b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/CompiledExpression.java
@@ -0,0 +1,8 @@
+package org.mvel2.compiler;
+
+import org.mvel2.integration.VariableResolverFactory;
+
+public class CompiledExpression implements ExecutableStatement {
+  public Object getDirectValue(Object staticContext, VariableResolverFactory factory) { return null; }
+  public Object getValue(Object staticContext, VariableResolverFactory factory) { return null; }
+}
\ No newline at end of file
diff --git a/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/ExecutableStatement.java b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/ExecutableStatement.java
index cdd510035fe5..1e15b8e6678e 100644
--- a/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/ExecutableStatement.java
+++ b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/ExecutableStatement.java
@@ -3,5 +3,5 @@
 import org.mvel2.integration.VariableResolverFactory;
 
 public interface ExecutableStatement {
-    public Object getValue(Object staticContext, VariableResolverFactory factory);
+  public Object getValue(Object staticContext, VariableResolverFactory factory);
 }
\ No newline at end of file
diff --git a/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/ExpressionCompiler.java b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/ExpressionCompiler.java
index 204bd5e283fd..3f22268b891b 100644
--- a/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/ExpressionCompiler.java
+++ b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/ExpressionCompiler.java
@@ -2,5 +2,5 @@
 
 public class ExpressionCompiler {
   public ExpressionCompiler(String expression) {}
-  public ExecutableStatement compile() { return null; }
+  public CompiledExpression compile() { return null; }
 }
\ No newline at end of file

From 12e0234d4094205ee33c5dcafad38d6434d83728 Mon Sep 17 00:00:00 2001
From: Artem Smotrakov <artem.smotrakov@gmail.com>
Date: Fri, 24 Apr 2020 23:17:52 +0200
Subject: [PATCH 03/10] Java: Added CompiledAccExpression sink for MVEL
 injections

---
 .../Security/CWE/CWE-094/MvelInjectionLib.qll | 46 +++++++++++++++----
 .../CWE/CWE-094/MvelInjection.expected        | 36 ++++++++-------
 .../Security/CWE/CWE-094/MvelInjection.java   | 12 +++++
 .../mvel2-2.4.7/org/mvel2/ParserContext.java  |  3 ++
 .../mvel2/compiler/CompiledAccExpression.java | 10 ++++
 5 files changed, 83 insertions(+), 24 deletions(-)
 create mode 100644 java/ql/test/stubs/mvel2-2.4.7/org/mvel2/ParserContext.java
 create mode 100644 java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/CompiledAccExpression.java

diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll b/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
index 04833b8537e8..1790a2d54b22 100644
--- a/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
+++ b/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
@@ -17,7 +17,8 @@ class MvelInjectionConfig extends TaintTracking::Configuration {
   override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
     expressionCompilationStep(node1, node2) or
     createExpressionCompilerStep(node1, node2) or
-    expressionCompilerCompileStep(node1, node2)
+    expressionCompilerCompileStep(node1, node2) or
+    createCompiledAccExpressionStep(node1, node2)
   }
 }
 
@@ -33,12 +34,11 @@ class MvelEvaluationSink extends DataFlow::ExprNode {
     )
     or
     exists(MethodAccess ma, Method m | m = ma.getMethod() |
-      m instanceof ExecutableStatementEvaluationMethod and
-      (ma = asExpr() or ma.getQualifier() = asExpr())
-    )
-    or
-    exists(MethodAccess ma, Method m | m = ma.getMethod() |
-      m instanceof CompiledExpressionEvaluationMethod and
+      (
+        m instanceof ExecutableStatementEvaluationMethod or
+        m instanceof CompiledExpressionEvaluationMethod or
+        m instanceof CompiledAccExpressionEvaluationMethod
+      ) and
       (ma = asExpr() or ma.getQualifier() = asExpr())
     )
   }
@@ -69,6 +69,22 @@ predicate createExpressionCompilerStep(DataFlow::Node node1, DataFlow::Node node
   )
 }
 
+/**
+ * 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() or cc.getQualifier() = node2.asExpr()) and
+    cc.getArgument(0) = node1.asExpr()
+  )
+}
+
+predicate test() {
+  exists(ConstructorCall cc | cc.getConstructedType() instanceof CompiledAccExpression)
+}
+
 /**
  * Holds if `node1` to `node2` is a dataflow step that compiles a MVEL expression
  * by calling `ExpressionCompiler.compile()`.
@@ -133,6 +149,16 @@ class CompiledExpressionEvaluationMethod extends Method {
   }
 }
 
+/**
+ * Methods in `CompiledAccExpression` that trigger evaluating a MVEL expression.
+ */
+class CompiledAccExpressionEvaluationMethod extends Method {
+  CompiledAccExpressionEvaluationMethod() {
+    getDeclaringType() instanceof CompiledAccExpression and
+    hasName("getValue")
+  }
+}
+
 class MVEL extends RefType {
   MVEL() { hasQualifiedName("org.mvel2", "MVEL") }
 }
@@ -146,5 +172,9 @@ class ExecutableStatement extends RefType {
 }
 
 class CompiledExpression extends RefType {
-    CompiledExpression() { hasQualifiedName("org.mvel2.compiler", "CompiledExpression") }
+  CompiledExpression() { hasQualifiedName("org.mvel2.compiler", "CompiledExpression") }
+}
+
+class CompiledAccExpression extends RefType {
+  CompiledAccExpression() { hasQualifiedName("org.mvel2.compiler", "CompiledAccExpression") }
 }
diff --git a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected
index 399ab3fe6acf..9ce66a171221 100644
--- a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected
+++ b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected
@@ -1,19 +1,23 @@
 edges
-| MvelInjection.java:14:27:14:49 | getInputStream(...) : InputStream | MvelInjection.java:18:17:18:21 | input |
-| MvelInjection.java:23:27:23:49 | getInputStream(...) : InputStream | MvelInjection.java:28:30:28:39 | expression |
-| MvelInjection.java:33:27:33:49 | getInputStream(...) : InputStream | MvelInjection.java:39:7:39:15 | statement |
-| MvelInjection.java:44:27:44:49 | getInputStream(...) : InputStream | MvelInjection.java:50:7:50:16 | expression |
+| MvelInjection.java:16:27:16:49 | getInputStream(...) : InputStream | MvelInjection.java:20:17:20:21 | input |
+| MvelInjection.java:25:27:25:49 | getInputStream(...) : InputStream | MvelInjection.java:30:30:30:39 | expression |
+| MvelInjection.java:35:27:35:49 | getInputStream(...) : InputStream | MvelInjection.java:41:7:41:15 | statement |
+| MvelInjection.java:46:27:46:49 | getInputStream(...) : InputStream | MvelInjection.java:52:7:52:16 | expression |
+| MvelInjection.java:57:27:57:49 | getInputStream(...) : InputStream | MvelInjection.java:62:7:62:16 | expression |
 nodes
-| MvelInjection.java:14:27:14:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:18:17:18:21 | input | semmle.label | input |
-| MvelInjection.java:23:27:23:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:28:30:28:39 | expression | semmle.label | expression |
-| MvelInjection.java:33:27:33:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:39:7:39:15 | statement | semmle.label | statement |
-| MvelInjection.java:44:27:44:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:50:7:50:16 | expression | semmle.label | expression |
+| MvelInjection.java:16:27:16:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:20:17:20:21 | input | semmle.label | input |
+| MvelInjection.java:25:27:25:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:30:30:30:39 | expression | semmle.label | expression |
+| MvelInjection.java:35:27:35:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:41:7:41:15 | statement | semmle.label | statement |
+| MvelInjection.java:46:27:46:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:52:7:52:16 | expression | semmle.label | expression |
+| MvelInjection.java:57:27:57:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:62:7:62:16 | expression | semmle.label | expression |
 #select
-| MvelInjection.java:18:17:18:21 | input | MvelInjection.java:14:27:14:49 | getInputStream(...) : InputStream | MvelInjection.java:18:17:18:21 | input | MVEL injection from $@. | MvelInjection.java:14:27:14:49 | getInputStream(...) | this user input |
-| MvelInjection.java:28:30:28:39 | expression | MvelInjection.java:23:27:23:49 | getInputStream(...) : InputStream | MvelInjection.java:28:30:28:39 | expression | MVEL injection from $@. | MvelInjection.java:23:27:23:49 | getInputStream(...) | this user input |
-| MvelInjection.java:39:7:39:15 | statement | MvelInjection.java:33:27:33:49 | getInputStream(...) : InputStream | MvelInjection.java:39:7:39:15 | statement | MVEL injection from $@. | MvelInjection.java:33:27:33:49 | getInputStream(...) | this user input |
-| MvelInjection.java:50:7:50:16 | expression | MvelInjection.java:44:27:44:49 | getInputStream(...) : InputStream | MvelInjection.java:50:7:50:16 | expression | MVEL injection from $@. | MvelInjection.java:44:27:44:49 | getInputStream(...) | this user input |
+| MvelInjection.java:20:17:20:21 | input | MvelInjection.java:16:27:16:49 | getInputStream(...) : InputStream | MvelInjection.java:20:17:20:21 | input | MVEL injection from $@. | MvelInjection.java:16:27:16:49 | getInputStream(...) | this user input |
+| MvelInjection.java:30:30:30:39 | expression | MvelInjection.java:25:27:25:49 | getInputStream(...) : InputStream | MvelInjection.java:30:30:30:39 | expression | MVEL injection from $@. | MvelInjection.java:25:27:25:49 | getInputStream(...) | this user input |
+| MvelInjection.java:41:7:41:15 | statement | MvelInjection.java:35:27:35:49 | getInputStream(...) : InputStream | MvelInjection.java:41:7:41:15 | statement | MVEL injection from $@. | MvelInjection.java:35:27:35:49 | getInputStream(...) | this user input |
+| MvelInjection.java:52:7:52:16 | expression | MvelInjection.java:46:27:46:49 | getInputStream(...) : InputStream | MvelInjection.java:52:7:52:16 | expression | MVEL injection from $@. | MvelInjection.java:46:27:46:49 | getInputStream(...) | this user input |
+| MvelInjection.java:62:7:62:16 | expression | MvelInjection.java:57:27:57:49 | getInputStream(...) : InputStream | MvelInjection.java:62:7:62:16 | expression | MVEL injection from $@. | MvelInjection.java:57:27:57:49 | getInputStream(...) | this user input |
diff --git a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java
index c5199b272c46..23218e3cfcb2 100644
--- a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java
+++ b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java
@@ -3,6 +3,8 @@
 import java.io.Serializable;
 import java.net.Socket;
 import org.mvel2.MVEL;
+import org.mvel2.ParserContext;
+import org.mvel2.compiler.CompiledAccExpression;
 import org.mvel2.compiler.CompiledExpression;
 import org.mvel2.compiler.ExecutableStatement;
 import org.mvel2.compiler.ExpressionCompiler;
@@ -50,4 +52,14 @@ public static void testWithCompiledExpressionGetDirectValue(Socket socket) throw
       expression.getDirectValue(new Object(), new ImmutableDefaultFactory());
     }
   }
+
+  public static void testCompiledAccExpressionGetValue(Socket socket) throws IOException {
+    try (InputStream in = socket.getInputStream()) {
+      byte[] bytes = new byte[1024];
+      int n = in.read(bytes);
+      String input = new String(bytes, 0, n);
+      CompiledAccExpression expression = new CompiledAccExpression(input.toCharArray(), Object.class, new ParserContext());
+      expression.getValue(new Object(), new ImmutableDefaultFactory());
+    }
+  }
 }
diff --git a/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/ParserContext.java b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/ParserContext.java
new file mode 100644
index 000000000000..ce160eec2288
--- /dev/null
+++ b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/ParserContext.java
@@ -0,0 +1,3 @@
+package org.mvel2;
+
+public class ParserContext {}
\ No newline at end of file
diff --git a/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/CompiledAccExpression.java b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/CompiledAccExpression.java
new file mode 100644
index 000000000000..88630888e67e
--- /dev/null
+++ b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/CompiledAccExpression.java
@@ -0,0 +1,10 @@
+package org.mvel2.compiler;
+
+import org.mvel2.ParserContext;
+import org.mvel2.integration.VariableResolverFactory;
+
+public class CompiledAccExpression implements ExecutableStatement {
+  public CompiledAccExpression(char[] expression, Class ingressType, ParserContext context) {}
+  public Object getValue(Object staticContext, VariableResolverFactory factory) { return null; }
+  public Object getValue(Object ctx, Object elCtx, VariableResolverFactory variableFactory) { return null; }
+}

From 6a6c805048fd09b0030eaddfd06a4dab6d2c63dc Mon Sep 17 00:00:00 2001
From: Artem Smotrakov <artem.smotrakov@gmail.com>
Date: Fri, 24 Apr 2020 23:49:19 +0200
Subject: [PATCH 04/10] Java: Added Accessor sink for MVEL injections

---
 .../Security/CWE/CWE-094/MvelInjectionLib.qll | 17 ++++++++++++++++-
 .../CWE/CWE-094/MvelInjection.expected        | 19 +++++++++++--------
 .../Security/CWE/CWE-094/MvelInjection.java   |  1 +
 .../org/mvel2/compiler/Accessor.java          |  7 +++++++
 .../mvel2/compiler/CompiledExpression.java    |  1 +
 .../mvel2/compiler/ExecutableStatement.java   |  2 +-
 6 files changed, 37 insertions(+), 10 deletions(-)
 create mode 100644 java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/Accessor.java

diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll b/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
index 1790a2d54b22..0e1d6d38fdc0 100644
--- a/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
+++ b/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
@@ -37,7 +37,8 @@ class MvelEvaluationSink extends DataFlow::ExprNode {
       (
         m instanceof ExecutableStatementEvaluationMethod or
         m instanceof CompiledExpressionEvaluationMethod or
-        m instanceof CompiledAccExpressionEvaluationMethod
+        m instanceof CompiledAccExpressionEvaluationMethod or
+        m instanceof AccessorEvaluationMethod
       ) and
       (ma = asExpr() or ma.getQualifier() = asExpr())
     )
@@ -159,6 +160,16 @@ class CompiledAccExpressionEvaluationMethod extends Method {
   }
 }
 
+/**
+ * Methods in `Accessor` that trigger evaluating a MVEL expression.
+ */
+class AccessorEvaluationMethod extends Method {
+  AccessorEvaluationMethod() {
+    getDeclaringType() instanceof Accessor and
+    hasName("getValue")
+  }
+}
+
 class MVEL extends RefType {
   MVEL() { hasQualifiedName("org.mvel2", "MVEL") }
 }
@@ -178,3 +189,7 @@ class CompiledExpression extends RefType {
 class CompiledAccExpression extends RefType {
   CompiledAccExpression() { hasQualifiedName("org.mvel2.compiler", "CompiledAccExpression") }
 }
+
+class Accessor extends RefType {
+  Accessor() { hasQualifiedName("org.mvel2.compiler", "Accessor") }
+}
diff --git a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected
index 9ce66a171221..2cf20b0d79f2 100644
--- a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected
+++ b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected
@@ -2,8 +2,9 @@ edges
 | MvelInjection.java:16:27:16:49 | getInputStream(...) : InputStream | MvelInjection.java:20:17:20:21 | input |
 | MvelInjection.java:25:27:25:49 | getInputStream(...) : InputStream | MvelInjection.java:30:30:30:39 | expression |
 | MvelInjection.java:35:27:35:49 | getInputStream(...) : InputStream | MvelInjection.java:41:7:41:15 | statement |
-| MvelInjection.java:46:27:46:49 | getInputStream(...) : InputStream | MvelInjection.java:52:7:52:16 | expression |
-| MvelInjection.java:57:27:57:49 | getInputStream(...) : InputStream | MvelInjection.java:62:7:62:16 | expression |
+| MvelInjection.java:35:27:35:49 | getInputStream(...) : InputStream | MvelInjection.java:42:7:42:15 | statement |
+| MvelInjection.java:47:27:47:49 | getInputStream(...) : InputStream | MvelInjection.java:53:7:53:16 | expression |
+| MvelInjection.java:58:27:58:49 | getInputStream(...) : InputStream | MvelInjection.java:63:7:63:16 | expression |
 nodes
 | MvelInjection.java:16:27:16:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
 | MvelInjection.java:20:17:20:21 | input | semmle.label | input |
@@ -11,13 +12,15 @@ nodes
 | MvelInjection.java:30:30:30:39 | expression | semmle.label | expression |
 | MvelInjection.java:35:27:35:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
 | MvelInjection.java:41:7:41:15 | statement | semmle.label | statement |
-| MvelInjection.java:46:27:46:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:52:7:52:16 | expression | semmle.label | expression |
-| MvelInjection.java:57:27:57:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:62:7:62:16 | expression | semmle.label | expression |
+| MvelInjection.java:42:7:42:15 | statement | semmle.label | statement |
+| MvelInjection.java:47:27:47:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:53:7:53:16 | expression | semmle.label | expression |
+| MvelInjection.java:58:27:58:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:63:7:63:16 | expression | semmle.label | expression |
 #select
 | MvelInjection.java:20:17:20:21 | input | MvelInjection.java:16:27:16:49 | getInputStream(...) : InputStream | MvelInjection.java:20:17:20:21 | input | MVEL injection from $@. | MvelInjection.java:16:27:16:49 | getInputStream(...) | this user input |
 | MvelInjection.java:30:30:30:39 | expression | MvelInjection.java:25:27:25:49 | getInputStream(...) : InputStream | MvelInjection.java:30:30:30:39 | expression | MVEL injection from $@. | MvelInjection.java:25:27:25:49 | getInputStream(...) | this user input |
 | MvelInjection.java:41:7:41:15 | statement | MvelInjection.java:35:27:35:49 | getInputStream(...) : InputStream | MvelInjection.java:41:7:41:15 | statement | MVEL injection from $@. | MvelInjection.java:35:27:35:49 | getInputStream(...) | this user input |
-| MvelInjection.java:52:7:52:16 | expression | MvelInjection.java:46:27:46:49 | getInputStream(...) : InputStream | MvelInjection.java:52:7:52:16 | expression | MVEL injection from $@. | MvelInjection.java:46:27:46:49 | getInputStream(...) | this user input |
-| MvelInjection.java:62:7:62:16 | expression | MvelInjection.java:57:27:57:49 | getInputStream(...) : InputStream | MvelInjection.java:62:7:62:16 | expression | MVEL injection from $@. | MvelInjection.java:57:27:57:49 | getInputStream(...) | this user input |
+| MvelInjection.java:42:7:42:15 | statement | MvelInjection.java:35:27:35:49 | getInputStream(...) : InputStream | MvelInjection.java:42:7:42:15 | statement | MVEL injection from $@. | MvelInjection.java:35:27:35:49 | getInputStream(...) | this user input |
+| MvelInjection.java:53:7:53:16 | expression | MvelInjection.java:47:27:47:49 | getInputStream(...) : InputStream | MvelInjection.java:53:7:53:16 | expression | MVEL injection from $@. | MvelInjection.java:47:27:47:49 | getInputStream(...) | this user input |
+| MvelInjection.java:63:7:63:16 | expression | MvelInjection.java:58:27:58:49 | getInputStream(...) : InputStream | MvelInjection.java:63:7:63:16 | expression | MVEL injection from $@. | MvelInjection.java:58:27:58:49 | getInputStream(...) | this user input |
diff --git a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java
index 23218e3cfcb2..cf62393ccac2 100644
--- a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java
+++ b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java
@@ -39,6 +39,7 @@ public static void testWithExpressionCompiler(Socket socket) throws IOException
       ExpressionCompiler compiler = new ExpressionCompiler(input);
       ExecutableStatement statement = compiler.compile();
       statement.getValue(new Object(), new ImmutableDefaultFactory());
+      statement.getValue(new Object(), new Object(), new ImmutableDefaultFactory());
     }
   }
 
diff --git a/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/Accessor.java b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/Accessor.java
new file mode 100644
index 000000000000..5c01ab6366da
--- /dev/null
+++ b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/Accessor.java
@@ -0,0 +1,7 @@
+package org.mvel2.compiler;
+
+import org.mvel2.integration.VariableResolverFactory;
+
+public interface Accessor {
+  public Object getValue(Object ctx, Object elCtx, VariableResolverFactory factory);
+}
\ No newline at end of file
diff --git a/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/CompiledExpression.java b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/CompiledExpression.java
index 234900b0fa32..b8a887b4f0db 100644
--- a/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/CompiledExpression.java
+++ b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/CompiledExpression.java
@@ -5,4 +5,5 @@
 public class CompiledExpression implements ExecutableStatement {
   public Object getDirectValue(Object staticContext, VariableResolverFactory factory) { return null; }
   public Object getValue(Object staticContext, VariableResolverFactory factory) { return null; }
+  public Object getValue(Object ctx, Object elCtx, VariableResolverFactory factory) { return null; }
 }
\ No newline at end of file
diff --git a/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/ExecutableStatement.java b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/ExecutableStatement.java
index 1e15b8e6678e..18f2f94feb06 100644
--- a/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/ExecutableStatement.java
+++ b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/ExecutableStatement.java
@@ -2,6 +2,6 @@
 
 import org.mvel2.integration.VariableResolverFactory;
 
-public interface ExecutableStatement {
+public interface ExecutableStatement extends Accessor {
   public Object getValue(Object staticContext, VariableResolverFactory factory);
 }
\ No newline at end of file

From 8fd72659eca3d1b0f3ede5ddc6f2c5a6c9f30ecb Mon Sep 17 00:00:00 2001
From: Artem Smotrakov <artem.smotrakov@gmail.com>
Date: Sat, 25 Apr 2020 13:21:41 +0200
Subject: [PATCH 05/10] Java: Added JSR 223 sinks for MVEL injections

- Updated MvelInjectionLib.qll
- Added tests and stubs for JSR 223 API
---
 .../Security/CWE/CWE-094/MvelInjectionLib.qll | 99 +++++++++++++++++--
 .../CWE/CWE-094/MvelInjection.expected        | 57 ++++++-----
 .../Security/CWE/CWE-094/MvelInjection.java   | 33 +++++++
 .../experimental/Security/CWE/CWE-094/options |  2 +-
 .../javax/script/CompiledScript.java          |  5 +
 .../javax/script/ScriptContext.java           |  3 +
 .../javax/script/ScriptException.java         |  3 +
 .../javax/script/SimpleScriptContext.java     |  3 +
 .../mvel2/compiler/ExecutableStatement.java   |  3 +-
 .../org/mvel2/jsr223/MvelCompiledScript.java  | 11 +++
 .../org/mvel2/jsr223/MvelScriptEngine.java    | 12 +++
 11 files changed, 199 insertions(+), 32 deletions(-)
 create mode 100644 java/ql/test/stubs/jsr223-api/javax/script/CompiledScript.java
 create mode 100644 java/ql/test/stubs/jsr223-api/javax/script/ScriptContext.java
 create mode 100644 java/ql/test/stubs/jsr223-api/javax/script/ScriptException.java
 create mode 100644 java/ql/test/stubs/jsr223-api/javax/script/SimpleScriptContext.java
 create mode 100644 java/ql/test/stubs/mvel2-2.4.7/org/mvel2/jsr223/MvelCompiledScript.java
 create mode 100644 java/ql/test/stubs/mvel2-2.4.7/org/mvel2/jsr223/MvelScriptEngine.java

diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll b/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
index 0e1d6d38fdc0..5db66ea673ea 100644
--- a/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
+++ b/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
@@ -18,7 +18,9 @@ class MvelInjectionConfig extends TaintTracking::Configuration {
     expressionCompilationStep(node1, node2) or
     createExpressionCompilerStep(node1, node2) or
     expressionCompilerCompileStep(node1, node2) or
-    createCompiledAccExpressionStep(node1, node2)
+    createCompiledAccExpressionStep(node1, node2) or
+    scriptCompileStep(node1, node2) or
+    createMvelCompiledScriptStep(node1, node2)
   }
 }
 
@@ -30,7 +32,12 @@ class MvelEvaluationSink extends DataFlow::ExprNode {
   MvelEvaluationSink() {
     exists(StaticMethodAccess ma, Method m | m = ma.getMethod() |
       m instanceof MvelEvalMethod and
-      ma.getAnArgument() = asExpr()
+      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() |
@@ -38,7 +45,9 @@ class MvelEvaluationSink extends DataFlow::ExprNode {
         m instanceof ExecutableStatementEvaluationMethod or
         m instanceof CompiledExpressionEvaluationMethod or
         m instanceof CompiledAccExpressionEvaluationMethod or
-        m instanceof AccessorEvaluationMethod
+        m instanceof AccessorEvaluationMethod or
+        m instanceof CompiledScriptEvaluationMethod or
+        m instanceof MvelCompiledScriptEvaluationMethod
       ) and
       (ma = asExpr() or ma.getQualifier() = asExpr())
     )
@@ -99,6 +108,30 @@ predicate expressionCompilerCompileStep(DataFlow::Node node1, DataFlow::Node nod
   )
 }
 
+/**
+ * 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 MvelScriptEngineCompilationnMethod and
+    (ma = node2.asExpr() or ma.getQualifier() = 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() or cc.getQualifier() = node2.asExpr()) and
+    cc.getArgument(1) = node1.asExpr()
+  )
+}
+
 /**
  * Methods in the MVEL class that evaluate a MVEL expression.
  */
@@ -131,7 +164,7 @@ class MvelCompileExpressionMethod extends Method {
 }
 
 /**
- * Methods in `ExecutableStatement` that trigger evaluating a MVEL expression.
+ * Methods in `ExecutableStatement` that evaluate a MVEL expression.
  */
 class ExecutableStatementEvaluationMethod extends Method {
   ExecutableStatementEvaluationMethod() {
@@ -141,7 +174,7 @@ class ExecutableStatementEvaluationMethod extends Method {
 }
 
 /**
- * Methods in `CompiledExpression` that trigger evaluating a MVEL expression.
+ * Methods in `CompiledExpression` that evaluate a MVEL expression.
  */
 class CompiledExpressionEvaluationMethod extends Method {
   CompiledExpressionEvaluationMethod() {
@@ -151,7 +184,7 @@ class CompiledExpressionEvaluationMethod extends Method {
 }
 
 /**
- * Methods in `CompiledAccExpression` that trigger evaluating a MVEL expression.
+ * Methods in `CompiledAccExpression` that evaluate a MVEL expression.
  */
 class CompiledAccExpressionEvaluationMethod extends Method {
   CompiledAccExpressionEvaluationMethod() {
@@ -161,7 +194,7 @@ class CompiledAccExpressionEvaluationMethod extends Method {
 }
 
 /**
- * Methods in `Accessor` that trigger evaluating a MVEL expression.
+ * Methods in `Accessor` that evaluate a MVEL expression.
  */
 class AccessorEvaluationMethod extends Method {
   AccessorEvaluationMethod() {
@@ -170,6 +203,46 @@ class AccessorEvaluationMethod extends Method {
   }
 }
 
+/**
+ * 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 MvelScriptEngineCompilationnMethod extends Method {
+  MvelScriptEngineCompilationnMethod() {
+    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 `MvelCompiledScript` that evaluate a MVEL expression.
+ */
+class MvelCompiledScriptEvaluationMethod extends Method {
+  MvelCompiledScriptEvaluationMethod() {
+    getDeclaringType() instanceof MvelCompiledScript and
+    hasName("eval")
+  }
+}
+
 class MVEL extends RefType {
   MVEL() { hasQualifiedName("org.mvel2", "MVEL") }
 }
@@ -193,3 +266,15 @@ class CompiledAccExpression extends RefType {
 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") }
+}
diff --git a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected
index 2cf20b0d79f2..841d76e4750e 100644
--- a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected
+++ b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected
@@ -1,26 +1,37 @@
 edges
-| MvelInjection.java:16:27:16:49 | getInputStream(...) : InputStream | MvelInjection.java:20:17:20:21 | input |
-| MvelInjection.java:25:27:25:49 | getInputStream(...) : InputStream | MvelInjection.java:30:30:30:39 | expression |
-| MvelInjection.java:35:27:35:49 | getInputStream(...) : InputStream | MvelInjection.java:41:7:41:15 | statement |
-| MvelInjection.java:35:27:35:49 | getInputStream(...) : InputStream | MvelInjection.java:42:7:42:15 | statement |
-| MvelInjection.java:47:27:47:49 | getInputStream(...) : InputStream | MvelInjection.java:53:7:53:16 | expression |
-| MvelInjection.java:58:27:58:49 | getInputStream(...) : InputStream | MvelInjection.java:63:7:63:16 | expression |
+| MvelInjection.java:20:27:20:49 | getInputStream(...) : InputStream | MvelInjection.java:24:17:24:21 | input |
+| MvelInjection.java:29:27:29:49 | getInputStream(...) : InputStream | MvelInjection.java:34:30:34:39 | expression |
+| MvelInjection.java:39:27:39:49 | getInputStream(...) : InputStream | MvelInjection.java:45:7:45:15 | statement |
+| MvelInjection.java:39:27:39:49 | getInputStream(...) : InputStream | MvelInjection.java:46:7:46:15 | statement |
+| MvelInjection.java:51:27:51:49 | getInputStream(...) : InputStream | MvelInjection.java:57:7:57:16 | expression |
+| MvelInjection.java:62:27:62:49 | getInputStream(...) : InputStream | MvelInjection.java:67:7:67:16 | expression |
+| MvelInjection.java:72:22:72:44 | getInputStream(...) : InputStream | MvelInjection.java:80:5:80:18 | compiledScript |
+| MvelInjection.java:72:22:72:44 | getInputStream(...) : InputStream | MvelInjection.java:83:21:83:26 | script |
+| MvelInjection.java:87:22:87:44 | getInputStream(...) : InputStream | MvelInjection.java:97:5:97:10 | script |
 nodes
-| MvelInjection.java:16:27:16:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:20:17:20:21 | input | semmle.label | input |
-| MvelInjection.java:25:27:25:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:30:30:30:39 | expression | semmle.label | expression |
-| MvelInjection.java:35:27:35:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:41:7:41:15 | statement | semmle.label | statement |
-| MvelInjection.java:42:7:42:15 | statement | semmle.label | statement |
-| MvelInjection.java:47:27:47:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:53:7:53:16 | expression | semmle.label | expression |
-| MvelInjection.java:58:27:58:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:63:7:63:16 | expression | semmle.label | expression |
+| MvelInjection.java:20:27:20:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:24:17:24:21 | input | semmle.label | input |
+| MvelInjection.java:29:27:29:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:34:30:34:39 | expression | semmle.label | expression |
+| MvelInjection.java:39:27:39:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:45:7:45:15 | statement | semmle.label | statement |
+| MvelInjection.java:46:7:46:15 | statement | semmle.label | statement |
+| MvelInjection.java:51:27:51:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:57:7:57:16 | expression | semmle.label | expression |
+| MvelInjection.java:62:27:62:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:67:7:67:16 | expression | semmle.label | expression |
+| MvelInjection.java:72:22:72:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:80:5:80:18 | compiledScript | semmle.label | compiledScript |
+| MvelInjection.java:83:21:83:26 | script | semmle.label | script |
+| MvelInjection.java:87:22:87:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:97:5:97:10 | script | semmle.label | script |
 #select
-| MvelInjection.java:20:17:20:21 | input | MvelInjection.java:16:27:16:49 | getInputStream(...) : InputStream | MvelInjection.java:20:17:20:21 | input | MVEL injection from $@. | MvelInjection.java:16:27:16:49 | getInputStream(...) | this user input |
-| MvelInjection.java:30:30:30:39 | expression | MvelInjection.java:25:27:25:49 | getInputStream(...) : InputStream | MvelInjection.java:30:30:30:39 | expression | MVEL injection from $@. | MvelInjection.java:25:27:25:49 | getInputStream(...) | this user input |
-| MvelInjection.java:41:7:41:15 | statement | MvelInjection.java:35:27:35:49 | getInputStream(...) : InputStream | MvelInjection.java:41:7:41:15 | statement | MVEL injection from $@. | MvelInjection.java:35:27:35:49 | getInputStream(...) | this user input |
-| MvelInjection.java:42:7:42:15 | statement | MvelInjection.java:35:27:35:49 | getInputStream(...) : InputStream | MvelInjection.java:42:7:42:15 | statement | MVEL injection from $@. | MvelInjection.java:35:27:35:49 | getInputStream(...) | this user input |
-| MvelInjection.java:53:7:53:16 | expression | MvelInjection.java:47:27:47:49 | getInputStream(...) : InputStream | MvelInjection.java:53:7:53:16 | expression | MVEL injection from $@. | MvelInjection.java:47:27:47:49 | getInputStream(...) | this user input |
-| MvelInjection.java:63:7:63:16 | expression | MvelInjection.java:58:27:58:49 | getInputStream(...) : InputStream | MvelInjection.java:63:7:63:16 | expression | MVEL injection from $@. | MvelInjection.java:58:27:58:49 | getInputStream(...) | this user input |
+| MvelInjection.java:24:17:24:21 | input | MvelInjection.java:20:27:20:49 | getInputStream(...) : InputStream | MvelInjection.java:24:17:24:21 | input | MVEL injection from $@. | MvelInjection.java:20:27:20:49 | getInputStream(...) | this user input |
+| MvelInjection.java:34:30:34:39 | expression | MvelInjection.java:29:27:29:49 | getInputStream(...) : InputStream | MvelInjection.java:34:30:34:39 | expression | MVEL injection from $@. | MvelInjection.java:29:27:29:49 | getInputStream(...) | this user input |
+| MvelInjection.java:45:7:45:15 | statement | MvelInjection.java:39:27:39:49 | getInputStream(...) : InputStream | MvelInjection.java:45:7:45:15 | statement | MVEL injection from $@. | MvelInjection.java:39:27:39:49 | getInputStream(...) | this user input |
+| MvelInjection.java:46:7:46:15 | statement | MvelInjection.java:39:27:39:49 | getInputStream(...) : InputStream | MvelInjection.java:46:7:46:15 | statement | MVEL injection from $@. | MvelInjection.java:39:27:39:49 | getInputStream(...) | this user input |
+| MvelInjection.java:57:7:57:16 | expression | MvelInjection.java:51:27:51:49 | getInputStream(...) : InputStream | MvelInjection.java:57:7:57:16 | expression | MVEL injection from $@. | MvelInjection.java:51:27:51:49 | getInputStream(...) | this user input |
+| MvelInjection.java:67:7:67:16 | expression | MvelInjection.java:62:27:62:49 | getInputStream(...) : InputStream | MvelInjection.java:67:7:67:16 | expression | MVEL injection from $@. | MvelInjection.java:62:27:62:49 | getInputStream(...) | this user input |
+| MvelInjection.java:80:5:80:18 | compiledScript | MvelInjection.java:72:22:72:44 | getInputStream(...) : InputStream | MvelInjection.java:80:5:80:18 | compiledScript | MVEL injection from $@. | MvelInjection.java:72:22:72:44 | getInputStream(...) | this user input |
+| MvelInjection.java:83:21:83:26 | script | MvelInjection.java:72:22:72:44 | getInputStream(...) : InputStream | MvelInjection.java:83:21:83:26 | script | MVEL injection from $@. | MvelInjection.java:72:22:72:44 | getInputStream(...) | this user input |
+| MvelInjection.java:97:5:97:10 | script | MvelInjection.java:87:22:87:44 | getInputStream(...) : InputStream | MvelInjection.java:97:5:97:10 | script | MVEL injection from $@. | MvelInjection.java:87:22:87:44 | getInputStream(...) | this user input |
diff --git a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java
index cf62393ccac2..aaa264b3c260 100644
--- a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java
+++ b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java
@@ -2,6 +2,8 @@
 import java.io.InputStream;
 import java.io.Serializable;
 import java.net.Socket;
+import javax.script.CompiledScript;
+import javax.script.SimpleScriptContext;
 import org.mvel2.MVEL;
 import org.mvel2.ParserContext;
 import org.mvel2.compiler.CompiledAccExpression;
@@ -9,6 +11,8 @@
 import org.mvel2.compiler.ExecutableStatement;
 import org.mvel2.compiler.ExpressionCompiler;
 import org.mvel2.integration.impl.ImmutableDefaultFactory;
+import org.mvel2.jsr223.MvelCompiledScript;
+import org.mvel2.jsr223.MvelScriptEngine;
 
 public class MvelInjection {
 
@@ -63,4 +67,33 @@ public static void testCompiledAccExpressionGetValue(Socket socket) throws IOExc
       expression.getValue(new Object(), new ImmutableDefaultFactory());
     }
   }
+
+  public static void testMvelScriptEngineCompileAndEvaluate(Socket socket) throws Exception {
+    InputStream in = socket.getInputStream();
+
+    byte[] bytes = new byte[1024];
+    int n = in.read(bytes);
+    String input = new String(bytes, 0, n);
+
+    MvelScriptEngine engine = new MvelScriptEngine();
+    CompiledScript compiledScript = engine.compile(input);
+    compiledScript.eval();
+
+    Serializable script = engine.compiledScript(input);
+    engine.evaluate(script, new SimpleScriptContext());
+  }
+
+  public static void testMvelCompiledScriptCompileAndEvaluate(Socket socket) throws Exception {
+    InputStream in = socket.getInputStream();
+
+    byte[] bytes = new byte[1024];
+    int n = in.read(bytes);
+    String input = new String(bytes, 0, n);
+
+    MvelScriptEngine engine = new MvelScriptEngine();
+    ExpressionCompiler compiler = new ExpressionCompiler(input);
+    ExecutableStatement statement = compiler.compile();
+    MvelCompiledScript script = new MvelCompiledScript(engine, statement);
+    script.eval(new SimpleScriptContext());
+  }
 }
diff --git a/java/ql/test/experimental/Security/CWE/CWE-094/options b/java/ql/test/experimental/Security/CWE/CWE-094/options
index 2048ced65b40..51fae354cec3 100644
--- a/java/ql/test/experimental/Security/CWE/CWE-094/options
+++ b/java/ql/test/experimental/Security/CWE/CWE-094/options
@@ -1 +1 @@
-//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.2.3:${testdir}/../../../../stubs/mvel2-2.4.7
+//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.2.3:${testdir}/../../../../stubs/mvel2-2.4.7:${testdir}/../../../../stubs/jsr223-api
diff --git a/java/ql/test/stubs/jsr223-api/javax/script/CompiledScript.java b/java/ql/test/stubs/jsr223-api/javax/script/CompiledScript.java
new file mode 100644
index 000000000000..dba6c9f88d24
--- /dev/null
+++ b/java/ql/test/stubs/jsr223-api/javax/script/CompiledScript.java
@@ -0,0 +1,5 @@
+package javax.script;
+
+public class CompiledScript {
+  public Object eval() throws ScriptException { return null; }
+}
\ No newline at end of file
diff --git a/java/ql/test/stubs/jsr223-api/javax/script/ScriptContext.java b/java/ql/test/stubs/jsr223-api/javax/script/ScriptContext.java
new file mode 100644
index 000000000000..1e587a0a9bb5
--- /dev/null
+++ b/java/ql/test/stubs/jsr223-api/javax/script/ScriptContext.java
@@ -0,0 +1,3 @@
+package javax.script;
+
+public interface ScriptContext {}
\ No newline at end of file
diff --git a/java/ql/test/stubs/jsr223-api/javax/script/ScriptException.java b/java/ql/test/stubs/jsr223-api/javax/script/ScriptException.java
new file mode 100644
index 000000000000..65d124925c9d
--- /dev/null
+++ b/java/ql/test/stubs/jsr223-api/javax/script/ScriptException.java
@@ -0,0 +1,3 @@
+package javax.script;
+
+public class ScriptException extends Exception {}
diff --git a/java/ql/test/stubs/jsr223-api/javax/script/SimpleScriptContext.java b/java/ql/test/stubs/jsr223-api/javax/script/SimpleScriptContext.java
new file mode 100644
index 000000000000..b292cb8b9a6b
--- /dev/null
+++ b/java/ql/test/stubs/jsr223-api/javax/script/SimpleScriptContext.java
@@ -0,0 +1,3 @@
+package javax.script;
+
+public class SimpleScriptContext implements ScriptContext {}
\ No newline at end of file
diff --git a/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/ExecutableStatement.java b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/ExecutableStatement.java
index 18f2f94feb06..8aed7035aa67 100644
--- a/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/ExecutableStatement.java
+++ b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/compiler/ExecutableStatement.java
@@ -1,7 +1,8 @@
 package org.mvel2.compiler;
 
+import java.io.Serializable;
 import org.mvel2.integration.VariableResolverFactory;
 
-public interface ExecutableStatement extends Accessor {
+public interface ExecutableStatement extends Accessor, Serializable {
   public Object getValue(Object staticContext, VariableResolverFactory factory);
 }
\ No newline at end of file
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
new file mode 100644
index 000000000000..a4be37ada323
--- /dev/null
+++ b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/jsr223/MvelCompiledScript.java
@@ -0,0 +1,11 @@
+package org.mvel2.jsr223;
+
+import java.io.Serializable;
+import javax.script.CompiledScript;
+import javax.script.ScriptContext;
+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
diff --git a/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/jsr223/MvelScriptEngine.java b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/jsr223/MvelScriptEngine.java
new file mode 100644
index 000000000000..6769a6198a4e
--- /dev/null
+++ b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/jsr223/MvelScriptEngine.java
@@ -0,0 +1,12 @@
+package org.mvel2.jsr223;
+
+import java.io.Serializable;
+import javax.script.CompiledScript;
+import javax.script.ScriptContext;
+import javax.script.ScriptException;
+
+public class MvelScriptEngine {
+  public CompiledScript compile(String script) throws ScriptException { return null; }
+  public Serializable compiledScript(String script) throws ScriptException { return null; }
+  public Object evaluate(Serializable expression, ScriptContext context) throws ScriptException { return null; }
+}
\ No newline at end of file

From fa717b2d864afa861f0cef73b2cbefc43734835b Mon Sep 17 00:00:00 2001
From: Artem Smotrakov <artem.smotrakov@gmail.com>
Date: Fri, 1 May 2020 16:48:20 +0200
Subject: [PATCH 06/10] Java: Added template sinks for MVEL injections

---
 .../Security/CWE/CWE-094/MvelInjectionLib.qll | 83 +++++++++++++++++--
 .../CWE/CWE-094/MvelInjection.expected        | 80 ++++++++++--------
 .../Security/CWE/CWE-094/MvelInjection.java   | 35 ++++++++
 .../org/mvel2/templates/CompiledTemplate.java |  3 +
 .../org/mvel2/templates/TemplateCompiler.java |  7 ++
 .../org/mvel2/templates/TemplateRuntime.java  |  8 ++
 6 files changed, 177 insertions(+), 39 deletions(-)
 create mode 100644 java/ql/test/stubs/mvel2-2.4.7/org/mvel2/templates/CompiledTemplate.java
 create mode 100644 java/ql/test/stubs/mvel2-2.4.7/org/mvel2/templates/TemplateCompiler.java
 create mode 100644 java/ql/test/stubs/mvel2-2.4.7/org/mvel2/templates/TemplateRuntime.java

diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll b/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
index 5db66ea673ea..4810aa8e5068 100644
--- a/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
+++ b/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
@@ -20,7 +20,9 @@ class MvelInjectionConfig extends TaintTracking::Configuration {
     expressionCompilerCompileStep(node1, node2) or
     createCompiledAccExpressionStep(node1, node2) or
     scriptCompileStep(node1, node2) or
-    createMvelCompiledScriptStep(node1, node2)
+    createMvelCompiledScriptStep(node1, node2) or
+    templateCompileStep(node1, node2) or
+    createTemplateCompilerStep(node1, node2)
   }
 }
 
@@ -31,7 +33,10 @@ class MvelInjectionConfig extends TaintTracking::Configuration {
 class MvelEvaluationSink extends DataFlow::ExprNode {
   MvelEvaluationSink() {
     exists(StaticMethodAccess ma, Method m | m = ma.getMethod() |
-      m instanceof MvelEvalMethod and
+      (
+        m instanceof MvelEvalMethod or
+        m instanceof TemplateRuntimeEvaluationMethod
+      ) and
       ma.getArgument(0) = asExpr()
     )
     or
@@ -114,7 +119,7 @@ predicate expressionCompilerCompileStep(DataFlow::Node node1, DataFlow::Node nod
  */
 predicate scriptCompileStep(DataFlow::Node node1, DataFlow::Node node2) {
   exists(MethodAccess ma, Method m | ma.getMethod() = m |
-    m instanceof MvelScriptEngineCompilationnMethod and
+    m instanceof MvelScriptEngineCompilationMethod and
     (ma = node2.asExpr() or ma.getQualifier() = node2.asExpr()) and
     ma.getArgument(0) = node1.asExpr()
   )
@@ -132,6 +137,36 @@ predicate createMvelCompiledScriptStep(DataFlow::Node node1, DataFlow::Node node
   )
 }
 
+/**
+ * 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() or cc.getQualifier() = 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() or ma.getQualifier() = node2.asExpr()) and
+    ma.getArgument(0) = node1.asExpr()
+  )
+}
+
 /**
  * Methods in the MVEL class that evaluate a MVEL expression.
  */
@@ -216,8 +251,8 @@ class MvelScriptEngineEvaluationMethod extends Method {
 /**
  * Methods in `MvelScriptEngine` that compile a MVEL expression.
  */
-class MvelScriptEngineCompilationnMethod extends Method {
-  MvelScriptEngineCompilationnMethod() {
+class MvelScriptEngineCompilationMethod extends Method {
+  MvelScriptEngineCompilationMethod() {
     getDeclaringType() instanceof MvelScriptEngine and
     (hasName("compile") or hasName("compiledScript"))
   }
@@ -233,6 +268,36 @@ class CompiledScriptEvaluationMethod extends Method {
   }
 }
 
+/**
+ * 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.
  */
@@ -278,3 +343,11 @@ class MvelScriptEngine extends RefType {
 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") }
+}
diff --git a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected
index 841d76e4750e..c340cd5a19a8 100644
--- a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected
+++ b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected
@@ -1,37 +1,49 @@
 edges
-| MvelInjection.java:20:27:20:49 | getInputStream(...) : InputStream | MvelInjection.java:24:17:24:21 | input |
-| MvelInjection.java:29:27:29:49 | getInputStream(...) : InputStream | MvelInjection.java:34:30:34:39 | expression |
-| MvelInjection.java:39:27:39:49 | getInputStream(...) : InputStream | MvelInjection.java:45:7:45:15 | statement |
-| MvelInjection.java:39:27:39:49 | getInputStream(...) : InputStream | MvelInjection.java:46:7:46:15 | statement |
-| MvelInjection.java:51:27:51:49 | getInputStream(...) : InputStream | MvelInjection.java:57:7:57:16 | expression |
-| MvelInjection.java:62:27:62:49 | getInputStream(...) : InputStream | MvelInjection.java:67:7:67:16 | expression |
-| MvelInjection.java:72:22:72:44 | getInputStream(...) : InputStream | MvelInjection.java:80:5:80:18 | compiledScript |
-| MvelInjection.java:72:22:72:44 | getInputStream(...) : InputStream | MvelInjection.java:83:21:83:26 | script |
-| MvelInjection.java:87:22:87:44 | getInputStream(...) : InputStream | MvelInjection.java:97:5:97:10 | script |
+| MvelInjection.java:24:27:24:49 | getInputStream(...) : InputStream | MvelInjection.java:28:17:28:21 | input |
+| MvelInjection.java:33:27:33:49 | getInputStream(...) : InputStream | MvelInjection.java:38:30:38:39 | expression |
+| MvelInjection.java:43:27:43:49 | getInputStream(...) : InputStream | MvelInjection.java:49:7:49:15 | statement |
+| MvelInjection.java:43:27:43:49 | getInputStream(...) : InputStream | MvelInjection.java:50:7:50:15 | statement |
+| MvelInjection.java:55:27:55:49 | getInputStream(...) : InputStream | MvelInjection.java:61:7:61:16 | expression |
+| MvelInjection.java:66:27:66:49 | getInputStream(...) : InputStream | MvelInjection.java:71:7:71:16 | expression |
+| MvelInjection.java:76:22:76:44 | getInputStream(...) : InputStream | MvelInjection.java:84:5:84:18 | compiledScript |
+| MvelInjection.java:76:22:76:44 | getInputStream(...) : InputStream | MvelInjection.java:87:21:87:26 | script |
+| MvelInjection.java:91:22:91:44 | getInputStream(...) : InputStream | MvelInjection.java:101:5:101:10 | script |
+| MvelInjection.java:105:22:105:44 | getInputStream(...) : InputStream | MvelInjection.java:111:26:111:30 | input |
+| MvelInjection.java:115:22:115:44 | getInputStream(...) : InputStream | MvelInjection.java:121:29:121:67 | compileTemplate(...) |
+| MvelInjection.java:125:22:125:44 | getInputStream(...) : InputStream | MvelInjection.java:132:54:132:71 | compile(...) |
 nodes
-| MvelInjection.java:20:27:20:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:24:17:24:21 | input | semmle.label | input |
-| MvelInjection.java:29:27:29:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:34:30:34:39 | expression | semmle.label | expression |
-| MvelInjection.java:39:27:39:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:45:7:45:15 | statement | semmle.label | statement |
-| MvelInjection.java:46:7:46:15 | statement | semmle.label | statement |
-| MvelInjection.java:51:27:51:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:57:7:57:16 | expression | semmle.label | expression |
-| MvelInjection.java:62:27:62:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:67:7:67:16 | expression | semmle.label | expression |
-| MvelInjection.java:72:22:72:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:80:5:80:18 | compiledScript | semmle.label | compiledScript |
-| MvelInjection.java:83:21:83:26 | script | semmle.label | script |
-| MvelInjection.java:87:22:87:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:97:5:97:10 | script | semmle.label | script |
+| MvelInjection.java:24:27:24:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:28:17:28:21 | input | semmle.label | input |
+| MvelInjection.java:33:27:33:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:38:30:38:39 | expression | semmle.label | expression |
+| MvelInjection.java:43:27:43:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:49:7:49:15 | statement | semmle.label | statement |
+| MvelInjection.java:50:7:50:15 | statement | semmle.label | statement |
+| MvelInjection.java:55:27:55:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:61:7:61:16 | expression | semmle.label | expression |
+| MvelInjection.java:66:27:66:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:71:7:71:16 | expression | semmle.label | expression |
+| MvelInjection.java:76:22:76:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:84:5:84:18 | compiledScript | semmle.label | compiledScript |
+| MvelInjection.java:87:21:87:26 | script | semmle.label | script |
+| MvelInjection.java:91:22:91:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:101:5:101:10 | script | semmle.label | script |
+| MvelInjection.java:105:22:105:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:111:26:111:30 | input | semmle.label | input |
+| MvelInjection.java:115:22:115:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:121:29:121:67 | compileTemplate(...) | semmle.label | compileTemplate(...) |
+| MvelInjection.java:125:22:125:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:132:54:132:71 | compile(...) | semmle.label | compile(...) |
 #select
-| MvelInjection.java:24:17:24:21 | input | MvelInjection.java:20:27:20:49 | getInputStream(...) : InputStream | MvelInjection.java:24:17:24:21 | input | MVEL injection from $@. | MvelInjection.java:20:27:20:49 | getInputStream(...) | this user input |
-| MvelInjection.java:34:30:34:39 | expression | MvelInjection.java:29:27:29:49 | getInputStream(...) : InputStream | MvelInjection.java:34:30:34:39 | expression | MVEL injection from $@. | MvelInjection.java:29:27:29:49 | getInputStream(...) | this user input |
-| MvelInjection.java:45:7:45:15 | statement | MvelInjection.java:39:27:39:49 | getInputStream(...) : InputStream | MvelInjection.java:45:7:45:15 | statement | MVEL injection from $@. | MvelInjection.java:39:27:39:49 | getInputStream(...) | this user input |
-| MvelInjection.java:46:7:46:15 | statement | MvelInjection.java:39:27:39:49 | getInputStream(...) : InputStream | MvelInjection.java:46:7:46:15 | statement | MVEL injection from $@. | MvelInjection.java:39:27:39:49 | getInputStream(...) | this user input |
-| MvelInjection.java:57:7:57:16 | expression | MvelInjection.java:51:27:51:49 | getInputStream(...) : InputStream | MvelInjection.java:57:7:57:16 | expression | MVEL injection from $@. | MvelInjection.java:51:27:51:49 | getInputStream(...) | this user input |
-| MvelInjection.java:67:7:67:16 | expression | MvelInjection.java:62:27:62:49 | getInputStream(...) : InputStream | MvelInjection.java:67:7:67:16 | expression | MVEL injection from $@. | MvelInjection.java:62:27:62:49 | getInputStream(...) | this user input |
-| MvelInjection.java:80:5:80:18 | compiledScript | MvelInjection.java:72:22:72:44 | getInputStream(...) : InputStream | MvelInjection.java:80:5:80:18 | compiledScript | MVEL injection from $@. | MvelInjection.java:72:22:72:44 | getInputStream(...) | this user input |
-| MvelInjection.java:83:21:83:26 | script | MvelInjection.java:72:22:72:44 | getInputStream(...) : InputStream | MvelInjection.java:83:21:83:26 | script | MVEL injection from $@. | MvelInjection.java:72:22:72:44 | getInputStream(...) | this user input |
-| MvelInjection.java:97:5:97:10 | script | MvelInjection.java:87:22:87:44 | getInputStream(...) : InputStream | MvelInjection.java:97:5:97:10 | script | MVEL injection from $@. | MvelInjection.java:87:22:87:44 | getInputStream(...) | this user input |
+| MvelInjection.java:28:17:28:21 | input | MvelInjection.java:24:27:24:49 | getInputStream(...) : InputStream | MvelInjection.java:28:17:28:21 | input | MVEL injection from $@. | MvelInjection.java:24:27:24:49 | getInputStream(...) | this user input |
+| MvelInjection.java:38:30:38:39 | expression | MvelInjection.java:33:27:33:49 | getInputStream(...) : InputStream | MvelInjection.java:38:30:38:39 | expression | MVEL injection from $@. | MvelInjection.java:33:27:33:49 | getInputStream(...) | this user input |
+| MvelInjection.java:49:7:49:15 | statement | MvelInjection.java:43:27:43:49 | getInputStream(...) : InputStream | MvelInjection.java:49:7:49:15 | statement | MVEL injection from $@. | MvelInjection.java:43:27:43:49 | getInputStream(...) | this user input |
+| MvelInjection.java:50:7:50:15 | statement | MvelInjection.java:43:27:43:49 | getInputStream(...) : InputStream | MvelInjection.java:50:7:50:15 | statement | MVEL injection from $@. | MvelInjection.java:43:27:43:49 | getInputStream(...) | this user input |
+| MvelInjection.java:61:7:61:16 | expression | MvelInjection.java:55:27:55:49 | getInputStream(...) : InputStream | MvelInjection.java:61:7:61:16 | expression | MVEL injection from $@. | MvelInjection.java:55:27:55:49 | getInputStream(...) | this user input |
+| MvelInjection.java:71:7:71:16 | expression | MvelInjection.java:66:27:66:49 | getInputStream(...) : InputStream | MvelInjection.java:71:7:71:16 | expression | MVEL injection from $@. | MvelInjection.java:66:27:66:49 | getInputStream(...) | this user input |
+| MvelInjection.java:84:5:84:18 | compiledScript | MvelInjection.java:76:22:76:44 | getInputStream(...) : InputStream | MvelInjection.java:84:5:84:18 | compiledScript | MVEL injection from $@. | MvelInjection.java:76:22:76:44 | getInputStream(...) | this user input |
+| MvelInjection.java:87:21:87:26 | script | MvelInjection.java:76:22:76:44 | getInputStream(...) : InputStream | MvelInjection.java:87:21:87:26 | script | MVEL injection from $@. | MvelInjection.java:76:22:76:44 | getInputStream(...) | this user input |
+| MvelInjection.java:101:5:101:10 | script | MvelInjection.java:91:22:91:44 | getInputStream(...) : InputStream | MvelInjection.java:101:5:101:10 | script | MVEL injection from $@. | MvelInjection.java:91:22:91:44 | getInputStream(...) | this user input |
+| MvelInjection.java:111:26:111:30 | input | MvelInjection.java:105:22:105:44 | getInputStream(...) : InputStream | MvelInjection.java:111:26:111:30 | input | MVEL injection from $@. | MvelInjection.java:105:22:105:44 | getInputStream(...) | this user input |
+| MvelInjection.java:121:29:121:67 | compileTemplate(...) | MvelInjection.java:115:22:115:44 | getInputStream(...) : InputStream | MvelInjection.java:121:29:121:67 | compileTemplate(...) | MVEL injection from $@. | MvelInjection.java:115:22:115:44 | getInputStream(...) | this user input |
+| MvelInjection.java:132:54:132:71 | compile(...) | MvelInjection.java:125:22:125:44 | getInputStream(...) : InputStream | MvelInjection.java:132:54:132:71 | compile(...) | MVEL injection from $@. | MvelInjection.java:125:22:125:44 | getInputStream(...) | this user input |
diff --git a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java
index aaa264b3c260..b5ef1625502f 100644
--- a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java
+++ b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java
@@ -2,6 +2,7 @@
 import java.io.InputStream;
 import java.io.Serializable;
 import java.net.Socket;
+import java.util.HashMap;
 import javax.script.CompiledScript;
 import javax.script.SimpleScriptContext;
 import org.mvel2.MVEL;
@@ -13,6 +14,9 @@
 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 {
 
@@ -96,4 +100,35 @@ public static void testMvelCompiledScriptCompileAndEvaluate(Socket socket) throw
     MvelCompiledScript script = new MvelCompiledScript(engine, statement);
     script.eval(new SimpleScriptContext());
   }
+
+  public static void testTemplateRuntimeEval(Socket socket) throws Exception {
+    InputStream in = socket.getInputStream();
+
+    byte[] bytes = new byte[1024];
+    int n = in.read(bytes);
+    String input = new String(bytes, 0, n);
+
+    TemplateRuntime.eval(input, new HashMap());
+  }
+
+  public static void testTemplateRuntimeCompileTemplateAndExecute(Socket socket) throws Exception {
+    InputStream in = socket.getInputStream();
+
+    byte[] bytes = new byte[1024];
+    int n = in.read(bytes);
+    String input = new String(bytes, 0, n);
+
+    TemplateRuntime.execute(TemplateCompiler.compileTemplate(input), new HashMap());
+  }
+
+  public static void testTemplateRuntimeCompileAndExecute(Socket socket) throws Exception {
+    InputStream in = socket.getInputStream();
+
+    byte[] bytes = new byte[1024];
+    int n = in.read(bytes);
+    String input = new String(bytes, 0, n);
+
+    TemplateCompiler compiler = new TemplateCompiler(input);
+    String output = (String) TemplateRuntime.execute(compiler.compile(), new HashMap());
+  }
 }
diff --git a/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/templates/CompiledTemplate.java b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/templates/CompiledTemplate.java
new file mode 100644
index 000000000000..d0152d28318e
--- /dev/null
+++ b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/templates/CompiledTemplate.java
@@ -0,0 +1,3 @@
+package org.mvel2.templates;
+
+public class CompiledTemplate {}
\ No newline at end of file
diff --git a/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/templates/TemplateCompiler.java b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/templates/TemplateCompiler.java
new file mode 100644
index 000000000000..333e4164bcd4
--- /dev/null
+++ b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/templates/TemplateCompiler.java
@@ -0,0 +1,7 @@
+package org.mvel2.templates;
+
+public class TemplateCompiler {
+  public TemplateCompiler(String template) {}
+  public static CompiledTemplate compileTemplate(String template) { return null; }
+  public CompiledTemplate compile() { return null; }
+}
\ No newline at end of file
diff --git a/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/templates/TemplateRuntime.java b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/templates/TemplateRuntime.java
new file mode 100644
index 000000000000..4fec9fa4e30b
--- /dev/null
+++ b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/templates/TemplateRuntime.java
@@ -0,0 +1,8 @@
+package org.mvel2.templates;
+
+import java.util.Map;
+
+public class TemplateRuntime {
+  public static Object eval(String template, Map vars) { return null; }
+  public static Object execute(CompiledTemplate compiled, Map vars) { return null; }
+}
\ No newline at end of file

From df9d10f2ac2f9d42993d1cc13ed6b3fb5c37180a Mon Sep 17 00:00:00 2001
From: Artem Smotrakov <artem.smotrakov@gmail.com>
Date: Fri, 1 May 2020 17:02:15 +0200
Subject: [PATCH 07/10] Java: Added MVELRuntime.execute() sink for MVEL
 injections

---
 .../Security/CWE/CWE-094/MvelInjectionLib.qll | 19 ++++
 .../CWE/CWE-094/MvelInjection.expected        | 94 ++++++++++---------
 .../Security/CWE/CWE-094/MvelInjection.java   | 13 +++
 .../mvel2-2.4.7/org/mvel2/MVELRuntime.java    |  8 ++
 4 files changed, 89 insertions(+), 45 deletions(-)
 create mode 100644 java/ql/test/stubs/mvel2-2.4.7/org/mvel2/MVELRuntime.java

diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll b/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
index 4810aa8e5068..5bd6c122015b 100644
--- a/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
+++ b/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
@@ -56,6 +56,11 @@ class MvelEvaluationSink extends DataFlow::ExprNode {
       ) and
       (ma = asExpr() or ma.getQualifier() = asExpr())
     )
+    or
+    exists(StaticMethodAccess ma, Method m | m = ma.getMethod() |
+      m instanceof MvelRuntimeEvaluationMethod and
+      ma.getArgument(1) = asExpr()
+    )
   }
 }
 
@@ -308,6 +313,16 @@ class MvelCompiledScriptEvaluationMethod extends Method {
   }
 }
 
+/**
+ * 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") }
 }
@@ -351,3 +366,7 @@ class TemplateRuntime extends RefType {
 class TemplateCompiler extends RefType {
   TemplateCompiler() { hasQualifiedName("org.mvel2.templates", "TemplateCompiler") }
 }
+
+class MVELRuntime extends RefType {
+  MVELRuntime() { hasQualifiedName("org.mvel2", "MVELRuntime") }
+}
diff --git a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected
index c340cd5a19a8..c40da7f5f870 100644
--- a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected
+++ b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected
@@ -1,49 +1,53 @@
 edges
-| MvelInjection.java:24:27:24:49 | getInputStream(...) : InputStream | MvelInjection.java:28:17:28:21 | input |
-| MvelInjection.java:33:27:33:49 | getInputStream(...) : InputStream | MvelInjection.java:38:30:38:39 | expression |
-| MvelInjection.java:43:27:43:49 | getInputStream(...) : InputStream | MvelInjection.java:49:7:49:15 | statement |
-| MvelInjection.java:43:27:43:49 | getInputStream(...) : InputStream | MvelInjection.java:50:7:50:15 | statement |
-| MvelInjection.java:55:27:55:49 | getInputStream(...) : InputStream | MvelInjection.java:61:7:61:16 | expression |
-| MvelInjection.java:66:27:66:49 | getInputStream(...) : InputStream | MvelInjection.java:71:7:71:16 | expression |
-| MvelInjection.java:76:22:76:44 | getInputStream(...) : InputStream | MvelInjection.java:84:5:84:18 | compiledScript |
-| MvelInjection.java:76:22:76:44 | getInputStream(...) : InputStream | MvelInjection.java:87:21:87:26 | script |
-| MvelInjection.java:91:22:91:44 | getInputStream(...) : InputStream | MvelInjection.java:101:5:101:10 | script |
-| MvelInjection.java:105:22:105:44 | getInputStream(...) : InputStream | MvelInjection.java:111:26:111:30 | input |
-| MvelInjection.java:115:22:115:44 | getInputStream(...) : InputStream | MvelInjection.java:121:29:121:67 | compileTemplate(...) |
-| MvelInjection.java:125:22:125:44 | getInputStream(...) : InputStream | MvelInjection.java:132:54:132:71 | compile(...) |
+| MvelInjection.java:25:27:25:49 | getInputStream(...) : InputStream | MvelInjection.java:29:17:29:21 | input |
+| MvelInjection.java:34:27:34:49 | getInputStream(...) : InputStream | MvelInjection.java:39:30:39:39 | expression |
+| MvelInjection.java:44:27:44:49 | getInputStream(...) : InputStream | MvelInjection.java:50:7:50:15 | statement |
+| MvelInjection.java:44:27:44:49 | getInputStream(...) : InputStream | MvelInjection.java:51:7:51:15 | statement |
+| MvelInjection.java:56:27:56:49 | getInputStream(...) : InputStream | MvelInjection.java:62:7:62:16 | expression |
+| MvelInjection.java:67:27:67:49 | getInputStream(...) : InputStream | MvelInjection.java:72:7:72:16 | expression |
+| MvelInjection.java:77:22:77:44 | getInputStream(...) : InputStream | MvelInjection.java:85:5:85:18 | compiledScript |
+| MvelInjection.java:77:22:77:44 | getInputStream(...) : InputStream | MvelInjection.java:88:21:88:26 | script |
+| MvelInjection.java:92:22:92:44 | getInputStream(...) : InputStream | MvelInjection.java:102:5:102:10 | script |
+| MvelInjection.java:106:22:106:44 | getInputStream(...) : InputStream | MvelInjection.java:112:26:112:30 | input |
+| MvelInjection.java:116:22:116:44 | getInputStream(...) : InputStream | MvelInjection.java:122:29:122:67 | compileTemplate(...) |
+| MvelInjection.java:126:22:126:44 | getInputStream(...) : InputStream | MvelInjection.java:133:54:133:71 | compile(...) |
+| MvelInjection.java:137:22:137:44 | getInputStream(...) : InputStream | MvelInjection.java:145:32:145:41 | expression |
 nodes
-| MvelInjection.java:24:27:24:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:28:17:28:21 | input | semmle.label | input |
-| MvelInjection.java:33:27:33:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:38:30:38:39 | expression | semmle.label | expression |
-| MvelInjection.java:43:27:43:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:49:7:49:15 | statement | semmle.label | statement |
+| MvelInjection.java:25:27:25:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:29:17:29:21 | input | semmle.label | input |
+| MvelInjection.java:34:27:34:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:39:30:39:39 | expression | semmle.label | expression |
+| MvelInjection.java:44:27:44:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
 | MvelInjection.java:50:7:50:15 | statement | semmle.label | statement |
-| MvelInjection.java:55:27:55:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:61:7:61:16 | expression | semmle.label | expression |
-| MvelInjection.java:66:27:66:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:71:7:71:16 | expression | semmle.label | expression |
-| MvelInjection.java:76:22:76:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:84:5:84:18 | compiledScript | semmle.label | compiledScript |
-| MvelInjection.java:87:21:87:26 | script | semmle.label | script |
-| MvelInjection.java:91:22:91:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:101:5:101:10 | script | semmle.label | script |
-| MvelInjection.java:105:22:105:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:111:26:111:30 | input | semmle.label | input |
-| MvelInjection.java:115:22:115:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:121:29:121:67 | compileTemplate(...) | semmle.label | compileTemplate(...) |
-| MvelInjection.java:125:22:125:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:132:54:132:71 | compile(...) | semmle.label | compile(...) |
+| MvelInjection.java:51:7:51:15 | statement | semmle.label | statement |
+| MvelInjection.java:56:27:56:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:62:7:62:16 | expression | semmle.label | expression |
+| MvelInjection.java:67:27:67:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:72:7:72:16 | expression | semmle.label | expression |
+| MvelInjection.java:77:22:77:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:85:5:85:18 | compiledScript | semmle.label | compiledScript |
+| MvelInjection.java:88:21:88:26 | script | semmle.label | script |
+| MvelInjection.java:92:22:92:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:102:5:102:10 | script | semmle.label | script |
+| MvelInjection.java:106:22:106:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:112:26:112:30 | input | semmle.label | input |
+| MvelInjection.java:116:22:116:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:122:29:122:67 | compileTemplate(...) | semmle.label | compileTemplate(...) |
+| MvelInjection.java:126:22:126:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:133:54:133:71 | compile(...) | semmle.label | compile(...) |
+| MvelInjection.java:137:22:137:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
+| MvelInjection.java:145:32:145:41 | expression | semmle.label | expression |
 #select
-| MvelInjection.java:28:17:28:21 | input | MvelInjection.java:24:27:24:49 | getInputStream(...) : InputStream | MvelInjection.java:28:17:28:21 | input | MVEL injection from $@. | MvelInjection.java:24:27:24:49 | getInputStream(...) | this user input |
-| MvelInjection.java:38:30:38:39 | expression | MvelInjection.java:33:27:33:49 | getInputStream(...) : InputStream | MvelInjection.java:38:30:38:39 | expression | MVEL injection from $@. | MvelInjection.java:33:27:33:49 | getInputStream(...) | this user input |
-| MvelInjection.java:49:7:49:15 | statement | MvelInjection.java:43:27:43:49 | getInputStream(...) : InputStream | MvelInjection.java:49:7:49:15 | statement | MVEL injection from $@. | MvelInjection.java:43:27:43:49 | getInputStream(...) | this user input |
-| MvelInjection.java:50:7:50:15 | statement | MvelInjection.java:43:27:43:49 | getInputStream(...) : InputStream | MvelInjection.java:50:7:50:15 | statement | MVEL injection from $@. | MvelInjection.java:43:27:43:49 | getInputStream(...) | this user input |
-| MvelInjection.java:61:7:61:16 | expression | MvelInjection.java:55:27:55:49 | getInputStream(...) : InputStream | MvelInjection.java:61:7:61:16 | expression | MVEL injection from $@. | MvelInjection.java:55:27:55:49 | getInputStream(...) | this user input |
-| MvelInjection.java:71:7:71:16 | expression | MvelInjection.java:66:27:66:49 | getInputStream(...) : InputStream | MvelInjection.java:71:7:71:16 | expression | MVEL injection from $@. | MvelInjection.java:66:27:66:49 | getInputStream(...) | this user input |
-| MvelInjection.java:84:5:84:18 | compiledScript | MvelInjection.java:76:22:76:44 | getInputStream(...) : InputStream | MvelInjection.java:84:5:84:18 | compiledScript | MVEL injection from $@. | MvelInjection.java:76:22:76:44 | getInputStream(...) | this user input |
-| MvelInjection.java:87:21:87:26 | script | MvelInjection.java:76:22:76:44 | getInputStream(...) : InputStream | MvelInjection.java:87:21:87:26 | script | MVEL injection from $@. | MvelInjection.java:76:22:76:44 | getInputStream(...) | this user input |
-| MvelInjection.java:101:5:101:10 | script | MvelInjection.java:91:22:91:44 | getInputStream(...) : InputStream | MvelInjection.java:101:5:101:10 | script | MVEL injection from $@. | MvelInjection.java:91:22:91:44 | getInputStream(...) | this user input |
-| MvelInjection.java:111:26:111:30 | input | MvelInjection.java:105:22:105:44 | getInputStream(...) : InputStream | MvelInjection.java:111:26:111:30 | input | MVEL injection from $@. | MvelInjection.java:105:22:105:44 | getInputStream(...) | this user input |
-| MvelInjection.java:121:29:121:67 | compileTemplate(...) | MvelInjection.java:115:22:115:44 | getInputStream(...) : InputStream | MvelInjection.java:121:29:121:67 | compileTemplate(...) | MVEL injection from $@. | MvelInjection.java:115:22:115:44 | getInputStream(...) | this user input |
-| MvelInjection.java:132:54:132:71 | compile(...) | MvelInjection.java:125:22:125:44 | getInputStream(...) : InputStream | MvelInjection.java:132:54:132:71 | compile(...) | MVEL injection from $@. | MvelInjection.java:125:22:125:44 | getInputStream(...) | this user input |
+| MvelInjection.java:29:17:29:21 | input | MvelInjection.java:25:27:25:49 | getInputStream(...) : InputStream | MvelInjection.java:29:17:29:21 | input | MVEL injection from $@. | MvelInjection.java:25:27:25:49 | getInputStream(...) | this user input |
+| MvelInjection.java:39:30:39:39 | expression | MvelInjection.java:34:27:34:49 | getInputStream(...) : InputStream | MvelInjection.java:39:30:39:39 | expression | MVEL injection from $@. | MvelInjection.java:34:27:34:49 | getInputStream(...) | this user input |
+| MvelInjection.java:50:7:50:15 | statement | MvelInjection.java:44:27:44:49 | getInputStream(...) : InputStream | MvelInjection.java:50:7:50:15 | statement | MVEL injection from $@. | MvelInjection.java:44:27:44:49 | getInputStream(...) | this user input |
+| MvelInjection.java:51:7:51:15 | statement | MvelInjection.java:44:27:44:49 | getInputStream(...) : InputStream | MvelInjection.java:51:7:51:15 | statement | MVEL injection from $@. | MvelInjection.java:44:27:44:49 | getInputStream(...) | this user input |
+| MvelInjection.java:62:7:62:16 | expression | MvelInjection.java:56:27:56:49 | getInputStream(...) : InputStream | MvelInjection.java:62:7:62:16 | expression | MVEL injection from $@. | MvelInjection.java:56:27:56:49 | getInputStream(...) | this user input |
+| MvelInjection.java:72:7:72:16 | expression | MvelInjection.java:67:27:67:49 | getInputStream(...) : InputStream | MvelInjection.java:72:7:72:16 | expression | MVEL injection from $@. | MvelInjection.java:67:27:67:49 | getInputStream(...) | this user input |
+| MvelInjection.java:85:5:85:18 | compiledScript | MvelInjection.java:77:22:77:44 | getInputStream(...) : InputStream | MvelInjection.java:85:5:85:18 | compiledScript | MVEL injection from $@. | MvelInjection.java:77:22:77:44 | getInputStream(...) | this user input |
+| MvelInjection.java:88:21:88:26 | script | MvelInjection.java:77:22:77:44 | getInputStream(...) : InputStream | MvelInjection.java:88:21:88:26 | script | MVEL injection from $@. | MvelInjection.java:77:22:77:44 | getInputStream(...) | this user input |
+| MvelInjection.java:102:5:102:10 | script | MvelInjection.java:92:22:92:44 | getInputStream(...) : InputStream | MvelInjection.java:102:5:102:10 | script | MVEL injection from $@. | MvelInjection.java:92:22:92:44 | getInputStream(...) | this user input |
+| MvelInjection.java:112:26:112:30 | input | MvelInjection.java:106:22:106:44 | getInputStream(...) : InputStream | MvelInjection.java:112:26:112:30 | input | MVEL injection from $@. | MvelInjection.java:106:22:106:44 | getInputStream(...) | this user input |
+| MvelInjection.java:122:29:122:67 | compileTemplate(...) | MvelInjection.java:116:22:116:44 | getInputStream(...) : InputStream | MvelInjection.java:122:29:122:67 | compileTemplate(...) | MVEL injection from $@. | MvelInjection.java:116:22:116:44 | getInputStream(...) | this user input |
+| MvelInjection.java:133:54:133:71 | compile(...) | MvelInjection.java:126:22:126:44 | getInputStream(...) : InputStream | MvelInjection.java:133:54:133:71 | compile(...) | MVEL injection from $@. | MvelInjection.java:126:22:126:44 | getInputStream(...) | this user input |
+| MvelInjection.java:145:32:145:41 | expression | MvelInjection.java:137:22:137:44 | getInputStream(...) : InputStream | MvelInjection.java:145:32:145:41 | expression | MVEL injection from $@. | MvelInjection.java:137:22:137:44 | getInputStream(...) | this user input |
diff --git a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java
index b5ef1625502f..4454a782bf89 100644
--- a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java
+++ b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java
@@ -6,6 +6,7 @@
 import javax.script.CompiledScript;
 import javax.script.SimpleScriptContext;
 import org.mvel2.MVEL;
+import org.mvel2.MVELRuntime;
 import org.mvel2.ParserContext;
 import org.mvel2.compiler.CompiledAccExpression;
 import org.mvel2.compiler.CompiledExpression;
@@ -131,4 +132,16 @@ public static void testTemplateRuntimeCompileAndExecute(Socket socket) throws Ex
     TemplateCompiler compiler = new TemplateCompiler(input);
     String output = (String) TemplateRuntime.execute(compiler.compile(), new HashMap());
   }
+
+  public static void testMvelRuntimeExecute(Socket socket) throws Exception {
+    InputStream in = socket.getInputStream();
+
+    byte[] bytes = new byte[1024];
+    int n = in.read(bytes);
+    String input = new String(bytes, 0, n);
+
+    ExpressionCompiler compiler = new ExpressionCompiler(input);
+    CompiledExpression expression = compiler.compile();
+    MVELRuntime.execute(false, expression, new Object(), new ImmutableDefaultFactory());
+  }
 }
diff --git a/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/MVELRuntime.java b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/MVELRuntime.java
new file mode 100644
index 000000000000..30fef49f0b88
--- /dev/null
+++ b/java/ql/test/stubs/mvel2-2.4.7/org/mvel2/MVELRuntime.java
@@ -0,0 +1,8 @@
+package org.mvel2;
+
+import org.mvel2.compiler.CompiledExpression;
+import org.mvel2.integration.VariableResolverFactory;
+
+public class MVELRuntime {
+  public static Object execute(boolean debugger, CompiledExpression expression, Object ctx, VariableResolverFactory variableFactory) { return null; }
+}
\ No newline at end of file

From 4a83fb8cc17ee4a5fc476e86ccc247443160ee75 Mon Sep 17 00:00:00 2001
From: Artem Smotrakov <artem.smotrakov@gmail.com>
Date: Fri, 1 May 2020 17:13:11 +0200
Subject: [PATCH 08/10] Java: Simplified MvelInjection test

---
 .../CWE/CWE-094/MvelInjection.expected        | 110 ++++++++++--------
 .../Security/CWE/CWE-094/MvelInjection.java   | 107 +++++------------
 2 files changed, 89 insertions(+), 128 deletions(-)

diff --git a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected
index c40da7f5f870..b2325fa78a42 100644
--- a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected
+++ b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.expected
@@ -1,53 +1,63 @@
 edges
-| MvelInjection.java:25:27:25:49 | getInputStream(...) : InputStream | MvelInjection.java:29:17:29:21 | input |
-| MvelInjection.java:34:27:34:49 | getInputStream(...) : InputStream | MvelInjection.java:39:30:39:39 | expression |
-| MvelInjection.java:44:27:44:49 | getInputStream(...) : InputStream | MvelInjection.java:50:7:50:15 | statement |
-| MvelInjection.java:44:27:44:49 | getInputStream(...) : InputStream | MvelInjection.java:51:7:51:15 | statement |
-| MvelInjection.java:56:27:56:49 | getInputStream(...) : InputStream | MvelInjection.java:62:7:62:16 | expression |
-| MvelInjection.java:67:27:67:49 | getInputStream(...) : InputStream | MvelInjection.java:72:7:72:16 | expression |
-| MvelInjection.java:77:22:77:44 | getInputStream(...) : InputStream | MvelInjection.java:85:5:85:18 | compiledScript |
-| MvelInjection.java:77:22:77:44 | getInputStream(...) : InputStream | MvelInjection.java:88:21:88:26 | script |
-| MvelInjection.java:92:22:92:44 | getInputStream(...) : InputStream | MvelInjection.java:102:5:102:10 | script |
-| MvelInjection.java:106:22:106:44 | getInputStream(...) : InputStream | MvelInjection.java:112:26:112:30 | input |
-| MvelInjection.java:116:22:116:44 | getInputStream(...) : InputStream | MvelInjection.java:122:29:122:67 | compileTemplate(...) |
-| MvelInjection.java:126:22:126:44 | getInputStream(...) : InputStream | MvelInjection.java:133:54:133:71 | compile(...) |
-| MvelInjection.java:137:22:137:44 | getInputStream(...) : InputStream | MvelInjection.java:145:32:145:41 | expression |
+| 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: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:27:25:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:29:17:29:21 | input | semmle.label | input |
-| MvelInjection.java:34:27:34:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:39:30:39:39 | expression | semmle.label | expression |
-| MvelInjection.java:44:27:44:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:50:7:50:15 | statement | semmle.label | statement |
-| MvelInjection.java:51:7:51:15 | statement | semmle.label | statement |
-| MvelInjection.java:56:27:56:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:62:7:62:16 | expression | semmle.label | expression |
-| MvelInjection.java:67:27:67:49 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:72:7:72:16 | expression | semmle.label | expression |
-| MvelInjection.java:77:22:77:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:85:5:85:18 | compiledScript | semmle.label | compiledScript |
-| MvelInjection.java:88:21:88:26 | script | semmle.label | script |
-| MvelInjection.java:92:22:92:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:102:5:102:10 | script | semmle.label | script |
-| MvelInjection.java:106:22:106:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:112:26:112:30 | input | semmle.label | input |
-| MvelInjection.java:116:22:116:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:122:29:122:67 | compileTemplate(...) | semmle.label | compileTemplate(...) |
-| MvelInjection.java:126:22:126:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:133:54:133:71 | compile(...) | semmle.label | compile(...) |
-| MvelInjection.java:137:22:137:44 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
-| MvelInjection.java:145:32:145:41 | expression | semmle.label | expression |
+| 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:95:14:95:36 | new String(...) : String | semmle.label | new String(...) : String |
 #select
-| MvelInjection.java:29:17:29:21 | input | MvelInjection.java:25:27:25:49 | getInputStream(...) : InputStream | MvelInjection.java:29:17:29:21 | input | MVEL injection from $@. | MvelInjection.java:25:27:25:49 | getInputStream(...) | this user input |
-| MvelInjection.java:39:30:39:39 | expression | MvelInjection.java:34:27:34:49 | getInputStream(...) : InputStream | MvelInjection.java:39:30:39:39 | expression | MVEL injection from $@. | MvelInjection.java:34:27:34:49 | getInputStream(...) | this user input |
-| MvelInjection.java:50:7:50:15 | statement | MvelInjection.java:44:27:44:49 | getInputStream(...) : InputStream | MvelInjection.java:50:7:50:15 | statement | MVEL injection from $@. | MvelInjection.java:44:27:44:49 | getInputStream(...) | this user input |
-| MvelInjection.java:51:7:51:15 | statement | MvelInjection.java:44:27:44:49 | getInputStream(...) : InputStream | MvelInjection.java:51:7:51:15 | statement | MVEL injection from $@. | MvelInjection.java:44:27:44:49 | getInputStream(...) | this user input |
-| MvelInjection.java:62:7:62:16 | expression | MvelInjection.java:56:27:56:49 | getInputStream(...) : InputStream | MvelInjection.java:62:7:62:16 | expression | MVEL injection from $@. | MvelInjection.java:56:27:56:49 | getInputStream(...) | this user input |
-| MvelInjection.java:72:7:72:16 | expression | MvelInjection.java:67:27:67:49 | getInputStream(...) : InputStream | MvelInjection.java:72:7:72:16 | expression | MVEL injection from $@. | MvelInjection.java:67:27:67:49 | getInputStream(...) | this user input |
-| MvelInjection.java:85:5:85:18 | compiledScript | MvelInjection.java:77:22:77:44 | getInputStream(...) : InputStream | MvelInjection.java:85:5:85:18 | compiledScript | MVEL injection from $@. | MvelInjection.java:77:22:77:44 | getInputStream(...) | this user input |
-| MvelInjection.java:88:21:88:26 | script | MvelInjection.java:77:22:77:44 | getInputStream(...) : InputStream | MvelInjection.java:88:21:88:26 | script | MVEL injection from $@. | MvelInjection.java:77:22:77:44 | getInputStream(...) | this user input |
-| MvelInjection.java:102:5:102:10 | script | MvelInjection.java:92:22:92:44 | getInputStream(...) : InputStream | MvelInjection.java:102:5:102:10 | script | MVEL injection from $@. | MvelInjection.java:92:22:92:44 | getInputStream(...) | this user input |
-| MvelInjection.java:112:26:112:30 | input | MvelInjection.java:106:22:106:44 | getInputStream(...) : InputStream | MvelInjection.java:112:26:112:30 | input | MVEL injection from $@. | MvelInjection.java:106:22:106:44 | getInputStream(...) | this user input |
-| MvelInjection.java:122:29:122:67 | compileTemplate(...) | MvelInjection.java:116:22:116:44 | getInputStream(...) : InputStream | MvelInjection.java:122:29:122:67 | compileTemplate(...) | MVEL injection from $@. | MvelInjection.java:116:22:116:44 | getInputStream(...) | this user input |
-| MvelInjection.java:133:54:133:71 | compile(...) | MvelInjection.java:126:22:126:44 | getInputStream(...) : InputStream | MvelInjection.java:133:54:133:71 | compile(...) | MVEL injection from $@. | MvelInjection.java:126:22:126:44 | getInputStream(...) | this user input |
-| MvelInjection.java:145:32:145:41 | expression | MvelInjection.java:137:22:137:44 | getInputStream(...) : InputStream | MvelInjection.java:145:32:145:41 | expression | MVEL injection from $@. | MvelInjection.java:137:22:137:44 | getInputStream(...) | this user input |
+| 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/Security/CWE/CWE-094/MvelInjection.java b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java
index 4454a782bf89..88f9b861caee 100644
--- a/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java
+++ b/java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.java
@@ -22,63 +22,35 @@
 public class MvelInjection {
 
   public static void testWithMvelEval(Socket socket) throws IOException {
-    try (InputStream in = socket.getInputStream()) {
-      byte[] bytes = new byte[1024];
-      int n = in.read(bytes);
-      String input = new String(bytes, 0, n);
-      MVEL.eval(input);
-    }
+    MVEL.eval(read(socket));
   }
 
   public static void testWithMvelCompileAndExecute(Socket socket) throws IOException {
-    try (InputStream in = socket.getInputStream()) {
-      byte[] bytes = new byte[1024];
-      int n = in.read(bytes);
-      String input = new String(bytes, 0, n);
-      Serializable expression = MVEL.compileExpression(input);
-      MVEL.executeExpression(expression);
-    }
+    Serializable expression = MVEL.compileExpression(read(socket));
+    MVEL.executeExpression(expression);
   }
 
   public static void testWithExpressionCompiler(Socket socket) throws IOException {
-    try (InputStream in = socket.getInputStream()) {
-      byte[] bytes = new byte[1024];
-      int n = in.read(bytes);
-      String input = new String(bytes, 0, n);
-      ExpressionCompiler compiler = new ExpressionCompiler(input);
-      ExecutableStatement statement = compiler.compile();
-      statement.getValue(new Object(), new ImmutableDefaultFactory());
-      statement.getValue(new Object(), new Object(), new ImmutableDefaultFactory());
-    }
+    ExpressionCompiler compiler = new ExpressionCompiler(read(socket));
+    ExecutableStatement statement = compiler.compile();
+    statement.getValue(new Object(), new ImmutableDefaultFactory());
+    statement.getValue(new Object(), new Object(), new ImmutableDefaultFactory());
   }
 
   public static void testWithCompiledExpressionGetDirectValue(Socket socket) throws IOException {
-    try (InputStream in = socket.getInputStream()) {
-      byte[] bytes = new byte[1024];
-      int n = in.read(bytes);
-      String input = new String(bytes, 0, n);
-      ExpressionCompiler compiler = new ExpressionCompiler(input);
-      CompiledExpression expression = compiler.compile();
-      expression.getDirectValue(new Object(), new ImmutableDefaultFactory());
-    }
+    ExpressionCompiler compiler = new ExpressionCompiler(read(socket));
+    CompiledExpression expression = compiler.compile();
+    expression.getDirectValue(new Object(), new ImmutableDefaultFactory());
   }
 
   public static void testCompiledAccExpressionGetValue(Socket socket) throws IOException {
-    try (InputStream in = socket.getInputStream()) {
-      byte[] bytes = new byte[1024];
-      int n = in.read(bytes);
-      String input = new String(bytes, 0, n);
-      CompiledAccExpression expression = new CompiledAccExpression(input.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());
   }
 
   public static void testMvelScriptEngineCompileAndEvaluate(Socket socket) throws Exception {
-    InputStream in = socket.getInputStream();
-
-    byte[] bytes = new byte[1024];
-    int n = in.read(bytes);
-    String input = new String(bytes, 0, n);
+    String input = read(socket);
 
     MvelScriptEngine engine = new MvelScriptEngine();
     CompiledScript compiledScript = engine.compile(input);
@@ -89,59 +61,38 @@ public static void testMvelScriptEngineCompileAndEvaluate(Socket socket) throws
   }
 
   public static void testMvelCompiledScriptCompileAndEvaluate(Socket socket) throws Exception {
-    InputStream in = socket.getInputStream();
-
-    byte[] bytes = new byte[1024];
-    int n = in.read(bytes);
-    String input = new String(bytes, 0, n);
-
     MvelScriptEngine engine = new MvelScriptEngine();
-    ExpressionCompiler compiler = new ExpressionCompiler(input);
+    ExpressionCompiler compiler = new ExpressionCompiler(read(socket));
     ExecutableStatement statement = compiler.compile();
     MvelCompiledScript script = new MvelCompiledScript(engine, statement);
     script.eval(new SimpleScriptContext());
   }
 
   public static void testTemplateRuntimeEval(Socket socket) throws Exception {
-    InputStream in = socket.getInputStream();
-
-    byte[] bytes = new byte[1024];
-    int n = in.read(bytes);
-    String input = new String(bytes, 0, n);
-
-    TemplateRuntime.eval(input, new HashMap());
+    TemplateRuntime.eval(read(socket), new HashMap());
   }
 
   public static void testTemplateRuntimeCompileTemplateAndExecute(Socket socket) throws Exception {
-    InputStream in = socket.getInputStream();
-
-    byte[] bytes = new byte[1024];
-    int n = in.read(bytes);
-    String input = new String(bytes, 0, n);
-
-    TemplateRuntime.execute(TemplateCompiler.compileTemplate(input), new HashMap());
+    TemplateRuntime.execute(
+      TemplateCompiler.compileTemplate(read(socket)), new HashMap());
   }
 
   public static void testTemplateRuntimeCompileAndExecute(Socket socket) throws Exception {
-    InputStream in = socket.getInputStream();
-
-    byte[] bytes = new byte[1024];
-    int n = in.read(bytes);
-    String input = new String(bytes, 0, n);
-
-    TemplateCompiler compiler = new TemplateCompiler(input);
-    String output = (String) TemplateRuntime.execute(compiler.compile(), new HashMap());
+    TemplateCompiler compiler = new TemplateCompiler(read(socket));
+    TemplateRuntime.execute(compiler.compile(), new HashMap());
   }
 
   public static void testMvelRuntimeExecute(Socket socket) throws Exception {
-    InputStream in = socket.getInputStream();
-
-    byte[] bytes = new byte[1024];
-    int n = in.read(bytes);
-    String input = new String(bytes, 0, n);
-
-    ExpressionCompiler compiler = new ExpressionCompiler(input);
+    ExpressionCompiler compiler = new ExpressionCompiler(read(socket));
     CompiledExpression expression = compiler.compile();
     MVELRuntime.execute(false, expression, new Object(), new ImmutableDefaultFactory());
   }
+
+  public static String read(Socket socket) throws IOException {
+    try (InputStream is = socket.getInputStream()) {
+      byte[] bytes = new byte[1024];
+      int n = is.read(bytes);
+      return new String(bytes, 0, n);
+    }
+  }
 }

From 2842aeee72cf79d8f7c66d56017155523502b2a1 Mon Sep 17 00:00:00 2001
From: Artem Smotrakov <artem.smotrakov@gmail.com>
Date: Sun, 31 May 2020 21:08:07 +0300
Subject: [PATCH 09/10] Java: Simplified MvelInjectionLib

---
 .../Security/CWE/CWE-094/MvelInjectionLib.qll   | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll b/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
index 5bd6c122015b..791ee0b32eb9 100644
--- a/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
+++ b/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
@@ -1,7 +1,6 @@
 import java
 import semmle.code.java.dataflow.FlowSources
 import semmle.code.java.dataflow.TaintTracking
-import DataFlow::PathGraph
 
 /**
  * A taint-tracking configuration for unsafe user input
@@ -54,7 +53,7 @@ class MvelEvaluationSink extends DataFlow::ExprNode {
         m instanceof CompiledScriptEvaluationMethod or
         m instanceof MvelCompiledScriptEvaluationMethod
       ) and
-      (ma = asExpr() or ma.getQualifier() = asExpr())
+      ma.getQualifier() = asExpr()
     )
     or
     exists(StaticMethodAccess ma, Method m | m = ma.getMethod() |
@@ -73,7 +72,7 @@ predicate expressionCompilationStep(DataFlow::Node node1, DataFlow::Node node2)
     m.getDeclaringType() instanceof MVEL and
     m.hasName("compileExpression") and
     ma.getAnArgument() = node1.asExpr() and
-    (node2.asExpr() = ma.getQualifier() or node2.asExpr() = ma)
+    node2.asExpr() = ma
   )
 }
 
@@ -84,7 +83,7 @@ predicate expressionCompilationStep(DataFlow::Node node1, DataFlow::Node node2)
 predicate createExpressionCompilerStep(DataFlow::Node node1, DataFlow::Node node2) {
   exists(ConstructorCall cc |
     cc.getConstructedType() instanceof ExpressionCompiler and
-    (cc = node2.asExpr() or cc.getQualifier() = node2.asExpr()) and
+    cc = node2.asExpr() and
     cc.getArgument(0) = node1.asExpr()
   )
 }
@@ -96,7 +95,7 @@ predicate createExpressionCompilerStep(DataFlow::Node node1, DataFlow::Node node
 predicate createCompiledAccExpressionStep(DataFlow::Node node1, DataFlow::Node node2) {
   exists(ConstructorCall cc |
     cc.getConstructedType() instanceof CompiledAccExpression and
-    (cc = node2.asExpr() or cc.getQualifier() = node2.asExpr()) and
+    cc = node2.asExpr() and
     cc.getArgument(0) = node1.asExpr()
   )
 }
@@ -125,7 +124,7 @@ predicate expressionCompilerCompileStep(DataFlow::Node node1, DataFlow::Node nod
 predicate scriptCompileStep(DataFlow::Node node1, DataFlow::Node node2) {
   exists(MethodAccess ma, Method m | ma.getMethod() = m |
     m instanceof MvelScriptEngineCompilationMethod and
-    (ma = node2.asExpr() or ma.getQualifier() = node2.asExpr()) and
+    ma = node2.asExpr() and
     ma.getArgument(0) = node1.asExpr()
   )
 }
@@ -137,7 +136,7 @@ predicate scriptCompileStep(DataFlow::Node node1, DataFlow::Node node2) {
 predicate createMvelCompiledScriptStep(DataFlow::Node node1, DataFlow::Node node2) {
   exists(ConstructorCall cc |
     cc.getConstructedType() instanceof MvelCompiledScript and
-    (cc = node2.asExpr() or cc.getQualifier() = node2.asExpr()) and
+    cc = node2.asExpr() and
     cc.getArgument(1) = node1.asExpr()
   )
 }
@@ -149,7 +148,7 @@ predicate createMvelCompiledScriptStep(DataFlow::Node node1, DataFlow::Node node
 predicate createTemplateCompilerStep(DataFlow::Node node1, DataFlow::Node node2) {
   exists(ConstructorCall cc |
     cc.getConstructedType() instanceof TemplateCompiler and
-    (cc = node2.asExpr() or cc.getQualifier() = node2.asExpr()) and
+    cc = node2.asExpr() and
     cc.getArgument(0) = node1.asExpr()
   )
 }
@@ -167,7 +166,7 @@ predicate templateCompileStep(DataFlow::Node node1, DataFlow::Node node2) {
   or
   exists(StaticMethodAccess ma, Method m | ma.getMethod() = m |
     m instanceof TemplateCompilerCompileTemplateMethod and
-    (ma = node2.asExpr() or ma.getQualifier() = node2.asExpr()) and
+    ma = node2.asExpr() and
     ma.getArgument(0) = node1.asExpr()
   )
 }

From b7c3dd666cea52c2b32b66fe38165d74ec0a18e9 Mon Sep 17 00:00:00 2001
From: Artem Smotrakov <artem.smotrakov@gmail.com>
Date: Fri, 5 Jun 2020 17:22:45 +0300
Subject: [PATCH 10/10] Java: Clean up MVEL injection query

---
 .../src/experimental/Security/CWE/CWE-094/MvelInjection.qhelp | 1 +
 .../experimental/Security/CWE/CWE-094/MvelInjectionLib.qll    | 4 ----
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjection.qhelp b/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjection.qhelp
index 1055847020f6..d68d298b5f5a 100644
--- a/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjection.qhelp
+++ b/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjection.qhelp
@@ -23,6 +23,7 @@ The following example uses untrusted data to build a MVEL expression
 and then runs it in the default powerfull context.
 </p>
 <sample src="UnsafeMvelExpressionEvaluation.java" />
+</example>
 
 <references>
 <li>
diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll b/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
index 791ee0b32eb9..a6cf891330f0 100644
--- a/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
+++ b/java/ql/src/experimental/Security/CWE/CWE-094/MvelInjectionLib.qll
@@ -100,10 +100,6 @@ predicate createCompiledAccExpressionStep(DataFlow::Node node1, DataFlow::Node n
   )
 }
 
-predicate test() {
-  exists(ConstructorCall cc | cc.getConstructedType() instanceof CompiledAccExpression)
-}
-
 /**
  * Holds if `node1` to `node2` is a dataflow step that compiles a MVEL expression
  * by calling `ExpressionCompiler.compile()`.