Java: Added JSR 223 sinks for MVEL injections

- Updated MvelInjectionLib.qll
- Added tests and stubs for JSR 223 API

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

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 |

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

@@ -2,6 +2,8 @@ import java.io.IOException;
 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.CompiledExpression;
 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 class MvelInjection {
       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());
+  }
 }

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

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

java/ql/test/stubs/jsr223-api/javax/script/ScriptContext.java

@@ -0,0 +1,3 @@
+package javax.script;
+
+public interface ScriptContext {}

java/ql/test/stubs/jsr223-api/javax/script/ScriptException.java

@@ -0,0 +1,3 @@
+package javax.script;
+
+public class ScriptException extends Exception {}

java/ql/test/stubs/jsr223-api/javax/script/SimpleScriptContext.java

@@ -0,0 +1,3 @@
+package javax.script;
+
+public class SimpleScriptContext implements ScriptContext {}

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

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

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