Decouple MvelInjection.qll to reuse the taint tracking configuration

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

@@ -11,31 +11,9 @@
  */
 
 import java
-import semmle.code.java.dataflow.FlowSources
-import semmle.code.java.dataflow.TaintTracking
-import semmle.code.java.security.MvelInjection
+import semmle.code.java.security.MvelInjectionQuery
 import DataFlow::PathGraph
 
-/**
- * A taint-tracking configuration for unsafe user input
- * that is used to construct and evaluate a MVEL expression.
- */
-class MvelInjectionFlowConfig extends TaintTracking::Configuration {
-  MvelInjectionFlowConfig() { this = "MvelInjectionFlowConfig" }
-
-  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
-
-  override predicate isSink(DataFlow::Node sink) { sink instanceof MvelEvaluationSink }
-
-  override predicate isSanitizer(DataFlow::Node sanitizer) {
-    sanitizer instanceof MvelInjectionSanitizer
-  }
-
-  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
-    any(MvelInjectionAdditionalTaintStep c).step(node1, node2)
-  }
-}
-
 from DataFlow::PathNode source, DataFlow::PathNode sink, MvelInjectionFlowConfig conf
 where conf.hasFlowPath(source, sink)
 select sink.getNode(), source, sink, "MVEL injection from $@.", source.getNode(), "this user input"

java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll

@@ -98,7 +98,7 @@ private module Frameworks {
   private import semmle.code.java.security.InformationLeak
   private import semmle.code.java.security.JexlInjectionSinkModels
   private import semmle.code.java.security.LdapInjection
-  private import semmle.code.java.security.MvelInjection
+  private import semmle.code.java.security.MvelInjectionSinkModels
   private import semmle.code.java.security.XPath
   private import semmle.code.java.frameworks.android.SQLite
   private import semmle.code.java.frameworks.Jdbc

java/ql/src/semmle/code/java/security/MvelInjectionQuery.qll

@@ -3,6 +3,8 @@
 import java
 import semmle.code.java.dataflow.DataFlow
 import semmle.code.java.dataflow.ExternalFlow
+import semmle.code.java.dataflow.FlowSources
+import semmle.code.java.dataflow.TaintTracking
 
 /** A data flow sink for unvalidated user input that is used to construct MVEL expressions. */
 abstract class MvelEvaluationSink extends DataFlow::Node { }
@@ -23,31 +25,6 @@ class MvelInjectionAdditionalTaintStep extends Unit {
   abstract predicate step(DataFlow::Node n1, DataFlow::Node n2);
 }
 
-private class DefaulMvelEvaluationSinkModel extends SinkModelCsv {
-  override predicate row(string row) {
-    row =
-      [
-        "javax.script;CompiledScript;false;eval;;;Argument[-1];mvel",
-        "org.mvel2;MVEL;false;eval;;;Argument[0];mvel",
-        "org.mvel2;MVEL;false;executeExpression;;;Argument[0];mvel",
-        "org.mvel2;MVEL;false;evalToBoolean;;;Argument[0];mvel",
-        "org.mvel2;MVEL;false;evalToString;;;Argument[0];mvel",
-        "org.mvel2;MVEL;false;executeAllExpression;;;Argument[0];mvel",
-        "org.mvel2;MVEL;false;executeSetExpression;;;Argument[0];mvel",
-        "org.mvel2;MVELRuntime;false;execute;;;Argument[1];mvel",
-        "org.mvel2.templates;TemplateRuntime;false;eval;;;Argument[0];mvel",
-        "org.mvel2.templates;TemplateRuntime;false;execute;;;Argument[0];mvel",
-        "org.mvel2.jsr223;MvelScriptEngine;false;eval;;;Argument[0];mvel",
-        "org.mvel2.jsr223;MvelScriptEngine;false;evaluate;;;Argument[0];mvel",
-        "org.mvel2.jsr223;MvelCompiledScript;false;eval;;;Argument[-1];mvel",
-        "org.mvel2.compiler;ExecutableStatement;false;getValue;;;Argument[-1];mvel",
-        "org.mvel2.compiler;CompiledExpression;false;getDirectValue;;;Argument[-1];mvel",
-        "org.mvel2.compiler;CompiledAccExpression;false;getValue;;;Argument[-1];mvel",
-        "org.mvel2.compiler;Accessor;false;getValue;;;Argument[-1];mvel"
-      ]
-  }
-}
-
 /** Default sink for MVEL injection vulnerabilities. */
 private class DefaultMvelEvaluationSink extends MvelEvaluationSink {
   DefaultMvelEvaluationSink() { sinkNode(this, "mvel") }
@@ -74,6 +51,26 @@ private class DefaultMvelInjectionAdditionalTaintStep extends MvelInjectionAddit
   }
 }
 
+/**
+ * A taint-tracking configuration for unsafe user input
+ * that is used to construct and evaluate a MVEL expression.
+ */
+class MvelInjectionFlowConfig extends TaintTracking::Configuration {
+  MvelInjectionFlowConfig() { this = "MvelInjectionFlowConfig" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof MvelEvaluationSink }
+
+  override predicate isSanitizer(DataFlow::Node sanitizer) {
+    sanitizer instanceof MvelInjectionSanitizer
+  }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
+    any(MvelInjectionAdditionalTaintStep c).step(node1, node2)
+  }
+}
+
 /**
  * Holds if `node1` to `node2` is a dataflow step that compiles a MVEL expression
  * by callilng `MVEL.compileExpression(tainted)`.

java/ql/src/semmle/code/java/security/MvelInjectionSinkModels.qll

@@ -0,0 +1,28 @@
+/** Provides sink models relating to MVEL injection vulnerabilities. */
+
+import semmle.code.java.dataflow.ExternalFlow
+
+private class DefaulMvelEvaluationSinkModel extends SinkModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        "javax.script;CompiledScript;false;eval;;;Argument[-1];mvel",
+        "org.mvel2;MVEL;false;eval;;;Argument[0];mvel",
+        "org.mvel2;MVEL;false;executeExpression;;;Argument[0];mvel",
+        "org.mvel2;MVEL;false;evalToBoolean;;;Argument[0];mvel",
+        "org.mvel2;MVEL;false;evalToString;;;Argument[0];mvel",
+        "org.mvel2;MVEL;false;executeAllExpression;;;Argument[0];mvel",
+        "org.mvel2;MVEL;false;executeSetExpression;;;Argument[0];mvel",
+        "org.mvel2;MVELRuntime;false;execute;;;Argument[1];mvel",
+        "org.mvel2.templates;TemplateRuntime;false;eval;;;Argument[0];mvel",
+        "org.mvel2.templates;TemplateRuntime;false;execute;;;Argument[0];mvel",
+        "org.mvel2.jsr223;MvelScriptEngine;false;eval;;;Argument[0];mvel",
+        "org.mvel2.jsr223;MvelScriptEngine;false;evaluate;;;Argument[0];mvel",
+        "org.mvel2.jsr223;MvelCompiledScript;false;eval;;;Argument[-1];mvel",
+        "org.mvel2.compiler;ExecutableStatement;false;getValue;;;Argument[-1];mvel",
+        "org.mvel2.compiler;CompiledExpression;false;getDirectValue;;;Argument[-1];mvel",
+        "org.mvel2.compiler;CompiledAccExpression;false;getValue;;;Argument[-1];mvel",
+        "org.mvel2.compiler;Accessor;false;getValue;;;Argument[-1];mvel"
+      ]
+  }
+}

java/ql/test/experimental/query-tests/security/CWE-094/options

@@ -1,5 +1 @@
-<<<<<<< HEAD
-//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.2.3:${testdir}/../../../../stubs/jsr223-api:${testdir}/../../../../stubs/scriptengine:${testdir}/../../../../stubs/java-ee-el:${testdir}/../../../../stubs/juel-2.2:${testdir}/../../../stubs/groovy-all-3.0.7:${testdir}/../../../../stubs/servlet-api-2.4:${testdir}/../../../../stubs/jython-2.7.2:${testdir}/../../../../experimental/stubs/rhino-1.7.13
-=======
 //semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.3.8:${testdir}/../../../../stubs/jsr223-api:${testdir}/../../../../stubs/scriptengine:${testdir}/../../../../stubs/java-ee-el:${testdir}/../../../../stubs/juel-2.2:${testdir}/../../../stubs/groovy-all-3.0.7:${testdir}/../../../../stubs/servlet-api-2.4:${testdir}/../../../../stubs/jython-2.7.2:${testdir}/../../../../experimental/stubs/rhino-1.7.13:${testdir}/../../../../stubs/bsh-2.0b5:${testdir}/../../../../experimental/stubs/jshell
->>>>>>> main

java/ql/test/query-tests/security/CWE-094/MvelInjectionTest.ql

@@ -1,25 +1,9 @@
 import java
 import semmle.code.java.dataflow.TaintTracking
 import semmle.code.java.dataflow.FlowSources
-import semmle.code.java.security.MvelInjection
+import semmle.code.java.security.MvelInjectionQuery
 import TestUtilities.InlineExpectationsTest
 
-class Conf extends TaintTracking::Configuration {
-  Conf() { this = "test:cwe:mvel-injection" }
-
-  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
-
-  override predicate isSink(DataFlow::Node sink) { sink instanceof MvelEvaluationSink }
-
-  override predicate isSanitizer(DataFlow::Node sanitizer) {
-    sanitizer instanceof MvelInjectionSanitizer
-  }
-
-  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
-    any(MvelInjectionAdditionalTaintStep c).step(node1, node2)
-  }
-}
-
 class HasMvelInjectionTest extends InlineExpectationsTest {
   HasMvelInjectionTest() { this = "HasMvelInjectionTest" }
 
@@ -27,7 +11,9 @@ class HasMvelInjectionTest extends InlineExpectationsTest {
 
   override predicate hasActualResult(Location location, string element, string tag, string value) {
     tag = "hasMvelInjection" and
-    exists(DataFlow::Node src, DataFlow::Node sink, Conf conf | conf.hasFlow(src, sink) |
+    exists(DataFlow::Node src, DataFlow::Node sink, MvelInjectionFlowConfig conf |
+      conf.hasFlow(src, sink)
+    |
       sink.getLocation() = location and
       element = sink.toString() and
       value = ""