Merge pull request github#6218 from tausbn/python-add-typetrackingnode

Approved by RasmusWL

Author: codeql-ci

python/.vscode/ql.code-snippets

@@ -106,7 +106,7 @@
         "prefix": "type tracking",
         "body": [
             "/** Gets a reference to ${3:a thing}. */",
-            "private DataFlow::LocalSourceNode ${1:myType}(DataFlow::TypeTracker t) {",
+            "private DataFlow::TypeTrackingNode ${1:myType}(DataFlow::TypeTracker t) {",
             "  t.start() and",
             "  result = ${2:value}",
             "  or",

python/change-notes/2021-07-12-add-typetrackingnode.md

@@ -0,0 +1,3 @@
+lgtm,codescanning
+* The `track` and `backtrack` methods on `LocalSourceNode` have been deprecated. When writing
+  type trackers, the corresponding methods on `TypeTrackingNode` should be used instead.

python/ql/src/Security/CVE-2018-1281/BindToAllInterfaces.ql

@@ -27,7 +27,7 @@ private string vulnerableHostname() {
 }
 
 /** Gets a reference to a hostname that can be used to bind to all interfaces. */
-private DataFlow::LocalSourceNode vulnerableHostnameRef(DataFlow::TypeTracker t, string hostname) {
+private DataFlow::TypeTrackingNode vulnerableHostnameRef(DataFlow::TypeTracker t, string hostname) {
   t.start() and
   exists(StrConst allInterfacesStrConst | hostname = vulnerableHostname() |
     allInterfacesStrConst.getText() = hostname and
@@ -43,7 +43,7 @@ DataFlow::Node vulnerableHostnameRef(string hostname) {
 }
 
 /** Gets a reference to a tuple for which the first element is a hostname that can be used to bind to all interfaces. */
-private DataFlow::LocalSourceNode vulnerableAddressTuple(DataFlow::TypeTracker t, string hostname) {
+private DataFlow::TypeTrackingNode vulnerableAddressTuple(DataFlow::TypeTracker t, string hostname) {
   t.start() and
   result.asExpr() = any(Tuple tup | tup.getElt(0) = vulnerableHostnameRef(hostname).asExpr())
   or

python/ql/src/Security/CWE-215/FlaskDebug.ql

@@ -17,7 +17,7 @@ import semmle.python.ApiGraphs
 import semmle.python.frameworks.Flask
 
 /** Gets a reference to a truthy literal. */
-private DataFlow::LocalSourceNode truthyLiteral(DataFlow::TypeTracker t) {
+private DataFlow::TypeTrackingNode truthyLiteral(DataFlow::TypeTracker t) {
   t.start() and
   result.asExpr().(ImmutableLiteral).booleanValue() = true
   or

python/ql/src/semmle/python/ApiGraphs.qll

@@ -512,7 +512,7 @@ module API {
      *
      * The flow from `src` to that node may be inter-procedural.
      */
-    private DataFlow::LocalSourceNode trackUseNode(
+    private DataFlow::TypeTrackingNode trackUseNode(
       DataFlow::LocalSourceNode src, DataFlow::TypeTracker t
     ) {
       t.start() and
@@ -530,7 +530,6 @@ module API {
     cached
     DataFlow::LocalSourceNode trackUseNode(DataFlow::LocalSourceNode src) {
       result = trackUseNode(src, DataFlow::TypeTracker::end()) and
-      // We exclude module variable nodes, as these do not correspond to real uses.
       not result instanceof DataFlow::ModuleVariableNode
     }
 

python/ql/src/semmle/python/Concepts.qll

@@ -758,7 +758,7 @@ module Cryptography {
     /** Provides classes for modeling new key-pair generation APIs. */
     module KeyGeneration {
       /** Gets a back-reference to the keysize argument `arg` that was used to generate a new key-pair. */
-      private DataFlow::LocalSourceNode keysizeBacktracker(
+      private DataFlow::TypeTrackingNode keysizeBacktracker(
         DataFlow::TypeBackTracker t, DataFlow::Node arg
       ) {
         t.start() and

python/ql/src/semmle/python/dataflow/new/SensitiveDataSources.qll

@@ -55,7 +55,7 @@ private module SensitiveDataModeling {
    * Gets a reference to a function that is considered to be a sensitive source of
    * `classification`.
    */
-  private DataFlow::LocalSourceNode sensitiveFunction(
+  private DataFlow::TypeTrackingNode sensitiveFunction(
     DataFlow::TypeTracker t, SensitiveDataClassification classification
   ) {
     t.start() and
@@ -109,7 +109,7 @@ private module SensitiveDataModeling {
    *
    * Also see `extraStepForCalls`.
    */
-  private DataFlow::LocalSourceNode possibleSensitiveCallable(DataFlow::TypeTracker t) {
+  private DataFlow::TypeTrackingNode possibleSensitiveCallable(DataFlow::TypeTracker t) {
     t.start() and
     result instanceof SensitiveDataSource
     or

python/ql/src/semmle/python/dataflow/new/TypeTracker.qll

@@ -23,7 +23,7 @@ class OptionalAttributeName = Internal::OptionalContentName;
  * It is recommended that all uses of this type are written in the following form,
  * for tracking some type `myType`:
  * ```ql
- * DataFlow::LocalSourceNode myType(DataFlow::TypeTracker t) {
+ * DataFlow::TypeTrackingNode myType(DataFlow::TypeTracker t) {
  *   t.start() and
  *   result = < source of myType >
  *   or

python/ql/src/semmle/python/dataflow/new/internal/LocalSources.qll

@@ -36,8 +36,13 @@ class LocalSourceNode extends Node {
     this instanceof ExprNode and
     not simpleLocalFlowStep(_, this)
     or
-    // Module variable nodes must be local source nodes, otherwise type trackers cannot step through
-    // them.
+    // We include all module variable nodes, as these act as stepping stones between writes and
+    // reads of global variables. Without them, type tracking based on `LocalSourceNode`s would be
+    // unable to track across global variables.
+    //
+    // Once the `track` and `backtrack` methods have been fully deprecated, this disjunct can be
+    // removed, and the entire class can extend `ExprNode`. At that point, `TypeTrackingNode` should
+    // be used for type tracking instead of `LocalSourceNode`.
     this instanceof ModuleVariableNode
     or
     // We explicitly include any read of a global variable, as some of these may have local flow going
@@ -98,21 +103,68 @@ class LocalSourceNode extends Node {
     result = this.getAnAttributeRead(methodName).getACall()
   }
 
+  /**
+   * DEPRECATED. Use `TypeTrackingNode::track` instead.
+   *
+   * Gets a node that this node may flow to using one heap and/or interprocedural step.
+   *
+   * See `TypeTracker` for more details about how to use this.
+   */
+  pragma[inline]
+  deprecated LocalSourceNode track(TypeTracker t2, TypeTracker t) { t = t2.step(this, result) }
+
+  /**
+   * DEPRECATED. Use `TypeTrackingNode::backtrack` instead.
+   *
+   * Gets a node that may flow into this one using one heap and/or interprocedural step.
+   *
+   * See `TypeBackTracker` for more details about how to use this.
+   */
+  pragma[inline]
+  deprecated LocalSourceNode backtrack(TypeBackTracker t2, TypeBackTracker t) {
+    t2 = t.step(result, this)
+  }
+}
+
+/**
+ * A node that can be used for type tracking or type back-tracking.
+ *
+ * All steps made during type tracking should be between instances of this class.
+ */
+class TypeTrackingNode extends Node {
+  TypeTrackingNode() {
+    this instanceof LocalSourceNode
+    or
+    this instanceof ModuleVariableNode
+  }
+
+  /**
+   * Holds if this node can flow to `nodeTo` in one or more local flow steps.
+   *
+   * For `ModuleVariableNode`s, the only "local" step is to the node itself.
+   * For `LocalSourceNode`s, this is the usual notion of local flow.
+   */
+  predicate flowsTo(Node node) {
+    this instanceof ModuleVariableNode and this = node
+    or
+    this.(LocalSourceNode).flowsTo(node)
+  }
+
   /**
    * Gets a node that this node may flow to using one heap and/or interprocedural step.
    *
    * See `TypeTracker` for more details about how to use this.
    */
   pragma[inline]
-  LocalSourceNode track(TypeTracker t2, TypeTracker t) { t = t2.step(this, result) }
+  TypeTrackingNode track(TypeTracker t2, TypeTracker t) { t = t2.step(this, result) }
 
   /**
    * Gets a node that may flow into this one using one heap and/or interprocedural step.
    *
    * See `TypeBackTracker` for more details about how to use this.
    */
   pragma[inline]
-  LocalSourceNode backtrack(TypeBackTracker t2, TypeBackTracker t) { t2 = t.step(result, this) }
+  TypeTrackingNode backtrack(TypeBackTracker t2, TypeBackTracker t) { t2 = t.step(result, this) }
 }
 
 cached

python/ql/src/semmle/python/dataflow/new/internal/TypeTracker.qll

@@ -59,7 +59,7 @@ private module Cached {
    * Steps contained in this predicate should _not_ depend on the call graph.
    */
   cached
-  predicate stepNoCall(LocalSourceNode nodeFrom, LocalSourceNode nodeTo, StepSummary summary) {
+  predicate stepNoCall(TypeTrackingNode nodeFrom, TypeTrackingNode nodeTo, StepSummary summary) {
     exists(Node mid | nodeFrom.flowsTo(mid) and smallstepNoCall(mid, nodeTo, summary))
   }
 
@@ -68,7 +68,7 @@ private module Cached {
    * inter-procedural step from `nodeFrom` to `nodeTo`.
    */
   cached
-  predicate stepCall(LocalSourceNode nodeFrom, LocalSourceNode nodeTo, StepSummary summary) {
+  predicate stepCall(TypeTrackingNode nodeFrom, TypeTrackingNode nodeTo, StepSummary summary) {
     exists(Node mid | nodeFrom.flowsTo(mid) and smallstepCall(mid, nodeTo, summary))
   }
 }
@@ -96,7 +96,7 @@ class StepSummary extends TStepSummary {
 }
 
 pragma[noinline]
-private predicate smallstepNoCall(Node nodeFrom, LocalSourceNode nodeTo, StepSummary summary) {
+private predicate smallstepNoCall(Node nodeFrom, TypeTrackingNode nodeTo, StepSummary summary) {
   jumpStep(nodeFrom, nodeTo) and
   summary = LevelStep()
   or
@@ -109,7 +109,7 @@ private predicate smallstepNoCall(Node nodeFrom, LocalSourceNode nodeTo, StepSum
 }
 
 pragma[noinline]
-private predicate smallstepCall(Node nodeFrom, LocalSourceNode nodeTo, StepSummary summary) {
+private predicate smallstepCall(Node nodeFrom, TypeTrackingNode nodeTo, StepSummary summary) {
   callStep(nodeFrom, nodeTo) and summary = CallStep()
   or
   returnStep(nodeFrom, nodeTo) and
@@ -129,7 +129,7 @@ module StepSummary {
    * call graph.
    */
   pragma[inline]
-  predicate step(LocalSourceNode nodeFrom, LocalSourceNode nodeTo, StepSummary summary) {
+  predicate step(TypeTrackingNode nodeFrom, TypeTrackingNode nodeTo, StepSummary summary) {
     stepNoCall(nodeFrom, nodeTo, summary)
     or
     stepCall(nodeFrom, nodeTo, summary)
@@ -143,7 +143,7 @@ module StepSummary {
    * type-preserving steps.
    */
   pragma[inline]
-  predicate smallstep(Node nodeFrom, LocalSourceNode nodeTo, StepSummary summary) {
+  predicate smallstep(Node nodeFrom, TypeTrackingNode nodeTo, StepSummary summary) {
     smallstepNoCall(nodeFrom, nodeTo, summary)
     or
     smallstepCall(nodeFrom, nodeTo, summary)
@@ -174,7 +174,7 @@ module StepSummary {
    * function. This means we will track the fact that `x.attr` can have the type of `y` into the
    * assignment to `z` inside `bar`, even though this attribute write happens _after_ `bar` is called.
    */
-  predicate localSourceStoreStep(Node nodeFrom, LocalSourceNode nodeTo, string content) {
+  predicate localSourceStoreStep(Node nodeFrom, TypeTrackingNode nodeTo, string content) {
     exists(Node obj | nodeTo.flowsTo(obj) and basicStoreStep(nodeFrom, obj, content))
   }
 }
@@ -192,7 +192,7 @@ private newtype TTypeTracker = MkTypeTracker(Boolean hasCall, OptionalContentNam
  * It is recommended that all uses of this type are written in the following form,
  * for tracking some type `myType`:
  * ```ql
- * DataFlow::LocalSourceNode myType(DataFlow::TypeTracker t) {
+ * DataFlow::TypeTrackingNode myType(DataFlow::TypeTracker t) {
  *   t.start() and
  *   result = < source of myType >
  *   or
@@ -275,7 +275,7 @@ class TypeTracker extends TTypeTracker {
    * heap and/or inter-procedural step from `nodeFrom` to `nodeTo`.
    */
   pragma[inline]
-  TypeTracker step(LocalSourceNode nodeFrom, LocalSourceNode nodeTo) {
+  TypeTracker step(TypeTrackingNode nodeFrom, TypeTrackingNode nodeTo) {
     exists(StepSummary summary |
       StepSummary::step(nodeFrom, pragma[only_bind_out](nodeTo), pragma[only_bind_into](summary)) and
       result = this.append(pragma[only_bind_into](summary))
@@ -342,7 +342,7 @@ private newtype TTypeBackTracker = MkTypeBackTracker(Boolean hasReturn, Optional
  * for back-tracking some callback type `myCallback`:
  *
  * ```ql
- * DataFlow::LocalSourceNode myCallback(DataFlow::TypeBackTracker t) {
+ * DataFlow::TypeTrackingNode myCallback(DataFlow::TypeBackTracker t) {
  *   t.start() and
  *   result = (< some API call >).getArgument(< n >).getALocalSource()
  *   or
@@ -351,7 +351,7 @@ private newtype TTypeBackTracker = MkTypeBackTracker(Boolean hasReturn, Optional
  *   )
  * }
  *
- * DataFlow::LocalSourceNode myCallback() { result = myCallback(DataFlow::TypeBackTracker::end()) }
+ * DataFlow::TypeTrackingNode myCallback() { result = myCallback(DataFlow::TypeBackTracker::end()) }
  * ```
  *
  * Instead of `result = myCallback(t2).backtrack(t2, t)`, you can also use the equivalent
@@ -418,7 +418,7 @@ class TypeBackTracker extends TTypeBackTracker {
    * heap and/or inter-procedural step from `nodeTo` to `nodeFrom`.
    */
   pragma[inline]
-  TypeBackTracker step(LocalSourceNode nodeFrom, LocalSourceNode nodeTo) {
+  TypeBackTracker step(TypeTrackingNode nodeFrom, TypeTrackingNode nodeTo) {
     exists(StepSummary summary |
       StepSummary::step(pragma[only_bind_out](nodeFrom), nodeTo, pragma[only_bind_into](summary)) and
       this = result.prepend(pragma[only_bind_into](summary))
@@ -431,7 +431,7 @@ class TypeBackTracker extends TTypeBackTracker {
    *
    * Unlike `TypeBackTracker::step`, this predicate exposes all edges
    * in the flowgraph, and not just the edges between
-   * `LocalSourceNode`s. It may therefore be less performant.
+   * `TypeTrackingNode`s. It may therefore be less performant.
    *
    * Type tracking predicates using small steps typically take the following form:
    * ```ql