Merge pull request #1790 from jbj/tainttracking-cross-language

C++/C#/Java: Shared TaintTrackingImpl.qll

Author: hvitved

config/identical-files.json

@@ -25,18 +25,14 @@
     "cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImplCommon.qll",
     "csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImplCommon.qll"
   ],
-  "Taint tracking C/C++": [
+  "TaintTracking::Configuration Java/C++/C#": [
     "cpp/ql/src/semmle/code/cpp/dataflow/internal/tainttracking1/TaintTrackingImpl.qll",
-    "cpp/ql/src/semmle/code/cpp/dataflow/internal/tainttracking2/TaintTrackingImpl.qll"
-  ],
-  "Taint tracking C#": [
+    "cpp/ql/src/semmle/code/cpp/dataflow/internal/tainttracking2/TaintTrackingImpl.qll",
     "csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking1/TaintTrackingImpl.qll",
     "csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking2/TaintTrackingImpl.qll",
     "csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking3/TaintTrackingImpl.qll",
     "csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking4/TaintTrackingImpl.qll",
-    "csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking5/TaintTrackingImpl.qll"
-  ],
-  "Taint tracking Java": [
+    "csharp/ql/src/semmle/code/csharp/dataflow/internal/tainttracking5/TaintTrackingImpl.qll",
     "java/ql/src/semmle/code/java/dataflow/internal/tainttracking1/TaintTrackingImpl.qll",
     "java/ql/src/semmle/code/java/dataflow/internal/tainttracking2/TaintTrackingImpl.qll"
   ],

cpp/ql/src/semmle/code/cpp/dataflow/internal/TaintTrackingUtil.qll

@@ -18,10 +18,31 @@ private module DataFlow {
  * Holds if taint propagates from `nodeFrom` to `nodeTo` in exactly one local
  * (intra-procedural) step.
  */
-predicate localTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
-  // Taint can flow into using ordinary data flow.
-  DataFlow::localFlowStep(nodeFrom, nodeTo)
-  or
+predicate localTaintStep(DataFlow::Node src, DataFlow::Node sink) {
+  DataFlow::localFlowStep(src, sink) or
+  localAdditionalTaintStep(src, sink)
+}
+
+/**
+ * Holds if the additional step from `src` to `sink` should be included in all
+ * global taint flow configurations.
+ */
+predicate defaultAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) {
+  localAdditionalTaintStep(src, sink)
+}
+
+/**
+ * Holds if `node` should be a barrier in all global taint flow configurations
+ * but not in local taint.
+ */
+predicate defaultTaintBarrier(DataFlow::Node node) { none() }
+
+/**
+ * Holds if taint can flow in one local step from `nodeFrom` to `nodeTo` excluding
+ * local data flow steps. That is, `nodeFrom` and `nodeTo` are likely to represent
+ * different objects.
+ */
+predicate localAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
   // Taint can flow through expressions that alter the value but preserve
   // more than one bit of it _or_ expressions that follow data through
   // pointer indirections.

cpp/ql/src/semmle/code/cpp/dataflow/internal/tainttracking1/TaintTrackingImpl.qll

@@ -1,12 +1,3 @@
-/**
- * Provides classes for performing local (intra-procedural) and
- * global (inter-procedural) taint-tracking analyses.
- *
- * We define _taint propagation_ informally to mean that a substantial part of
- * the information from the source is preserved at the sink. For example, taint
- * propagates from `x` to `x + 100`, but it does not propagate from `x` to `x >
- * 100` since we consider a single bit of information to be too little.
- */
 import TaintTrackingParameter::Public
 private import TaintTrackingParameter::Private
 
@@ -18,7 +9,7 @@ private import TaintTrackingParameter::Private
  *
  * 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
+ * necessarily preserve values but are still relevant from a taint tracking
  * perspective. (For example, string concatenation, where one of the operands
  * is tainted.)
  *
@@ -30,7 +21,9 @@ private import TaintTrackingParameter::Private
  *   MyAnalysisConfiguration() { this = "MyAnalysisConfiguration" }
  *   // Override `isSource` and `isSink`.
  *   // Optionally override `isSanitizer`.
- *   // Optionally override `isSanitizerEdge`.
+ *   // Optionally override `isSanitizerIn`.
+ *   // Optionally override `isSanitizerOut`.
+ *   // Optionally override `isSanitizerGuard`.
  *   // Optionally override `isAdditionalTaintStep`.
  * }
  * ```
@@ -42,57 +35,78 @@ private import TaintTrackingParameter::Private
  * exists(MyAnalysisConfiguration cfg | cfg.hasFlow(source, sink))
  * ```
  *
- * Multiple configurations can coexist, but it is unsupported to depend on a
- * `TaintTracking::Configuration` or a `DataFlow::Configuration` in the
+ * 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 `DataFlow{2,3,4}::Configuration`.
+ * 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 taint source. */
+  /**
+   * 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 taint sink. */
+  /**
+   * 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 taint should not flow into `node`.
-   */
+  /** Holds if the node `node` is a taint sanitizer. */
   predicate isSanitizer(DataFlow::Node node) { none() }
 
-  /** Holds if data flow from `node1` to `node2` is prohibited. */
-  predicate isSanitizerEdge(DataFlow::Node node1, DataFlow::Node node2) {
-    none()
+  final override predicate isBarrier(DataFlow::Node node) {
+    isSanitizer(node) or
+    defaultTaintBarrier(node)
+  }
+
+  /** DEPRECATED: override `isSanitizerIn` and `isSanitizerOut` instead. */
+  deprecated predicate isSanitizerEdge(DataFlow::Node node1, DataFlow::Node node2) { none() }
+
+  deprecated final override predicate isBarrierEdge(DataFlow::Node node1, DataFlow::Node node2) {
+    isSanitizerEdge(node1, node2)
   }
 
+  /** Holds if data flow into `node` is prohibited. */
+  predicate isSanitizerIn(DataFlow::Node node) { none() }
+
+  final override predicate isBarrierIn(DataFlow::Node node) { isSanitizerIn(node) }
+
+  /** Holds if data flow out of `node` is prohibited. */
+  predicate isSanitizerOut(DataFlow::Node node) { none() }
+
+  final override predicate isBarrierOut(DataFlow::Node node) { isSanitizerOut(node) }
+
+  /** Holds if data flow 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 `source` to `target` must be taken into account in the analysis.
-   * This step will only be followed if `target` is not in the `isSanitizer`
-   * predicate.
+   * Holds if the additional taint propagation step from `node1` to `node2`
+   * must be taken into account in the analysis.
    */
-  predicate isAdditionalTaintStep(DataFlow::Node source,
-                                  DataFlow::Node target)
-  { none() }
-
-  final override
-  predicate isBarrier(DataFlow::Node node) { isSanitizer(node) }
+  predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { none() }
 
-  /** DEPRECATED: use `isSanitizerEdge` instead. */
-  override deprecated
-  predicate isBarrierEdge(DataFlow::Node node1, DataFlow::Node node2) {
-    this.isSanitizerEdge(node1, node2)
+  final override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+    isAdditionalTaintStep(node1, node2) or
+    defaultAdditionalTaintStep(node1, node2)
   }
 
-  final override
-  predicate isAdditionalFlowStep(DataFlow::Node source, DataFlow::Node target) {
-    this.isAdditionalTaintStep(source, target)
-    or
-    localTaintStep(source, target)
+  /**
+   * 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/dataflow/internal/tainttracking2/TaintTrackingImpl.qll

@@ -1,12 +1,3 @@
-/**
- * Provides classes for performing local (intra-procedural) and
- * global (inter-procedural) taint-tracking analyses.
- *
- * We define _taint propagation_ informally to mean that a substantial part of
- * the information from the source is preserved at the sink. For example, taint
- * propagates from `x` to `x + 100`, but it does not propagate from `x` to `x >
- * 100` since we consider a single bit of information to be too little.
- */
 import TaintTrackingParameter::Public
 private import TaintTrackingParameter::Private
 
@@ -18,7 +9,7 @@ private import TaintTrackingParameter::Private
  *
  * 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
+ * necessarily preserve values but are still relevant from a taint tracking
  * perspective. (For example, string concatenation, where one of the operands
  * is tainted.)
  *
@@ -30,7 +21,9 @@ private import TaintTrackingParameter::Private
  *   MyAnalysisConfiguration() { this = "MyAnalysisConfiguration" }
  *   // Override `isSource` and `isSink`.
  *   // Optionally override `isSanitizer`.
- *   // Optionally override `isSanitizerEdge`.
+ *   // Optionally override `isSanitizerIn`.
+ *   // Optionally override `isSanitizerOut`.
+ *   // Optionally override `isSanitizerGuard`.
  *   // Optionally override `isAdditionalTaintStep`.
  * }
  * ```
@@ -42,57 +35,78 @@ private import TaintTrackingParameter::Private
  * exists(MyAnalysisConfiguration cfg | cfg.hasFlow(source, sink))
  * ```
  *
- * Multiple configurations can coexist, but it is unsupported to depend on a
- * `TaintTracking::Configuration` or a `DataFlow::Configuration` in the
+ * 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 `DataFlow{2,3,4}::Configuration`.
+ * 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 taint source. */
+  /**
+   * 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 taint sink. */
+  /**
+   * 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 taint should not flow into `node`.
-   */
+  /** Holds if the node `node` is a taint sanitizer. */
   predicate isSanitizer(DataFlow::Node node) { none() }
 
-  /** Holds if data flow from `node1` to `node2` is prohibited. */
-  predicate isSanitizerEdge(DataFlow::Node node1, DataFlow::Node node2) {
-    none()
+  final override predicate isBarrier(DataFlow::Node node) {
+    isSanitizer(node) or
+    defaultTaintBarrier(node)
+  }
+
+  /** DEPRECATED: override `isSanitizerIn` and `isSanitizerOut` instead. */
+  deprecated predicate isSanitizerEdge(DataFlow::Node node1, DataFlow::Node node2) { none() }
+
+  deprecated final override predicate isBarrierEdge(DataFlow::Node node1, DataFlow::Node node2) {
+    isSanitizerEdge(node1, node2)
   }
 
+  /** Holds if data flow into `node` is prohibited. */
+  predicate isSanitizerIn(DataFlow::Node node) { none() }
+
+  final override predicate isBarrierIn(DataFlow::Node node) { isSanitizerIn(node) }
+
+  /** Holds if data flow out of `node` is prohibited. */
+  predicate isSanitizerOut(DataFlow::Node node) { none() }
+
+  final override predicate isBarrierOut(DataFlow::Node node) { isSanitizerOut(node) }
+
+  /** Holds if data flow 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 `source` to `target` must be taken into account in the analysis.
-   * This step will only be followed if `target` is not in the `isSanitizer`
-   * predicate.
+   * Holds if the additional taint propagation step from `node1` to `node2`
+   * must be taken into account in the analysis.
    */
-  predicate isAdditionalTaintStep(DataFlow::Node source,
-                                  DataFlow::Node target)
-  { none() }
-
-  final override
-  predicate isBarrier(DataFlow::Node node) { isSanitizer(node) }
+  predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { none() }
 
-  /** DEPRECATED: use `isSanitizerEdge` instead. */
-  override deprecated
-  predicate isBarrierEdge(DataFlow::Node node1, DataFlow::Node node2) {
-    this.isSanitizerEdge(node1, node2)
+  final override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+    isAdditionalTaintStep(node1, node2) or
+    defaultAdditionalTaintStep(node1, node2)
   }
 
-  final override
-  predicate isAdditionalFlowStep(DataFlow::Node source, DataFlow::Node target) {
-    this.isAdditionalTaintStep(source, target)
-    or
-    localTaintStep(source, target)
+  /**
+   * 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)
   }
 }

csharp/ql/src/semmle/code/csharp/dataflow/internal/TaintTrackingPrivate.qll

@@ -10,6 +10,22 @@ private import semmle.code.csharp.frameworks.JsonNET
 private import cil
 private import dotnet
 
+/**
+ * Holds if `node` should be a barrier in all global taint flow configurations
+ * but not in local taint.
+ */
+predicate defaultTaintBarrier(DataFlow::Node node) { none() }
+
+/**
+ * Holds if the additional step from `src` to `sink` should be included in all
+ * global taint flow configurations.
+ */
+predicate defaultAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
+  localAdditionalTaintStep(pred, succ)
+  or
+  succ = pred.(DataFlow::NonLocalJumpNode).getAJumpSuccessor(false)
+}
+
 private CIL::DataFlowNode asCilDataFlowNode(DataFlow::Node node) {
   result = node.asParameter() or
   result = node.asExpr()