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

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>

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"

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

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

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 |

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

java/ql/test/experimental/Security/CWE/CWE-094/MvelInjection.qlref

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

java/ql/test/experimental/Security/CWE/CWE-094/options

@@ -1 +1 @@
-//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.2.3
+//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.2.3:${testdir}/../../../../stubs/mvel2-2.4.7

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

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

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

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

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