C++: AdjustedConfiguration should not extend the same dataflow configuration as FromGlobalVarTaintTrackingCfg as this causes multiple configurations to be in scope for dataflow.

Author: MathiasVP

cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll

@@ -2,14 +2,15 @@ import cpp
 import semmle.code.cpp.security.Security
 private import semmle.code.cpp.ir.dataflow.DataFlow
 private import semmle.code.cpp.ir.dataflow.internal.DataFlowUtil
-private import semmle.code.cpp.ir.dataflow.DataFlow2
+private import semmle.code.cpp.ir.dataflow.DataFlow3
 private import semmle.code.cpp.ir.IR
 private import semmle.code.cpp.ir.dataflow.internal.DataFlowDispatch as Dispatch
 private import semmle.code.cpp.controlflow.IRGuards
 private import semmle.code.cpp.models.interfaces.Taint
 private import semmle.code.cpp.models.interfaces.DataFlow
 private import semmle.code.cpp.ir.dataflow.TaintTracking
 private import semmle.code.cpp.ir.dataflow.TaintTracking2
+private import semmle.code.cpp.ir.dataflow.TaintTracking3
 private import semmle.code.cpp.ir.dataflow.internal.ModelUtil
 
 /**
@@ -380,7 +381,7 @@ module TaintedWithPath {
     string toString() { result = "TaintTrackingConfiguration" }
   }
 
-  private class AdjustedConfiguration extends TaintTracking2::Configuration {
+  private class AdjustedConfiguration extends TaintTracking3::Configuration {
     AdjustedConfiguration() { this = "AdjustedConfiguration" }
 
     override predicate isSource(DataFlow::Node source) {
@@ -438,11 +439,11 @@ module TaintedWithPath {
    */
 
   private newtype TPathNode =
-    TWrapPathNode(DataFlow2::PathNode n) or
+    TWrapPathNode(DataFlow3::PathNode n) or
     // There's a single newtype constructor for both sources and sinks since
     // that makes it easiest to deal with the case where source = sink.
     TEndpointPathNode(Element e) {
-      exists(AdjustedConfiguration cfg, DataFlow2::Node sourceNode, DataFlow2::Node sinkNode |
+      exists(AdjustedConfiguration cfg, DataFlow3::Node sourceNode, DataFlow3::Node sinkNode |
         cfg.hasFlow(sourceNode, sinkNode)
       |
         sourceNode = getNodeForExpr(e) and
@@ -473,7 +474,7 @@ module TaintedWithPath {
   }
 
   private class WrapPathNode extends PathNode, TWrapPathNode {
-    DataFlow2::PathNode inner() { this = TWrapPathNode(result) }
+    DataFlow3::PathNode inner() { this = TWrapPathNode(result) }
 
     override string toString() { result = this.inner().toString() }
 
@@ -510,25 +511,25 @@ module TaintedWithPath {
 
   /** Holds if `(a,b)` is an edge in the graph of data flow path explanations. */
   query predicate edges(PathNode a, PathNode b) {
-    DataFlow2::PathGraph::edges(a.(WrapPathNode).inner(), b.(WrapPathNode).inner())
+    DataFlow3::PathGraph::edges(a.(WrapPathNode).inner(), b.(WrapPathNode).inner())
     or
     // To avoid showing trivial-looking steps, we _replace_ the last node instead
     // of adding an edge out of it.
     exists(WrapPathNode sinkNode |
-      DataFlow2::PathGraph::edges(a.(WrapPathNode).inner(), sinkNode.inner()) and
+      DataFlow3::PathGraph::edges(a.(WrapPathNode).inner(), sinkNode.inner()) and
       b.(FinalPathNode).inner() = adjustedSink(sinkNode.inner().getNode())
     )
     or
     // Same for the first node
     exists(WrapPathNode sourceNode |
-      DataFlow2::PathGraph::edges(sourceNode.inner(), b.(WrapPathNode).inner()) and
+      DataFlow3::PathGraph::edges(sourceNode.inner(), b.(WrapPathNode).inner()) and
       sourceNode.inner().getNode() = getNodeForExpr(a.(InitialPathNode).inner())
     )
     or
     // Finally, handle the case where the path goes directly from a source to a
     // sink, meaning that they both need to be translated.
     exists(WrapPathNode sinkNode, WrapPathNode sourceNode |
-      DataFlow2::PathGraph::edges(sourceNode.inner(), sinkNode.inner()) and
+      DataFlow3::PathGraph::edges(sourceNode.inner(), sinkNode.inner()) and
       sourceNode.inner().getNode() = getNodeForExpr(a.(InitialPathNode).inner()) and
       b.(FinalPathNode).inner() = adjustedSink(sinkNode.inner().getNode())
     )
@@ -550,7 +551,7 @@ module TaintedWithPath {
    * the computation.
    */
   predicate taintedWithPath(Expr source, Element tainted, PathNode sourceNode, PathNode sinkNode) {
-    exists(AdjustedConfiguration cfg, DataFlow2::Node flowSource, DataFlow2::Node flowSink |
+    exists(AdjustedConfiguration cfg, DataFlow3::Node flowSource, DataFlow3::Node flowSink |
       source = sourceNode.(InitialPathNode).inner() and
       flowSource = getNodeForExpr(source) and
       cfg.hasFlow(flowSource, flowSink) and

cpp/ql/src/semmle/code/cpp/ir/dataflow/TaintTracking3.qll

@@ -0,0 +1,15 @@
+/**
+ * Provides a `TaintTracking3` module, which is a copy of the `TaintTracking`
+ * module. Use this class when data-flow configurations or taint-tracking
+ * configurations must depend on each other. Two classes extending
+ * `DataFlow::Configuration` should never depend on each other, but one of them
+ * should instead depend on a `DataFlow2::Configuration`, a
+ * `DataFlow3::Configuration`, or a `DataFlow4::Configuration`. The
+ * `TaintTracking::Configuration` class extends `DataFlow::Configuration`, and
+ * `TaintTracking2::Configuration` extends `DataFlow2::Configuration`.
+ *
+ * See `semmle.code.cpp.ir.dataflow.TaintTracking` for the full documentation.
+ */
+module TaintTracking3 {
+  import semmle.code.cpp.ir.dataflow.internal.tainttracking3.TaintTrackingImpl
+}

cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/tainttracking3/TaintTrackingImpl.qll

@@ -0,0 +1,115 @@
+/**
+ * Provides an implementation of global (interprocedural) taint tracking.
+ * This file re-exports the local (intraprocedural) taint-tracking analysis
+ * from `TaintTrackingParameter::Public` and adds a global analysis, mainly
+ * exposed through the `Configuration` class. For some languages, this file
+ * exists in several identical copies, allowing queries to use multiple
+ * `Configuration` classes that depend on each other without introducing
+ * mutual recursion among those configurations.
+ */
+
+import TaintTrackingParameter::Public
+private import TaintTrackingParameter::Private
+
+/**
+ * A configuration of interprocedural taint tracking analysis. This defines
+ * sources, sinks, and any other configurable aspect of the analysis. Each
+ * use of the taint tracking library must define its own unique extension of
+ * this abstract class.
+ *
+ * A taint-tracking configuration is a special data flow configuration
+ * (`DataFlow::Configuration`) that allows for flow through nodes that do not
+ * necessarily preserve values but are still relevant from a taint tracking
+ * perspective. (For example, string concatenation, where one of the operands
+ * is tainted.)
+ *
+ * To create a configuration, extend this class with a subclass whose
+ * characteristic predicate is a unique singleton string. For example, write
+ *
+ * ```ql
+ * class MyAnalysisConfiguration extends TaintTracking::Configuration {
+ *   MyAnalysisConfiguration() { this = "MyAnalysisConfiguration" }
+ *   // Override `isSource` and `isSink`.
+ *   // Optionally override `isSanitizer`.
+ *   // Optionally override `isSanitizerIn`.
+ *   // Optionally override `isSanitizerOut`.
+ *   // Optionally override `isSanitizerGuard`.
+ *   // Optionally override `isAdditionalTaintStep`.
+ * }
+ * ```
+ *
+ * Then, to query whether there is flow between some `source` and `sink`,
+ * write
+ *
+ * ```ql
+ * exists(MyAnalysisConfiguration cfg | cfg.hasFlow(source, sink))
+ * ```
+ *
+ * Multiple configurations can coexist, but it is unsupported to depend on
+ * another `TaintTracking::Configuration` or a `DataFlow::Configuration` in the
+ * overridden predicates that define sources, sinks, or additional steps.
+ * Instead, the dependency should go to a `TaintTracking2::Configuration` or a
+ * `DataFlow2::Configuration`, `DataFlow3::Configuration`, etc.
+ */
+abstract class Configuration extends DataFlow::Configuration {
+  bindingset[this]
+  Configuration() { any() }
+
+  /**
+   * Holds if `source` is a relevant taint source.
+   *
+   * The smaller this predicate is, the faster `hasFlow()` will converge.
+   */
+  // overridden to provide taint-tracking specific qldoc
+  abstract override predicate isSource(DataFlow::Node source);
+
+  /**
+   * 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
+  abstract override predicate isSink(DataFlow::Node sink);
+
+  /** Holds if the node `node` is a taint sanitizer. */
+  predicate isSanitizer(DataFlow::Node node) { none() }
+
+  final override predicate isBarrier(DataFlow::Node node) {
+    isSanitizer(node) or
+    defaultTaintSanitizer(node)
+  }
+
+  /** Holds if taint propagation into `node` is prohibited. */
+  predicate isSanitizerIn(DataFlow::Node node) { none() }
+
+  final override predicate isBarrierIn(DataFlow::Node node) { isSanitizerIn(node) }
+
+  /** Holds if taint propagation out of `node` is prohibited. */
+  predicate isSanitizerOut(DataFlow::Node node) { none() }
+
+  final override predicate isBarrierOut(DataFlow::Node node) { isSanitizerOut(node) }
+
+  /** Holds if taint propagation through nodes guarded by `guard` is prohibited. */
+  predicate isSanitizerGuard(DataFlow::BarrierGuard guard) { none() }
+
+  final override predicate isBarrierGuard(DataFlow::BarrierGuard guard) { isSanitizerGuard(guard) }
+
+  /**
+   * Holds if the additional taint propagation step from `node1` to `node2`
+   * must be taken into account in the analysis.
+   */
+  predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { none() }
+
+  final override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+    isAdditionalTaintStep(node1, node2) or
+    defaultAdditionalTaintStep(node1, node2)
+  }
+
+  /**
+   * Holds if taint may flow from `source` to `sink` for this configuration.
+   */
+  // overridden to provide taint-tracking specific qldoc
+  override predicate hasFlow(DataFlow::Node source, DataFlow::Node sink) {
+    super.hasFlow(source, sink)
+  }
+}

cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/tainttracking3/TaintTrackingParameter.qll

@@ -0,0 +1,5 @@
+import semmle.code.cpp.ir.dataflow.internal.TaintTrackingUtil as Public
+
+module Private {
+  import semmle.code.cpp.ir.dataflow.DataFlow3::DataFlow3 as DataFlow
+}