Merge pull request #8781 from yoff/python-dataflow/flow-summaries-from-scratch

Python dataflow: flow summaries restart

Author: yoff

config/identical-files.json

@@ -73,10 +73,11 @@
     "ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImplConsistency.qll",
     "swift/ql/lib/codeql/swift/dataflow/internal/DataFlowImplConsistency.qll"
   ],
-  "DataFlow Java/C# Flow Summaries": [
+  "DataFlow Java/C#/Ruby/Python/Swift Flow Summaries": [
     "java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll",
     "csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll",
     "ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImpl.qll",
+    "python/ql/lib/semmle/python/dataflow/new/internal/FlowSummaryImpl.qll",
     "swift/ql/lib/codeql/swift/dataflow/internal/FlowSummaryImpl.qll"
   ],
   "SsaReadPosition Java/C#": [
@@ -532,7 +533,7 @@
     "java/ql/lib/semmle/code/java/dataflow/internal/AccessPathSyntax.qll",
     "javascript/ql/lib/semmle/javascript/frameworks/data/internal/AccessPathSyntax.qll",
     "ruby/ql/lib/codeql/ruby/dataflow/internal/AccessPathSyntax.qll",
-    "python/ql/lib/semmle/python/frameworks/data/internal/AccessPathSyntax.qll",
+    "python/ql/lib/semmle/python/dataflow/new/internal/AccessPathSyntax.qll",
     "swift/ql/lib/codeql/swift/dataflow/internal/AccessPathSyntax.qll"
   ],
   "IncompleteUrlSubstringSanitization": [

python/ql/lib/design.md

@@ -0,0 +1,24 @@
+# The Python libraries
+
+The Python libraries are a collection of libraries for analysing Python code.
+Everythng can be imported by importing `python.qll`.
+
+## The analysis layers
+
+The analysis is built up in layers. the stack looks like this:
+
+- AST (coms from the extractor)
+- Control flow graph (CFG) (built by the extractor)
+- SSA
+- Call graph
+- Data flow
+
+## Avoiding non-monotonic recursion
+
+Given the many interactivg layers, it is imprtant to decie which predicates are allowed to be mutually recursive in order to avoid non-monotonic recursion when negation is used to express the predicates.
+As an example, we have defined local source as those whcih do not receive local flow. This means that the local flow relation is not allowed to be recursive with anything depending on local sources.
+
+Some particular reatrictions to keep in mind:
+
+- Typetracking needs to use a local flow step not including summaries
+- Typetracking needs to use a call graph not including summaries

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

@@ -380,6 +380,21 @@ module API {
     not m.matches("%.%")
   }
 
+  /**
+   * Holds if an import of module `m` exists.
+   *
+   * This is determined without referring to `Node`,
+   * allowing this predicate to be used in a negative
+   * context when constructing new nodes.
+   */
+  predicate moduleImportExists(string m) {
+    Impl::isImported(m) and
+    // restrict `moduleImport` so it will never give results for a dotted name. Note
+    // that we cannot move this logic to the `MkModuleImport` construction, since we
+    // need the intermediate API graph nodes for the prefixes in `import foo.bar.baz`.
+    not m.matches("%.%")
+  }
+
   /** Gets a node corresponding to the built-in with the given name, if any. */
   Node builtin(string n) { result = moduleImport("builtins").getMember(n) }
 
@@ -605,14 +620,38 @@ module API {
      *
      * Ignores relative imports, such as `from ..foo.bar import baz`.
      */
-    private predicate imports(DataFlow::Node imp, string name) {
+    private predicate imports(DataFlow::CfgNode imp, string name) {
       exists(PY::ImportExprNode iexpr |
-        imp.asCfgNode() = iexpr and
+        imp.getNode() = iexpr and
         not iexpr.getNode().isRelative() and
         name = iexpr.getNode().getImportedModuleName()
       )
     }
 
+    /**
+     * Holds if the module `name` is imported.
+     *
+     * This is determined syntactically.
+     */
+    cached
+    predicate isImported(string name) {
+      // Ignore the following module name for Python 2, as we alias `__builtin__` to `builtins` elsewhere
+      (name != "__builtin__" or PY::major_version() = 3) and
+      (
+        exists(PY::ImportExpr iexpr |
+          not iexpr.isRelative() and
+          name = iexpr.getImportedModuleName()
+        )
+        or
+        // When we `import foo.bar.baz` we want to create API graph nodes also for the prefixes
+        // `foo` and `foo.bar`:
+        name = any(PY::ImportExpr e | not e.isRelative()).getAnImportedModuleName()
+      )
+      or
+      // The `builtins` module should always be implicitly available
+      name = "builtins"
+    }
+
     private import semmle.python.dataflow.new.internal.Builtins
     private import semmle.python.dataflow.new.internal.ImportStar
 
@@ -631,7 +670,7 @@ module API {
      */
     private TApiNode potential_import_star_base(PY::Scope s) {
       exists(DataFlow::Node n |
-        n.asCfgNode() = ImportStar::potentialImportStarBase(s) and
+        n.(DataFlow::CfgNode).getNode() = ImportStar::potentialImportStarBase(s) and
         use(result, n)
       )
     }
@@ -653,17 +692,17 @@ module API {
         or
         // TODO: I had expected `DataFlow::AttrWrite` to contain the attribute writes from a dict, that's how JS works.
         exists(PY::Dict dict, PY::KeyValuePair item |
-          dict = pred.asExpr() and
+          dict = pred.(DataFlow::ExprNode).getNode().getNode() and
           dict.getItem(_) = item and
           lbl = Label::member(item.getKey().(PY::StrConst).getS()) and
-          rhs.asExpr() = item.getValue()
+          rhs.(DataFlow::ExprNode).getNode().getNode() = item.getValue()
         )
         or
-        exists(PY::CallableExpr fn | fn = pred.asExpr() |
+        exists(PY::CallableExpr fn | fn = pred.(DataFlow::ExprNode).getNode().getNode() |
           not fn.getInnerScope().isAsync() and
           lbl = Label::return() and
           exists(PY::Return ret |
-            rhs.asExpr() = ret.getValue() and
+            rhs.(DataFlow::ExprNode).getNode().getNode() = ret.getValue() and
             ret.getScope() = fn.getInnerScope()
           )
         )
@@ -716,9 +755,9 @@ module API {
           // "benign" and let subclasses edges flow through anyway.
           // see example in https://github.com/django/django/blob/c2250cfb80e27cdf8d098428824da2800a18cadf/tests/auth_tests/test_views.py#L40-L46
           (
-            ref.asExpr() = clsExpr
+            ref.(DataFlow::ExprNode).getNode().getNode() = clsExpr
             or
-            ref.asExpr() = clsExpr.getADecoratorCall()
+            ref.(DataFlow::ExprNode).getNode().getNode() = clsExpr.getADecoratorCall()
           )
         )
         or
@@ -731,26 +770,27 @@ module API {
       )
       or
       exists(DataFlow::Node def, PY::CallableExpr fn |
-        rhs(base, def) and fn = trackDefNode(def).asExpr()
+        rhs(base, def) and fn = trackDefNode(def).(DataFlow::ExprNode).getNode().getNode()
       |
         exists(int i, int offset |
           if exists(PY::Parameter p | p = fn.getInnerScope().getAnArg() and p.isSelf())
           then offset = 1
           else offset = 0
         |
           lbl = Label::parameter(i - offset) and
-          ref.asExpr() = fn.getInnerScope().getArg(i)
+          ref.(DataFlow::ExprNode).getNode().getNode() = fn.getInnerScope().getArg(i)
         )
         or
         exists(string name, PY::Parameter param |
           lbl = Label::keywordParameter(name) and
           param = fn.getInnerScope().getArgByName(name) and
           not param.isSelf() and
-          ref.asExpr() = param
+          ref.(DataFlow::ExprNode).getNode().getNode() = param
         )
         or
         lbl = Label::selfParameter() and
-        ref.asExpr() = any(PY::Parameter p | p = fn.getInnerScope().getAnArg() and p.isSelf())
+        ref.(DataFlow::ExprNode).getNode().getNode() =
+          any(PY::Parameter p | p = fn.getInnerScope().getAnArg() and p.isSelf())
       )
       or
       // Built-ins, treated as members of the module `builtins`
@@ -762,7 +802,7 @@ module API {
         base = potential_import_star_base(s) and
         lbl =
           Label::member(any(string name |
-              ImportStar::namePossiblyDefinedInImportStar(ref.asCfgNode(), name, s)
+              ImportStar::namePossiblyDefinedInImportStar(ref.(DataFlow::CfgNode).getNode(), name, s)
             ))
       )
       or
@@ -854,7 +894,7 @@ module API {
     DataFlow::LocalSourceNode trackUseNode(DataFlow::LocalSourceNode src) {
       Stages::TypeTracking::ref() and
       result = trackUseNode(src, DataFlow::TypeTracker::end()) and
-      not result instanceof DataFlow::ModuleVariableNode
+      result instanceof DataFlow::ExprNode
     }
 
     /**
@@ -1044,7 +1084,7 @@ module API {
     ApiLabel memberFromRef(DataFlow::AttrRef ref) {
       result = member(ref.getAttributeName())
       or
-      not exists(ref.getAttributeName()) and
+      ref.unknownAttribute() and
       result = unknownMember()
     }
 

python/ql/lib/semmle/python/dataflow/new/FlowSummary.qll

@@ -0,0 +1,107 @@
+/** Provides classes and predicates for defining flow summaries. */
+
+private import python
+private import semmle.python.dataflow.new.DataFlow
+private import semmle.python.frameworks.data.ModelsAsData
+private import semmle.python.ApiGraphs
+private import internal.FlowSummaryImpl as Impl
+private import internal.DataFlowUtil
+private import internal.DataFlowPrivate
+
+// import all instances below
+private module Summaries {
+  private import semmle.python.Frameworks
+}
+
+class SummaryComponent = Impl::Public::SummaryComponent;
+
+/** Provides predicates for constructing summary components. */
+module SummaryComponent {
+  private import Impl::Public::SummaryComponent as SC
+
+  predicate parameter = SC::parameter/1;
+
+  predicate argument = SC::argument/1;
+
+  predicate content = SC::content/1;
+
+  /** Gets a summary component that represents a list element. */
+  SummaryComponent listElement() { result = content(any(ListElementContent c)) }
+
+  /** Gets a summary component that represents the return value of a call. */
+  SummaryComponent return() { result = SC::return(any(ReturnKind rk)) }
+}
+
+class SummaryComponentStack = Impl::Public::SummaryComponentStack;
+
+/** Provides predicates for constructing stacks of summary components. */
+module SummaryComponentStack {
+  private import Impl::Public::SummaryComponentStack as SCS
+
+  predicate singleton = SCS::singleton/1;
+
+  predicate push = SCS::push/2;
+
+  predicate argument = SCS::argument/1;
+
+  /** Gets a singleton stack representing the return value of a call. */
+  SummaryComponentStack return() { result = singleton(SummaryComponent::return()) }
+}
+
+/** A callable with a flow summary, identified by a unique string. */
+abstract class SummarizedCallable extends LibraryCallable, Impl::Public::SummarizedCallable {
+  bindingset[this]
+  SummarizedCallable() { any() }
+
+  /**
+   * Same as
+   *
+   * ```ql
+   * propagatesFlow(
+   *   SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
+   * )
+   * ```
+   *
+   * but uses an external (string) representation of the input and output stacks.
+   */
+  pragma[nomagic]
+  predicate propagatesFlowExt(string input, string output, boolean preservesValue) { none() }
+}
+
+class RequiredSummaryComponentStack = Impl::Public::RequiredSummaryComponentStack;
+// // This gives access to getNodeFromPath, which is not constrained to `CallNode`s
+// // as `resolvedSummaryBase` is.
+// private import semmle.python.frameworks.data.internal.ApiGraphModels as AGM
+//
+// private class SummarizedCallableFromModel extends SummarizedCallable {
+//   string package;
+//   string type;
+//   string path;
+//   SummarizedCallableFromModel() {
+//     ModelOutput::relevantSummaryModel(package, type, path, _, _, _) and
+//     this = package + ";" + type + ";" + path
+//   }
+//   override CallCfgNode getACall() {
+//     exists(API::CallNode base |
+//       ModelOutput::resolvedSummaryBase(package, type, path, base) and
+//       result = base.getACall()
+//     )
+//   }
+//   override ArgumentNode getACallback() {
+//     exists(API::Node base |
+//       base = AGM::getNodeFromPath(package, type, path) and
+//       result = base.getAValueReachableFromSource()
+//     )
+//   }
+//   override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
+//     exists(string kind |
+//       ModelOutput::relevantSummaryModel(package, type, path, input, output, kind)
+//     |
+//       kind = "value" and
+//       preservesValue = true
+//       or
+//       kind = "taint" and
+//       preservesValue = false
+//     )
+//   }
+// }

python/ql/lib/semmle/python/frameworks/data/internal/AccessPathSyntax.qll renamed to python/ql/lib/semmle/python/dataflow/new/internal/AccessPathSyntax.qll

@@ -40,7 +40,7 @@ abstract class AttrRef extends Node {
     or
     exists(LocalSourceNode nodeFrom |
       nodeFrom.flowsTo(this.getAttributeNameExpr()) and
-      attrName = nodeFrom.asExpr().(StrConst).getText()
+      attrName = nodeFrom.(CfgNode).getNode().getNode().(StrConst).getText()
     )
   }
 
@@ -50,6 +50,9 @@ abstract class AttrRef extends Node {
    * better results.
    */
   abstract string getAttributeName();
+
+  /** Holds if a name could not be determined for this attribute. */
+  predicate unknownAttribute() { not exists(this.getAttributeName()) }
 }
 
 /**
@@ -175,7 +178,7 @@ private class SetAttrCallAsAttrWrite extends AttrWrite, CfgNode {
   override ExprNode getAttributeNameExpr() { result.asCfgNode() = node.getName() }
 
   override string getAttributeName() {
-    result = this.getAttributeNameExpr().asExpr().(StrConst).getText()
+    result = this.getAttributeNameExpr().(CfgNode).getNode().getNode().(StrConst).getText()
   }
 }
 
@@ -251,7 +254,7 @@ private class GetAttrCallAsAttrRead extends AttrRead, CfgNode {
   override ExprNode getAttributeNameExpr() { result.asCfgNode() = node.getName() }
 
   override string getAttributeName() {
-    result = this.getAttributeNameExpr().asExpr().(StrConst).getText()
+    result = this.getAttributeNameExpr().(CfgNode).getNode().getNode().(StrConst).getText()
   }
 }
 

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

@@ -60,9 +60,9 @@ module Builtins {
    * Currently this is an over-approximation, and may not account for things like overwriting a
    * built-in with a different value.
    */
-  DataFlow::Node likelyBuiltin(string name) {
+  DataFlow::CfgNode likelyBuiltin(string name) {
     exists(Module m |
-      result.asCfgNode() =
+      result.getNode() =
         any(NameNode n |
           possible_builtin_accessed_in_module(n, name, m) and
           not possible_builtin_defined_in_module(name, m)