python: add reads of captured variables to

type tracking and the API graph.

- In `TypeTrackerSpecific.qll` we add a jump step
  - to every scope entry definition
  - from the value of any defining `DefinitionNode`
    (In our example, the definition is the class name, `Users`,
     while the assigned value is the class definition, and it is
     the latter which receives flow in this case.)
- In `LocalSources.qll` we allow scope entry definitions as local sources.
  - This feels natural enough, as they are a local source for the value, they represent.
    It is perhaps a bit funne to see an Ssa variable here,
    rather than a control flow node.
 - This is necessary in order for type tracking to see the local flow
    from the scope entry definition.
- In `ApiGraphs.qll` we no longer restrict the result of `trackUseNode`
  to be an `ExprNode`. To keep the positive formulation, we do not
  prohibit module variable nodes. Instead we restrict to the new
  `LocalSourceNodeNotModule` which avoids those cases.

Author: yoff

python/ql/lib/semmle/python/ApiGraphs.qll

@@ -987,7 +987,7 @@ module API {
     DataFlow::LocalSourceNode trackUseNode(DataFlow::LocalSourceNode src) {
       Stages::TypeTracking::ref() and
       result = trackUseNode(src, DataFlow::TypeTracker::end()) and
-      result instanceof DataFlow::ExprNode
+      result instanceof DataFlow::LocalSourceNodeNotModule
     }
 
     /**

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

@@ -51,6 +51,10 @@ class LocalSourceNode extends Node {
     // We explicitly include any read of a global variable, as some of these may have local flow going
     // into them.
     this = any(ModuleVariableNode mvn).getARead()
+    or
+    // We include all scope entry definitions, as these act as the local source within the scope they
+    // enter.
+    this.asVar() instanceof ScopeEntryDefinition
   }
 
   /** Holds if this `LocalSourceNode` can flow to `nodeTo` in one or more local flow steps. */
@@ -133,6 +137,19 @@ class LocalSourceNode extends Node {
   LocalSourceNode backtrack(TypeBackTracker t2, TypeBackTracker t) { t2 = t.step(result, this) }
 }
 
+/**
+ * A LocalSourceNode that is not a ModuleVariableNode
+ * This class provides a positive formulation of that in its charpred.
+ */
+class LocalSourceNodeNotModule extends LocalSourceNode {
+  cached
+  LocalSourceNodeNotModule() {
+    this instanceof ExprNode
+    or
+    this.asVar() instanceof ScopeEntryDefinition
+  }
+}
+
 /**
  * A node that can be used for type tracking or type back-tracking.
  *

python/ql/lib/semmle/python/dataflow/new/internal/TypeTrackerSpecific.qll

@@ -43,7 +43,19 @@ predicate compatibleContents(TypeTrackerContent storeContent, TypeTrackerContent
 
 predicate simpleLocalFlowStep = DataFlowPrivate::simpleLocalFlowStepForTypetracking/2;
 
-predicate jumpStep = DataFlowPrivate::jumpStepSharedWithTypeTracker/2;
+predicate jumpStep(Node nodeFrom, Node nodeTo) {
+  DataFlowPrivate::jumpStepSharedWithTypeTracker(nodeFrom, nodeTo)
+  or
+  capturedJumpStep(nodeFrom, nodeTo)
+}
+
+predicate capturedJumpStep(Node nodeFrom, Node nodeTo) {
+  exists(SsaSourceVariable var, DefinitionNode def | var.hasDefiningNode(def) |
+    nodeTo.asVar().(ScopeEntryDefinition).getSourceVariable() = var and
+    nodeFrom.asCfgNode() = def.getValue() and
+    var.getScope().getScope*() = nodeFrom.getScope()
+  )
+}
 
 /** Holds if there is a level step from `nodeFrom` to `nodeTo`, which may depend on the call graph. */
 predicate levelStepCall(Node nodeFrom, Node nodeTo) { none() }

python/ql/test/library-tests/ApiGraphs/py3/dataflow-consistency.expected

@@ -3,6 +3,7 @@ uniqueCallEnclosingCallable
 | test_captured.py:7:22:7:25 | p() | Call should have one enclosing callable but has 0. |
 | test_captured.py:7:22:7:25 | p() | Call should have one enclosing callable but has 0. |
 | test_captured.py:14:26:14:30 | pp() | Call should have one enclosing callable but has 0. |
+| test_captured.py:14:26:14:30 | pp() | Call should have one enclosing callable but has 0. |
 uniqueType
 uniqueNodeLocation
 missingLocation

python/ql/test/library-tests/ApiGraphs/py3/test.py

@@ -89,7 +89,7 @@ def use_of_builtins():
 def imported_builtins():
     import builtins #$ use=moduleImport("builtins")
     def open(f):
-        return builtins.open(f) #$ MISSING: use=moduleImport("builtins").getMember("open").getReturn()
+        return builtins.open(f) #$ use=moduleImport("builtins").getMember("open").getReturn()
 
 def redefine_print():
     def my_print(x):

python/ql/test/library-tests/ApiGraphs/py3/test_captured.py

@@ -11,4 +11,4 @@ def pp(x):
         return escape(x) #$ use=moduleImport("html").getMember("escape").getReturn()
 
     def pp_list_inner(l):
-        return ", ".join(pp(x) for x in l) #$ MISSING: use=moduleImport("html").getMember("escape").getReturn()
+        return ", ".join(pp(x) for x in l) #$ use=moduleImport("html").getMember("escape").getReturn()

python/ql/test/library-tests/ApiGraphs/py3/test_captured_flask.py

@@ -21,6 +21,6 @@ def users(id):
       if not sid:
           return make_response(jsonify({'Error':'Token check failed: {0}'.format(sid)}))
       try:
-          user = Users.query.filter_by(id=id).first()  #$ MISSING: use=moduleImport("flask_sqlalchemy").getMember("SQLAlchemy").getReturn().getMember("Model").getASubclass().getMember("query").getMember("filter_by")
+          user = Users.query.filter_by(id=id).first()  #$ use=moduleImport("flask_sqlalchemy").getMember("SQLAlchemy").getReturn().getMember("Model").getASubclass().getMember("query").getMember("filter_by")
       except Exception as e:
           return make_response(jsonify({'error':str(e)}),500)

python/ql/test/library-tests/ApiGraphs/py3/test_import_star.py

@@ -32,5 +32,5 @@ def func2():
 def func3():
     var2 = print #$ use=moduleImport("builtins").getMember("print")
     def func4():
-        var2() #$ MISSING: use=moduleImport("builtins").getMember("print").getReturn()
+        var2() #$ use=moduleImport("builtins").getMember("print").getReturn()
     func4()