Refactor SpelInjectionQuery

Author: egregius313

java/ql/lib/semmle/code/java/security/SpelInjectionQuery.qll

@@ -7,10 +7,12 @@ private import semmle.code.java.frameworks.spring.SpringExpression
 private import semmle.code.java.security.SpelInjection
 
 /**
+ * DEPRECATED: Use `SpelInjectionFlow` instead.
+ *
  * A taint-tracking configuration for unsafe user input
  * that is used to construct and evaluate a SpEL expression.
  */
-class SpelInjectionConfig extends TaintTracking::Configuration {
+deprecated class SpelInjectionConfig extends TaintTracking::Configuration {
   SpelInjectionConfig() { this = "SpelInjectionConfig" }
 
   override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
@@ -22,37 +24,52 @@ class SpelInjectionConfig extends TaintTracking::Configuration {
   }
 }
 
+/**
+ * A taint-tracking configuration for unsafe user input
+ * that is used to construct and evaluate a SpEL expression.
+ */
+private module SpelInjectionConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  predicate isSink(DataFlow::Node sink) { sink instanceof SpelExpressionEvaluationSink }
+
+  predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+    any(SpelExpressionInjectionAdditionalTaintStep c).step(node1, node2)
+  }
+}
+
+/** Tracks flow of unsafe user input that is used to construct and evaluate a SpEL expression. */
+module SpelInjectionFlow = TaintTracking::Make<SpelInjectionConfig>;
+
 /** Default sink for SpEL injection vulnerabilities. */
 private class DefaultSpelExpressionEvaluationSink extends SpelExpressionEvaluationSink {
   DefaultSpelExpressionEvaluationSink() {
     exists(MethodAccess ma |
       ma.getMethod() instanceof ExpressionEvaluationMethod and
       ma.getQualifier() = this.asExpr() and
-      not exists(SafeEvaluationContextFlowConfig config |
-        config.hasFlowTo(DataFlow::exprNode(ma.getArgument(0)))
-      )
+      not SafeEvaluationContextFlow::hasFlowToExpr(ma.getArgument(0))
     )
   }
 }
 
 /**
  * A configuration for safe evaluation context that may be used in expression evaluation.
  */
-private class SafeEvaluationContextFlowConfig extends DataFlow2::Configuration {
-  SafeEvaluationContextFlowConfig() { this = "SpelInjection::SafeEvaluationContextFlowConfig" }
-
-  override predicate isSource(DataFlow::Node source) { source instanceof SafeContextSource }
+private module SafeEvaluationContextFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof SafeContextSource }
 
-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
     exists(MethodAccess ma |
       ma.getMethod() instanceof ExpressionEvaluationMethod and
       ma.getArgument(0) = sink.asExpr()
     )
   }
 
-  override int fieldFlowBranchLimit() { result = 0 }
+  int fieldFlowBranchLimit() { result = 0 }
 }
 
+private module SafeEvaluationContextFlow = DataFlow::Make<SafeEvaluationContextFlowConfig>;
+
 /**
  * A `ContextSource` that is safe from SpEL injection.
  */

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

@@ -14,9 +14,9 @@
 import java
 import semmle.code.java.security.SpelInjectionQuery
 import semmle.code.java.dataflow.DataFlow
-import DataFlow::PathGraph
+import SpelInjectionFlow::PathGraph
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, SpelInjectionConfig conf
-where conf.hasFlowPath(source, sink)
+from SpelInjectionFlow::PathNode source, SpelInjectionFlow::PathNode sink
+where SpelInjectionFlow::hasFlowPath(source, sink)
 select sink.getNode(), source, sink, "SpEL expression depends on a $@.", source.getNode(),
   "user-provided value"

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

@@ -11,7 +11,7 @@ class HasSpelInjectionTest extends InlineExpectationsTest {
 
   override predicate hasActualResult(Location location, string element, string tag, string value) {
     tag = "hasSpelInjection" and
-    exists(DataFlow::Node sink, SpelInjectionConfig conf | conf.hasFlowTo(sink) |
+    exists(DataFlow::Node sink | SpelInjectionFlow::hasFlowTo(sink) |
       sink.getLocation() = location and
       element = sink.toString() and
       value = ""