Merge pull request github#12639 from egregius313/egregius313/java/refactor-injection-queries

Java: Refactor injection queries to new dataflow API

Author: egregius313

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

@@ -6,10 +6,12 @@ import semmle.code.java.dataflow.TaintTracking
 import semmle.code.java.security.GroovyInjection
 
 /**
+ * DEPRECATED: Use `GroovyInjectionFlow` instead.
+ *
  * A taint-tracking configuration for unsafe user input
  * that is used to evaluate a Groovy expression.
  */
-class GroovyInjectionConfig extends TaintTracking::Configuration {
+deprecated class GroovyInjectionConfig extends TaintTracking::Configuration {
   GroovyInjectionConfig() { this = "GroovyInjectionConfig" }
 
   override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
@@ -20,3 +22,23 @@ class GroovyInjectionConfig extends TaintTracking::Configuration {
     any(GroovyInjectionAdditionalTaintStep c).step(fromNode, toNode)
   }
 }
+
+/**
+ * A taint-tracking configuration for unsafe user input
+ * that is used to evaluate a Groovy expression.
+ */
+module GroovyInjectionConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  predicate isSink(DataFlow::Node sink) { sink instanceof GroovyInjectionSink }
+
+  predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
+    any(GroovyInjectionAdditionalTaintStep c).step(fromNode, toNode)
+  }
+}
+
+/**
+ * Detect taint flow of unsafe user input
+ * that is used to evaluate a Groovy expression.
+ */
+module GroovyInjectionFlow = TaintTracking::Global<GroovyInjectionConfig>;

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

@@ -39,11 +39,13 @@ private class DefaultJexlInjectionAdditionalTaintStep extends JexlInjectionAddit
 }
 
 /**
+ * DEPRECATED: Use `JexlInjectionFlow` instead.
+ *
  * A taint-tracking configuration for unsafe user input
  * that is used to construct and evaluate a JEXL expression.
  * It supports both JEXL 2 and 3.
  */
-class JexlInjectionConfig extends TaintTracking::Configuration {
+deprecated class JexlInjectionConfig extends TaintTracking::Configuration {
   JexlInjectionConfig() { this = "JexlInjectionConfig" }
 
   override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
@@ -55,6 +57,27 @@ class JexlInjectionConfig extends TaintTracking::Configuration {
   }
 }
 
+/**
+ * A taint-tracking configuration for unsafe user input
+ * that is used to construct and evaluate a JEXL expression.
+ * It supports both JEXL 2 and 3.
+ */
+module JexlInjectionConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  predicate isSink(DataFlow::Node sink) { sink instanceof JexlEvaluationSink }
+
+  predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+    any(JexlInjectionAdditionalTaintStep c).step(node1, node2)
+  }
+}
+
+/**
+ * Tracks unsafe user input that is used to construct and evaluate a JEXL expression.
+ * It supports both JEXL 2 and 3.
+ */
+module JexlInjectionFlow = TaintTracking::Global<JexlInjectionConfig>;
+
 /**
  * Holds if `n1` to `n2` is a dataflow step that creates a JEXL script using an unsafe engine
  * by calling `tainted.createScript(jexlExpr)`.
@@ -99,19 +122,15 @@ private predicate createJexlTemplateStep(DataFlow::Node n1, DataFlow::Node n2) {
 /**
  * Holds if `expr` is a JEXL engine that is configured with a sandbox.
  */
-private predicate isSafeEngine(Expr expr) {
-  exists(SandboxedJexlFlowConfig config | config.hasFlowTo(DataFlow::exprNode(expr)))
-}
+private predicate isSafeEngine(Expr expr) { SandboxedJexlFlow::flowToExpr(expr) }
 
 /**
  * A configuration for tracking sandboxed JEXL engines.
  */
-private class SandboxedJexlFlowConfig extends DataFlow2::Configuration {
-  SandboxedJexlFlowConfig() { this = "JexlInjection::SandboxedJexlFlowConfig" }
+private module SandboxedJexlFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) { node instanceof SandboxedJexlSource }
 
-  override predicate isSource(DataFlow::Node node) { node instanceof SandboxedJexlSource }
-
-  override predicate isSink(DataFlow::Node node) {
+  predicate isSink(DataFlow::Node node) {
     exists(MethodAccess ma, Method m |
       m instanceof CreateJexlScriptMethod or
       m instanceof CreateJexlExpressionMethod or
@@ -121,11 +140,13 @@ private class SandboxedJexlFlowConfig extends DataFlow2::Configuration {
     )
   }
 
-  override predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
+  predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
     createJexlEngineStep(fromNode, toNode)
   }
 }
 
+private module SandboxedJexlFlow = DataFlow::Global<SandboxedJexlFlowConfig>;
+
 /**
  * Defines a data flow source for JEXL engines configured with a sandbox.
  */

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

@@ -7,9 +7,11 @@ import semmle.code.java.frameworks.SpringLdap
 import semmle.code.java.security.JndiInjection
 
 /**
+ * DEPRECATED: Use `JndiInjectionFlow` instead.
+ *
  * A taint-tracking configuration for unvalidated user input that is used in JNDI lookup.
  */
-class JndiInjectionFlowConfig extends TaintTracking::Configuration {
+deprecated class JndiInjectionFlowConfig extends TaintTracking::Configuration {
   JndiInjectionFlowConfig() { this = "JndiInjectionFlowConfig" }
 
   override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
@@ -27,14 +29,34 @@ class JndiInjectionFlowConfig extends TaintTracking::Configuration {
   }
 }
 
+/**
+ * A taint-tracking configuration for unvalidated user input that is used in JNDI lookup.
+ */
+module JndiInjectionFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  predicate isSink(DataFlow::Node sink) { sink instanceof JndiInjectionSink }
+
+  predicate isBarrier(DataFlow::Node node) {
+    node.getType() instanceof PrimitiveType or
+    node.getType() instanceof BoxedType or
+    node instanceof JndiInjectionSanitizer
+  }
+
+  predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+    any(JndiInjectionAdditionalTaintStep c).step(node1, node2)
+  }
+}
+
+/** Tracks flow of unvalidated user input that is used in JNDI lookup */
+module JndiInjectionFlow = TaintTracking::Global<JndiInjectionFlowConfig>;
+
 /**
  * A method that does a JNDI lookup when it receives a `SearchControls` argument with `setReturningObjFlag` = `true`
  */
 private class UnsafeSearchControlsSink extends JndiInjectionSink {
   UnsafeSearchControlsSink() {
-    exists(UnsafeSearchControlsConf conf, MethodAccess ma |
-      conf.hasFlowTo(DataFlow::exprNode(ma.getAnArgument()))
-    |
+    exists(MethodAccess ma | UnsafeSearchControlsFlow::flowToExpr(ma.getAnArgument()) |
       this.asExpr() = ma.getArgument(0)
     )
   }
@@ -44,14 +66,14 @@ private class UnsafeSearchControlsSink extends JndiInjectionSink {
  * Find flows between a `SearchControls` object with `setReturningObjFlag` = `true`
  * and an argument of an `LdapOperations.search` or `DirContext.search` call.
  */
-private class UnsafeSearchControlsConf extends DataFlow2::Configuration {
-  UnsafeSearchControlsConf() { this = "UnsafeSearchControlsConf" }
+private module UnsafeSearchControlsConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof UnsafeSearchControls }
 
-  override predicate isSource(DataFlow::Node source) { source instanceof UnsafeSearchControls }
-
-  override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeSearchControlsArgument }
+  predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeSearchControlsArgument }
 }
 
+private module UnsafeSearchControlsFlow = DataFlow::Global<UnsafeSearchControlsConfig>;
+
 /**
  * An argument of type `SearchControls` of an `LdapOperations.search` or `DirContext.search` call.
  */

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

@@ -6,10 +6,12 @@ import semmle.code.java.dataflow.TaintTracking
 import semmle.code.java.security.MvelInjection
 
 /**
+ * DEPRECATED: Use `MvelInjectionFlow` instead.
+ *
  * A taint-tracking configuration for unsafe user input
  * that is used to construct and evaluate a MVEL expression.
  */
-class MvelInjectionFlowConfig extends TaintTracking::Configuration {
+deprecated class MvelInjectionFlowConfig extends TaintTracking::Configuration {
   MvelInjectionFlowConfig() { this = "MvelInjectionFlowConfig" }
 
   override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
@@ -24,3 +26,22 @@ class MvelInjectionFlowConfig extends TaintTracking::Configuration {
     any(MvelInjectionAdditionalTaintStep c).step(node1, node2)
   }
 }
+
+/**
+ * A taint-tracking configuration for unsafe user input
+ * that is used to construct and evaluate a MVEL expression.
+ */
+module MvelInjectionFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  predicate isSink(DataFlow::Node sink) { sink instanceof MvelEvaluationSink }
+
+  predicate isBarrier(DataFlow::Node sanitizer) { sanitizer instanceof MvelInjectionSanitizer }
+
+  predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+    any(MvelInjectionAdditionalTaintStep c).step(node1, node2)
+  }
+}
+
+/** Tracks flow of unsafe user input that is used to construct and evaluate a MVEL expression. */
+module MvelInjectionFlow = TaintTracking::Global<MvelInjectionFlowConfig>;

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

@@ -5,9 +5,11 @@ import semmle.code.java.dataflow.FlowSources
 import semmle.code.java.security.OgnlInjection
 
 /**
+ * DEPRECATED: Use `OgnlInjectionFlow` instead.
+ *
  * A taint-tracking configuration for unvalidated user input that is used in OGNL EL evaluation.
  */
-class OgnlInjectionFlowConfig extends TaintTracking::Configuration {
+deprecated class OgnlInjectionFlowConfig extends TaintTracking::Configuration {
   OgnlInjectionFlowConfig() { this = "OgnlInjectionFlowConfig" }
 
   override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
@@ -22,3 +24,23 @@ class OgnlInjectionFlowConfig extends TaintTracking::Configuration {
     any(OgnlInjectionAdditionalTaintStep c).step(node1, node2)
   }
 }
+
+/**
+ * A taint-tracking configuration for unvalidated user input that is used in OGNL EL evaluation.
+ */
+module OgnlInjectionFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  predicate isSink(DataFlow::Node sink) { sink instanceof OgnlInjectionSink }
+
+  predicate isBarrier(DataFlow::Node node) {
+    node.getType() instanceof PrimitiveType or node.getType() instanceof BoxedType
+  }
+
+  predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+    any(OgnlInjectionAdditionalTaintStep c).step(node1, node2)
+  }
+}
+
+/** Tracks flow of unvalidated user input that is used in OGNL EL evaluation. */
+module OgnlInjectionFlow = TaintTracking::Global<OgnlInjectionFlowConfig>;

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.
+ */
+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::Global<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::flowToExpr(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::Global<SafeEvaluationContextFlowConfig>;
+
 /**
  * A `ContextSource` that is safe from SpEL injection.
  */