Merge pull request #5727 from tausbn/python-use-localsource-in-stepsummary

Python: Use `LocalSourceNode` in `StepSummary::step`

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

@@ -32,21 +32,7 @@ private DataFlow::LocalSourceNode vulnerableHostnameRef(DataFlow::TypeTracker t,
     result.asExpr() = allInterfacesStrConst
   )
   or
-  // Due to bad performance when using normal setup with `vulnerableHostnameRef(t2, hostname).track(t2, t)`
-  // we have inlined that code and forced a join
-  exists(DataFlow::TypeTracker t2 |
-    exists(DataFlow::StepSummary summary |
-      vulnerableHostnameRef_first_join(t2, hostname, result, summary) and
-      t = t2.append(summary)
-    )
-  )
-}
-
-pragma[nomagic]
-private predicate vulnerableHostnameRef_first_join(
-  DataFlow::TypeTracker t2, string hostname, DataFlow::Node res, DataFlow::StepSummary summary
-) {
-  DataFlow::StepSummary::step(vulnerableHostnameRef(t2, hostname), res, summary)
+  exists(DataFlow::TypeTracker t2 | result = vulnerableHostnameRef(t2, hostname).track(t2, t))
 }
 
 /** Gets a reference to a hostname that can be used to bind to all interfaces. */
@@ -59,21 +45,7 @@ private DataFlow::LocalSourceNode vulnerableAddressTuple(DataFlow::TypeTracker t
   t.start() and
   result.asExpr() = any(Tuple tup | tup.getElt(0) = vulnerableHostnameRef(hostname).asExpr())
   or
-  // Due to bad performance when using normal setup with `vulnerableAddressTuple(t2, hostname).track(t2, t)`
-  // we have inlined that code and forced a join
-  exists(DataFlow::TypeTracker t2 |
-    exists(DataFlow::StepSummary summary |
-      vulnerableAddressTuple_first_join(t2, hostname, result, summary) and
-      t = t2.append(summary)
-    )
-  )
-}
-
-pragma[nomagic]
-private predicate vulnerableAddressTuple_first_join(
-  DataFlow::TypeTracker t2, string hostname, DataFlow::Node res, DataFlow::StepSummary summary
-) {
-  DataFlow::StepSummary::step(vulnerableAddressTuple(t2, hostname), res, summary)
+  exists(DataFlow::TypeTracker t2 | result = vulnerableAddressTuple(t2, hostname).track(t2, t))
 }
 
 /** Gets a reference to a tuple for which the first element is a hostname that can be used to bind to all interfaces. */

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

@@ -422,9 +422,9 @@ module API {
     }
 
     /**
-     * Gets a data-flow node to which `nd`, which is a use of an API-graph node, flows.
+     * Gets a data-flow node to which `src`, which is a use of an API-graph node, flows.
      *
-     * The flow from `nd` to that node may be inter-procedural.
+     * The flow from `src` to that node may be inter-procedural.
      */
     private DataFlow::LocalSourceNode trackUseNode(
       DataFlow::LocalSourceNode src, DataFlow::TypeTracker t
@@ -433,23 +433,19 @@ module API {
       use(_, src) and
       result = src
       or
-      // Due to bad performance when using `trackUseNode(t2, attr_name).track(t2, t)`
-      // we have inlined that code and forced a join
-      exists(DataFlow::StepSummary summary |
-        t = trackUseNode_first_join(src, result, summary).append(summary)
-      )
-    }
-
-    pragma[nomagic]
-    private DataFlow::TypeTracker trackUseNode_first_join(
-      DataFlow::LocalSourceNode src, DataFlow::LocalSourceNode res, DataFlow::StepSummary summary
-    ) {
-      DataFlow::StepSummary::step(trackUseNode(src, result), res, summary)
+      exists(DataFlow::TypeTracker t2 | result = trackUseNode(src, t2).track(t2, t))
     }
 
+    /**
+     * Gets a data-flow node to which `src`, which is a use of an API-graph node, flows.
+     *
+     * The flow from `src` to that node may be inter-procedural.
+     */
     cached
     DataFlow::LocalSourceNode trackUseNode(DataFlow::LocalSourceNode src) {
-      result = trackUseNode(src, DataFlow::TypeTracker::end())
+      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

@@ -570,21 +570,7 @@ module Cryptography {
         arg = any(KeyGeneration::Range r).getKeySizeArg() and
         result = arg.getALocalSource()
         or
-        // Due to bad performance when using normal setup with we have inlined that code and forced a join
-        exists(DataFlow::TypeBackTracker t2 |
-          exists(DataFlow::StepSummary summary |
-            keysizeBacktracker_first_join(t2, arg, result, summary) and
-            t = t2.prepend(summary)
-          )
-        )
-      }
-
-      pragma[nomagic]
-      private predicate keysizeBacktracker_first_join(
-        DataFlow::TypeBackTracker t2, DataFlow::Node arg, DataFlow::Node res,
-        DataFlow::StepSummary summary
-      ) {
-        DataFlow::StepSummary::step(res, keysizeBacktracker(t2, arg), summary)
+        exists(DataFlow::TypeBackTracker t2 | result = keysizeBacktracker(t2, arg).backtrack(t2, t))
       }
 
       /** Gets a back-reference to the keysize argument `arg` that was used to generate a new key-pair. */

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

@@ -51,8 +51,8 @@ module StepSummary {
    * heap and/or inter-procedural step from `nodeFrom` to `nodeTo`.
    */
   cached
-  predicate step(LocalSourceNode nodeFrom, Node nodeTo, StepSummary summary) {
-    exists(Node mid | typePreservingStep*(nodeFrom, mid) and smallstep(mid, nodeTo, summary))
+  predicate step(LocalSourceNode nodeFrom, LocalSourceNode nodeTo, StepSummary summary) {
+    exists(Node mid | nodeFrom.flowsTo(mid) and smallstep(mid, nodeTo, summary))
   }
 
   /**
@@ -63,7 +63,7 @@ module StepSummary {
    * type-preserving steps.
    */
   predicate smallstep(Node nodeFrom, Node nodeTo, StepSummary summary) {
-    typePreservingStep(nodeFrom, nodeTo) and
+    jumpStep(nodeFrom, nodeTo) and
     summary = LevelStep()
     or
     callStep(nodeFrom, nodeTo) and summary = CallStep()
@@ -80,12 +80,6 @@ module StepSummary {
   }
 }
 
-/** Holds if it's reasonable to expect the data flow step from `nodeFrom` to `nodeTo` to preserve types. */
-private predicate typePreservingStep(Node nodeFrom, Node nodeTo) {
-  simpleLocalFlowStep(nodeFrom, nodeTo) or
-  jumpStep(nodeFrom, nodeTo)
-}
-
 /**
  * Gets a callable for the call where `nodeFrom` is used as the `i`'th argument.
  *
@@ -274,10 +268,10 @@ class TypeTracker extends TTypeTracker {
    * heap and/or inter-procedural step from `nodeFrom` to `nodeTo`.
    */
   pragma[inline]
-  TypeTracker step(LocalSourceNode nodeFrom, Node nodeTo) {
+  TypeTracker step(LocalSourceNode nodeFrom, LocalSourceNode nodeTo) {
     exists(StepSummary summary |
-      StepSummary::step(nodeFrom, nodeTo, summary) and
-      result = this.append(summary)
+      StepSummary::step(nodeFrom, pragma[only_bind_out](nodeTo), pragma[only_bind_into](summary)) and
+      result = this.append(pragma[only_bind_into](summary))
     )
   }
 
@@ -312,7 +306,7 @@ class TypeTracker extends TTypeTracker {
       result = this.append(summary)
     )
     or
-    typePreservingStep(nodeFrom, nodeTo) and
+    simpleLocalFlowStep(nodeFrom, nodeTo) and
     result = this
   }
 }
@@ -417,8 +411,8 @@ class TypeBackTracker extends TTypeBackTracker {
   pragma[inline]
   TypeBackTracker step(LocalSourceNode nodeFrom, LocalSourceNode nodeTo) {
     exists(StepSummary summary |
-      StepSummary::step(nodeFrom, nodeTo, summary) and
-      this = result.prepend(summary)
+      StepSummary::step(pragma[only_bind_out](nodeFrom), nodeTo, pragma[only_bind_into](summary)) and
+      this = result.prepend(pragma[only_bind_into](summary))
     )
   }
 
@@ -453,7 +447,7 @@ class TypeBackTracker extends TTypeBackTracker {
       this = result.prepend(summary)
     )
     or
-    typePreservingStep(nodeFrom, nodeTo) and
+    simpleLocalFlowStep(nodeFrom, nodeTo) and
     this = result
   }
 }

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

@@ -119,22 +119,6 @@ class Node extends TNode {
   /** Gets the expression corresponding to this node, if any. */
   Expr asExpr() { none() }
 
-  /**
-   * 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]
-  Node 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) }
-
   /**
    * Gets a local source node from which data may flow to this node in zero or more local data-flow steps.
    */

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

@@ -10,13 +10,6 @@ import python
 import DataFlowPublic
 private import DataFlowPrivate
 
-private predicate comes_from_cfgnode(Node node) {
-  exists(CfgNode first, Node second |
-    simpleLocalFlowStep(first, second) and
-    simpleLocalFlowStep*(second, node)
-  )
-}
-
 /**
  * A data flow node that is a source of local flow. This includes things like
  * - Expressions
@@ -40,8 +33,7 @@ private predicate comes_from_cfgnode(Node node) {
 class LocalSourceNode extends Node {
   cached
   LocalSourceNode() {
-    not comes_from_cfgnode(this) and
-    not this instanceof ModuleVariableNode and
+    not simpleLocalFlowStep(_, this) and
     // Currently, we create synthetic post-update nodes for
     // - arguments to calls that may modify said argument
     // - direct reads a writes of object attributes
@@ -85,6 +77,22 @@ class LocalSourceNode extends Node {
    * Gets a call to this node.
    */
   CallCfgNode getACall() { Cached::call(this, result) }
+
+  /**
+   * 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) }
+
+  /**
+   * 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) }
 }
 
 cached

python/ql/src/semmle/python/frameworks/Cryptography.qll

@@ -83,23 +83,11 @@ private module CryptographyModel {
       result.(DataFlow::CallCfgNode).getFunction() = curveClassWithKeySize(keySize) and
       origin = result
       or
-      // Due to bad performance when using normal setup with we have inlined that code and forced a join
       exists(DataFlow::TypeTracker t2 |
-        exists(DataFlow::StepSummary summary |
-          curveClassInstanceWithKeySize_first_join(t2, keySize, origin, result, summary) and
-          t = t2.append(summary)
-        )
+        result = curveClassInstanceWithKeySize(t2, keySize, origin).track(t2, t)
       )
     }
 
-    pragma[nomagic]
-    private predicate curveClassInstanceWithKeySize_first_join(
-      DataFlow::TypeTracker t2, int keySize, DataFlow::Node origin, DataFlow::Node res,
-      DataFlow::StepSummary summary
-    ) {
-      DataFlow::StepSummary::step(curveClassInstanceWithKeySize(t2, keySize, origin), res, summary)
-    }
-
     /** Gets a reference to a predefined curve class instance with a specific key size (in bits), as well as the origin of the class. */
     DataFlow::Node curveClassInstanceWithKeySize(int keySize, DataFlow::Node origin) {
       curveClassInstanceWithKeySize(DataFlow::TypeTracker::end(), keySize, origin).flowsTo(result)

python/ql/src/semmle/python/frameworks/Django.qll

@@ -1303,7 +1303,7 @@ private module PrivateDjango {
         }
 
         /** Gets a reference to the `django.http.response.HttpResponse.write` function. */
-        private DataFlow::Node write(
+        private DataFlow::LocalSourceNode write(
           django::http::response::HttpResponse::InstanceSource instance, DataFlow::TypeTracker t
         ) {
           t.startInAttr("write") and
@@ -1315,7 +1315,7 @@ private module PrivateDjango {
 
         /** Gets a reference to the `django.http.response.HttpResponse.write` function. */
         DataFlow::Node write(django::http::response::HttpResponse::InstanceSource instance) {
-          result = write(instance, DataFlow::TypeTracker::end())
+          write(instance, DataFlow::TypeTracker::end()).flowsTo(result)
         }
 
         /**

python/ql/src/semmle/python/frameworks/PEP249.qll

@@ -54,7 +54,7 @@ module Connection {
   }
 
   /** Gets a reference to an instance of `db.Connection`. */
-  private DataFlow::Node instance(DataFlow::TypeTracker t) {
+  private DataFlow::LocalSourceNode instance(DataFlow::TypeTracker t) {
     t.start() and
     result instanceof InstanceSource
     or
@@ -62,7 +62,7 @@ module Connection {
   }
 
   /** Gets a reference to an instance of `db.Connection`. */
-  DataFlow::Node instance() { result = instance(DataFlow::TypeTracker::end()) }
+  DataFlow::Node instance() { instance(DataFlow::TypeTracker::end()).flowsTo(result) }
 }
 
 /**
@@ -71,7 +71,7 @@ module Connection {
  */
 module cursor {
   /** Gets a reference to the `cursor` method on a connection. */
-  private DataFlow::Node methodRef(DataFlow::TypeTracker t) {
+  private DataFlow::LocalSourceNode methodRef(DataFlow::TypeTracker t) {
     t.startInAttr("cursor") and
     result = Connection::instance()
     or
@@ -79,10 +79,10 @@ module cursor {
   }
 
   /** Gets a reference to the `cursor` method on a connection. */
-  DataFlow::Node methodRef() { result = methodRef(DataFlow::TypeTracker::end()) }
+  DataFlow::Node methodRef() { methodRef(DataFlow::TypeTracker::end()).flowsTo(result) }
 
   /** Gets a reference to a result of calling the `cursor` method on a connection. */
-  private DataFlow::Node methodResult(DataFlow::TypeTracker t) {
+  private DataFlow::LocalSourceNode methodResult(DataFlow::TypeTracker t) {
     t.start() and
     result.asCfgNode().(CallNode).getFunction() = methodRef().asCfgNode()
     or
@@ -90,7 +90,7 @@ module cursor {
   }
 
   /** Gets a reference to a result of calling the `cursor` method on a connection. */
-  DataFlow::Node methodResult() { result = methodResult(DataFlow::TypeTracker::end()) }
+  DataFlow::Node methodResult() { methodResult(DataFlow::TypeTracker::end()).flowsTo(result) }
 }
 
 /**
@@ -101,7 +101,7 @@ module cursor {
  *
  * See https://www.python.org/dev/peps/pep-0249/#id15.
  */
-private DataFlow::Node execute(DataFlow::TypeTracker t) {
+private DataFlow::LocalSourceNode execute(DataFlow::TypeTracker t) {
   t.startInAttr("execute") and
   result in [cursor::methodResult(), Connection::instance()]
   or
@@ -116,7 +116,7 @@ private DataFlow::Node execute(DataFlow::TypeTracker t) {
  *
  * See https://www.python.org/dev/peps/pep-0249/#id15.
  */
-DataFlow::Node execute() { result = execute(DataFlow::TypeTracker::end()) }
+DataFlow::Node execute() { execute(DataFlow::TypeTracker::end()).flowsTo(result) }
 
 /** A call to the `execute` method on a cursor (or on a connection). */
 private class ExecuteCall extends SqlExecution::Range, DataFlow::CallCfgNode {

python/ql/src/semmle/python/regex.qll

@@ -82,21 +82,7 @@ private DataFlow::LocalSourceNode re_flag_tracker(string flag_name, DataFlow::Ty
     result.asCfgNode() = binop
   )
   or
-  // Due to bad performance when using normal setup with `re_flag_tracker(t2, attr_name).track(t2, t)`
-  // we have inlined that code and forced a join
-  exists(DataFlow::TypeTracker t2 |
-    exists(DataFlow::StepSummary summary |
-      re_flag_tracker_first_join(t2, flag_name, result, summary) and
-      t = t2.append(summary)
-    )
-  )
-}
-
-pragma[nomagic]
-private predicate re_flag_tracker_first_join(
-  DataFlow::TypeTracker t2, string flag_name, DataFlow::Node res, DataFlow::StepSummary summary
-) {
-  DataFlow::StepSummary::step(re_flag_tracker(flag_name, t2), res, summary)
+  exists(DataFlow::TypeTracker t2 | result = re_flag_tracker(flag_name, t2).track(t2, t))
 }
 
 /**