Merge pull request #8748 from smowton/smowton/admin/dependent-dataflow-configs

Java: Avoid higher-numbered dataflow configs that depend on lower-numbered ones

config/identical-files.json

@@ -51,6 +51,7 @@
     "csharp/ql/lib/semmle/code/csharp/dataflow/internal/tainttracking5/TaintTrackingImpl.qll",
     "java/ql/lib/semmle/code/java/dataflow/internal/tainttracking1/TaintTrackingImpl.qll",
     "java/ql/lib/semmle/code/java/dataflow/internal/tainttracking2/TaintTrackingImpl.qll",
+    "java/ql/lib/semmle/code/java/dataflow/internal/tainttracking3/TaintTrackingImpl.qll",
     "python/ql/lib/semmle/python/dataflow/new/internal/tainttracking1/TaintTrackingImpl.qll",
     "python/ql/lib/semmle/python/dataflow/new/internal/tainttracking2/TaintTrackingImpl.qll",
     "python/ql/lib/semmle/python/dataflow/new/internal/tainttracking3/TaintTrackingImpl.qll",

java/ql/lib/semmle/code/java/dataflow/TaintTracking3.qll

@@ -0,0 +1,7 @@
+/**
+ * Provides classes for performing local (intra-procedural) and
+ * global (inter-procedural) taint-tracking analyses.
+ */
+module TaintTracking3 {
+  import semmle.code.java.dataflow.internal.tainttracking3.TaintTrackingImpl
+}

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

@@ -0,0 +1,196 @@
+/**
+ * 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
+  override predicate isSource(DataFlow::Node source) { none() }
+
+  /**
+   * 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() }
+
+  final override predicate isBarrier(DataFlow::Node node) {
+    this.isSanitizer(node) or
+    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() }
+
+  /**
+   * Holds if taint propagation into `node` is prohibited when the flow state is
+   * `state`.
+   */
+  predicate isSanitizerIn(DataFlow::Node node, DataFlow::FlowState state) { none() }
+
+  final override predicate isBarrierIn(DataFlow::Node node, DataFlow::FlowState state) {
+    this.isSanitizerIn(node, state)
+  }
+
+  final override predicate isBarrierIn(DataFlow::Node node) { this.isSanitizerIn(node) }
+
+  /** Holds if taint propagation out of `node` is prohibited. */
+  predicate isSanitizerOut(DataFlow::Node node) { none() }
+
+  final override predicate isBarrierOut(DataFlow::Node node) { this.isSanitizerOut(node) }
+
+  /**
+   * Holds if taint propagation out of `node` is prohibited when the flow state is
+   * `state`.
+   */
+  predicate isSanitizerOut(DataFlow::Node node, DataFlow::FlowState state) { none() }
+
+  final override predicate isBarrierOut(DataFlow::Node node, DataFlow::FlowState state) {
+    this.isSanitizerOut(node, state)
+  }
+
+  /** Holds if taint propagation through nodes guarded by `guard` is prohibited. */
+  predicate isSanitizerGuard(DataFlow::BarrierGuard guard) { none() }
+
+  final override predicate isBarrierGuard(DataFlow::BarrierGuard guard) {
+    this.isSanitizerGuard(guard) or defaultTaintSanitizerGuard(guard)
+  }
+
+  /**
+   * Holds if taint propagation through nodes guarded by `guard` is prohibited
+   * when the flow state is `state`.
+   */
+  predicate isSanitizerGuard(DataFlow::BarrierGuard guard, DataFlow::FlowState state) { none() }
+
+  final override predicate isBarrierGuard(DataFlow::BarrierGuard guard, DataFlow::FlowState state) {
+    this.isSanitizerGuard(guard, state)
+  }
+
+  /**
+   * Holds if taint may propagate from `node1` to `node2` in addition to the normal data-flow and taint steps.
+   */
+  predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { none() }
+
+  final override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+    this.isAdditionalTaintStep(node1, node2) or
+    defaultAdditionalTaintStep(node1, node2)
+  }
+
+  /**
+   * Holds if taint may propagate from `node1` to `node2` in addition to the normal data-flow and taint steps.
+   * 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)
+  }
+
+  /**
+   * 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)
+  }
+}

java/ql/lib/semmle/code/java/dataflow/internal/tainttracking3/TaintTrackingParameter.qll

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

java/ql/lib/semmle/code/java/frameworks/SnakeYaml.qll

@@ -30,7 +30,7 @@ class Yaml extends RefType {
   Yaml() { this.getAnAncestor().hasQualifiedName("org.yaml.snakeyaml", "Yaml") }
 }
 
-private class SafeYamlConstructionFlowConfig extends DataFlow2::Configuration {
+private class SafeYamlConstructionFlowConfig extends DataFlow3::Configuration {
   SafeYamlConstructionFlowConfig() { this = "SnakeYaml::SafeYamlConstructionFlowConfig" }
 
   override predicate isSource(DataFlow::Node src) {
@@ -65,7 +65,7 @@ private class SnakeYamlParse extends MethodAccess {
   }
 }
 
-private class SafeYamlFlowConfig extends DataFlow3::Configuration {
+private class SafeYamlFlowConfig extends DataFlow2::Configuration {
   SafeYamlFlowConfig() { this = "SnakeYaml::SafeYamlFlowConfig" }
 
   override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeYaml }

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

@@ -2,9 +2,9 @@
 
 import java
 import semmle.code.java.dataflow.FlowSources
-import semmle.code.java.dataflow.DataFlow3
+import semmle.code.java.dataflow.DataFlow2
 import semmle.code.java.dataflow.TaintTracking
-import semmle.code.java.dataflow.TaintTracking2
+import semmle.code.java.dataflow.TaintTracking3
 import semmle.code.java.security.AndroidIntentRedirection
 
 /**
@@ -38,7 +38,7 @@ private class OriginalIntentSanitizer extends IntentRedirectionSanitizer {
  * Data flow configuration used to discard incoming Intents
  * flowing directly to sinks that start Android components.
  */
-private class SameIntentBeingRelaunchedConfiguration extends DataFlow3::Configuration {
+private class SameIntentBeingRelaunchedConfiguration extends DataFlow2::Configuration {
   SameIntentBeingRelaunchedConfiguration() { this = "SameIntentBeingRelaunchedConfiguration" }
 
   override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
@@ -74,7 +74,7 @@ private class IntentWithTaintedComponent extends DataFlow::Node {
 /**
  * A taint tracking configuration for tainted data flowing to an `Intent`'s component.
  */
-private class TaintedIntentComponentConf extends TaintTracking2::Configuration {
+private class TaintedIntentComponentConf extends TaintTracking3::Configuration {
   TaintedIntentComponentConf() { this = "TaintedIntentComponentConf" }
 
   override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }

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

@@ -3,7 +3,6 @@
 import java
 import semmle.code.java.frameworks.JAXB
 import semmle.code.java.dataflow.DataFlow
-import semmle.code.java.dataflow.DataFlow2
 import semmle.code.java.security.CleartextStorageQuery
 import semmle.code.java.security.CleartextStoragePropertiesQuery
 
@@ -74,7 +73,7 @@ private Expr getInstanceInput(DataFlow::Node instance, RefType t) {
   )
 }
 
-private class ClassStoreFlowConfig extends DataFlow2::Configuration {
+private class ClassStoreFlowConfig extends DataFlow::Configuration {
   ClassStoreFlowConfig() { this = "ClassStoreFlowConfig" }
 
   override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof ClassStore }

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

@@ -3,6 +3,7 @@
 import java
 private import semmle.code.java.dataflow.DataFlow4
 private import semmle.code.java.dataflow.TaintTracking
+private import semmle.code.java.dataflow.TaintTracking2
 private import semmle.code.java.security.SensitiveActions
 
 /** A sink representing persistent storage that saves data in clear text. */
@@ -39,7 +40,7 @@ abstract class Storable extends Call {
   abstract Expr getAStore();
 }
 
-private class SensitiveSourceFlowConfig extends TaintTracking::Configuration {
+private class SensitiveSourceFlowConfig extends TaintTracking2::Configuration {
   SensitiveSourceFlowConfig() { this = "SensitiveSourceFlowConfig" }
 
   override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SensitiveExpr }