Merge pull request #8401 from jketema/taint-flow

Extend taint tracking interface with flow states

java/ql/lib/change-notes/2022-03-14-taint-interface-cleanup.md

@@ -0,0 +1,4 @@
+---
+category: breaking
+---
+* The flow state variants of `isBarrier` and `isAdditionalFlowStep` are no longer exposed in the taint tracking library. The `isSanitizer` and `isAdditionalTaintStep` predicates should be used instead.

java/ql/lib/semmle/code/java/dataflow/internal/tainttracking1/TaintTrackingImpl.qll

@@ -64,13 +64,30 @@ abstract class Configuration extends DataFlow::Configuration {
   override predicate isSource(DataFlow::Node source) { none() }
 
   /**
-   * Holds if `sink` is a relevant taint sink.
+   * Holds if `source` is a relevant taint source with the given initial
+   * `state`.
+   *
+   * The smaller this predicate is, the faster `hasFlow()` will converge.
+   */
+  // overridden to provide taint-tracking specific qldoc
+  override predicate isSource(DataFlow::Node source, DataFlow::FlowState state) { none() }
+
+  /**
+   * Holds if `sink` is a relevant taint sink
    *
    * The smaller this predicate is, the faster `hasFlow()` will converge.
    */
   // overridden to provide taint-tracking specific qldoc
   override predicate isSink(DataFlow::Node sink) { none() }
 
+  /**
+   * Holds if `sink` is a relevant taint sink accepting `state`.
+   *
+   * The smaller this predicate is, the faster `hasFlow()` will converge.
+   */
+  // overridden to provide taint-tracking specific qldoc
+  override predicate isSink(DataFlow::Node sink, DataFlow::FlowState state) { none() }
+
   /** Holds if the node `node` is a taint sanitizer. */
   predicate isSanitizer(DataFlow::Node node) { none() }
 
@@ -79,6 +96,16 @@ abstract class Configuration extends DataFlow::Configuration {
     defaultTaintSanitizer(node)
   }
 
+  /**
+   * Holds if the node `node` is a taint sanitizer when the flow state is
+   * `state`.
+   */
+  predicate isSanitizer(DataFlow::Node node, DataFlow::FlowState state) { none() }
+
+  final override predicate isBarrier(DataFlow::Node node, DataFlow::FlowState state) {
+    this.isSanitizer(node, state)
+  }
+
   /** Holds if taint propagation into `node` is prohibited. */
   predicate isSanitizerIn(DataFlow::Node node) { none() }
 
@@ -107,6 +134,25 @@ abstract class Configuration extends DataFlow::Configuration {
     defaultAdditionalTaintStep(node1, node2)
   }
 
+  /**
+   * Holds if the additional taint propagation step from `node1` to `node2`
+   * must be taken into account in the analysis. This step is only applicable
+   * in `state1` and updates the flow state to `state2`.
+   */
+  predicate isAdditionalTaintStep(
+    DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
+    DataFlow::FlowState state2
+  ) {
+    none()
+  }
+
+  final override predicate isAdditionalFlowStep(
+    DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
+    DataFlow::FlowState state2
+  ) {
+    this.isAdditionalTaintStep(node1, state1, node2, state2)
+  }
+
   override predicate allowImplicitRead(DataFlow::Node node, DataFlow::Content c) {
     (this.isSink(node) or this.isAdditionalTaintStep(node, _)) and
     defaultImplicitTaintRead(node, c)

java/ql/lib/semmle/code/java/dataflow/internal/tainttracking2/TaintTrackingImpl.qll

@@ -64,13 +64,30 @@ abstract class Configuration extends DataFlow::Configuration {
   override predicate isSource(DataFlow::Node source) { none() }
 
   /**
-   * Holds if `sink` is a relevant taint sink.
+   * Holds if `source` is a relevant taint source with the given initial
+   * `state`.
+   *
+   * The smaller this predicate is, the faster `hasFlow()` will converge.
+   */
+  // overridden to provide taint-tracking specific qldoc
+  override predicate isSource(DataFlow::Node source, DataFlow::FlowState state) { none() }
+
+  /**
+   * Holds if `sink` is a relevant taint sink
    *
    * The smaller this predicate is, the faster `hasFlow()` will converge.
    */
   // overridden to provide taint-tracking specific qldoc
   override predicate isSink(DataFlow::Node sink) { none() }
 
+  /**
+   * Holds if `sink` is a relevant taint sink accepting `state`.
+   *
+   * The smaller this predicate is, the faster `hasFlow()` will converge.
+   */
+  // overridden to provide taint-tracking specific qldoc
+  override predicate isSink(DataFlow::Node sink, DataFlow::FlowState state) { none() }
+
   /** Holds if the node `node` is a taint sanitizer. */
   predicate isSanitizer(DataFlow::Node node) { none() }
 
@@ -79,6 +96,16 @@ abstract class Configuration extends DataFlow::Configuration {
     defaultTaintSanitizer(node)
   }
 
+  /**
+   * Holds if the node `node` is a taint sanitizer when the flow state is
+   * `state`.
+   */
+  predicate isSanitizer(DataFlow::Node node, DataFlow::FlowState state) { none() }
+
+  final override predicate isBarrier(DataFlow::Node node, DataFlow::FlowState state) {
+    this.isSanitizer(node, state)
+  }
+
   /** Holds if taint propagation into `node` is prohibited. */
   predicate isSanitizerIn(DataFlow::Node node) { none() }
 
@@ -107,6 +134,25 @@ abstract class Configuration extends DataFlow::Configuration {
     defaultAdditionalTaintStep(node1, node2)
   }
 
+  /**
+   * Holds if the additional taint propagation step from `node1` to `node2`
+   * must be taken into account in the analysis. This step is only applicable
+   * in `state1` and updates the flow state to `state2`.
+   */
+  predicate isAdditionalTaintStep(
+    DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
+    DataFlow::FlowState state2
+  ) {
+    none()
+  }
+
+  final override predicate isAdditionalFlowStep(
+    DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
+    DataFlow::FlowState state2
+  ) {
+    this.isAdditionalTaintStep(node1, state1, node2, state2)
+  }
+
   override predicate allowImplicitRead(DataFlow::Node node, DataFlow::Content c) {
     (this.isSink(node) or this.isAdditionalTaintStep(node, _)) and
     defaultImplicitTaintRead(node, c)

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

@@ -29,7 +29,7 @@ class ImplicitPendingIntentStartConf extends TaintTracking::Configuration {
     any(ImplicitPendingIntentAdditionalTaintStep c).step(node1, node2)
   }
 
-  override predicate isAdditionalFlowStep(
+  override predicate isAdditionalTaintStep(
     DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
     DataFlow::FlowState state2
   ) {

java/ql/src/utils/model-generator/CaptureSummaryModels.ql

@@ -61,7 +61,7 @@ class ThroughFlowConfig extends TaintTracking::Configuration {
     (state instanceof TaintRead or state instanceof TaintStore)
   }
 
-  override predicate isAdditionalFlowStep(
+  override predicate isAdditionalTaintStep(
     DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
     DataFlow::FlowState state2
   ) {