Decouple SpelInjection.qll to reuse the taint tracking configuration

Author: atorralba

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

@@ -11,25 +11,9 @@
  */
 
 import java
-import semmle.code.java.security.SpelInjection
+import semmle.code.java.security.SpelInjectionQuery
 import DataFlow::PathGraph
 
-/**
- * A taint-tracking configuration for unsafe user input
- * that is used to construct and evaluate a SpEL expression.
- */
-class SpELInjectionConfig extends TaintTracking::Configuration {
-  SpELInjectionConfig() { this = "SpELInjectionConfig" }
-
-  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
-
-  override predicate isSink(DataFlow::Node sink) { sink instanceof SpelExpressionEvaluationSink }
-
-  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
-    any(SpelExpressionInjectionAdditionalTaintStep c).step(node1, node2)
-  }
-}
-
-from DataFlow::PathNode source, DataFlow::PathNode sink, SpELInjectionConfig conf
+from DataFlow::PathNode source, DataFlow::PathNode sink, SpelInjectionConfig conf
 where conf.hasFlowPath(source, sink)
 select sink.getNode(), source, sink, "SpEL injection from $@.", source.getNode(), "this user input"

…mle/code/java/security/SpelInjection.qll …ode/java/security/SpelInjectionQuery.qlljava/ql/src/semmle/code/java/security/SpelInjection.qll renamed to java/ql/src/semmle/code/java/security/SpelInjectionQuery.qll java/ql/src/semmle/code/java/security/SpelInjection.qll renamed to java/ql/src/semmle/code/java/security/SpelInjectionQuery.qll

@@ -2,24 +2,11 @@
 
 import java
 import semmle.code.java.dataflow.DataFlow
-import semmle.code.java.dataflow.ExternalFlow
 import semmle.code.java.dataflow.FlowSources
 
 /** A data flow sink for unvalidated user input that is used to construct SpEL expressions. */
 abstract class SpelExpressionEvaluationSink extends DataFlow::ExprNode { }
 
-private class SpelExpressionEvaluationModel extends SinkModelCsv {
-  override predicate row(string row) {
-    row =
-      [
-        "org.springframework.expression;Expression;true;getValue;;;Argument[-1];spel",
-        "org.springframework.expression;Expression;true;getValueTypeDescriptor;;;Argument[-1];spel",
-        "org.springframework.expression;Expression;true;getValueType;;;Argument[-1];spel",
-        "org.springframework.expression;Expression;true;setValue;;;Argument[-1];spel"
-      ]
-  }
-}
-
 /** Default sink for SpEL injection vulnerabilities. */
 private class DefaultSpelExpressionEvaluationSink extends SpelExpressionEvaluationSink {
   DefaultSpelExpressionEvaluationSink() {
@@ -53,6 +40,22 @@ private class DefaultSpelExpressionInjectionAdditionalTaintStep extends SpelExpr
   }
 }
 
+/**
+ * A taint-tracking configuration for unsafe user input
+ * that is used to construct and evaluate a SpEL expression.
+ */
+class SpelInjectionConfig extends TaintTracking::Configuration {
+  SpelInjectionConfig() { this = "SpelInjectionConfig" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof SpelExpressionEvaluationSink }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
+    any(SpelExpressionInjectionAdditionalTaintStep c).step(node1, node2)
+  }
+}
+
 /**
  * A configuration for safe evaluation context that may be used in expression evaluation.
  */

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

@@ -0,0 +1,15 @@
+/** Provides sink models relating to SpEL injection vulnerabilities. */
+
+import semmle.code.java.dataflow.ExternalFlow
+
+private class SpelExpressionEvaluationModel extends SinkModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        "org.springframework.expression;Expression;true;getValue;;;Argument[-1];spel",
+        "org.springframework.expression;Expression;true;getValueTypeDescriptor;;;Argument[-1];spel",
+        "org.springframework.expression;Expression;true;getValueType;;;Argument[-1];spel",
+        "org.springframework.expression;Expression;true;setValue;;;Argument[-1];spel"
+      ]
+  }
+}

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

@@ -1,29 +1,19 @@
 import java
 import semmle.code.java.dataflow.TaintTracking
 import semmle.code.java.dataflow.FlowSources
-import semmle.code.java.security.SpelInjection
+import semmle.code.java.security.SpelInjectionQuery
 import TestUtilities.InlineExpectationsTest
 
-class Conf extends TaintTracking::Configuration {
-  Conf() { this = "test:cwe:spel-injection" }
-
-  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
-
-  override predicate isSink(DataFlow::Node sink) { sink instanceof SpelExpressionEvaluationSink }
-
-  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
-    any(SpelExpressionInjectionAdditionalTaintStep c).step(node1, node2)
-  }
-}
-
 class HasSpelInjectionTest extends InlineExpectationsTest {
   HasSpelInjectionTest() { this = "HasSpelInjectionTest" }
 
   override string getARelevantTag() { result = "hasSpelInjection" }
 
   override predicate hasActualResult(Location location, string element, string tag, string value) {
     tag = "hasSpelInjection" and
-    exists(DataFlow::Node src, DataFlow::Node sink, Conf conf | conf.hasFlow(src, sink) |
+    exists(DataFlow::Node src, DataFlow::Node sink, SpelInjectionConfig conf |
+      conf.hasFlow(src, sink)
+    |
       sink.getLocation() = location and
       element = sink.toString() and
       value = ""