Python: switch dataflow library to new (shared) CFG + SSA

python/ql/consistency-queries/DataFlowConsistency.ql

@@ -9,6 +9,7 @@ private import semmle.python.dataflow.new.internal.DataFlowImplSpecific
 private import semmle.python.dataflow.new.internal.DataFlowDispatch
 private import semmle.python.dataflow.new.internal.TaintTrackingImplSpecific
 private import codeql.dataflow.internal.DataFlowImplConsistency
+private import semmle.python.controlflow.internal.Cfg as Cfg
 
 private module Input implements InputSig<Location, PythonDataFlow> {
   private import Private
@@ -72,7 +73,7 @@ private module Input implements InputSig<Location, PythonDataFlow> {
     // resolve to multiple functions), but we only make _one_ ArgumentNode for each
     // argument in the CallNode, we end up violating this consistency check in those
     // cases. (see `getCallArg` in DataFlowDispatch.qll)
-    exists(DataFlowCall other, CallNode cfgCall | other != call |
+    exists(DataFlowCall other, Cfg::CallNode cfgCall | other != call |
       call.getNode() = cfgCall and
       other.getNode() = cfgCall and
       isArgumentNode(arg, call, _) and
@@ -88,16 +89,16 @@ private module Input implements InputSig<Location, PythonDataFlow> {
       // allow it instead.
       (
         call.getScope() = attr.getScope() and
-        any(CfgNode n | n.asCfgNode() = call.getNode().(CallNode).getFunction()).getALocalSource() =
-          attr
+        any(CfgNode n | n.asCfgNode() = call.getNode().(Cfg::CallNode).getFunction())
+            .getALocalSource() = attr
         or
         not exists(call.getScope().(Function).getDefinition()) and
         call.getScope().getScope+() = attr.getScope()
       ) and
       (
         other.getScope() = attr.getScope() and
-        any(CfgNode n | n.asCfgNode() = other.getNode().(CallNode).getFunction()).getALocalSource() =
-          attr
+        any(CfgNode n | n.asCfgNode() = other.getNode().(Cfg::CallNode).getFunction())
+            .getALocalSource() = attr
         or
         not exists(other.getScope().(Function).getDefinition()) and
         other.getScope().getScope+() = attr.getScope()

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

@@ -6,8 +6,9 @@
  * directed and labeled; they specify how the components represented by nodes relate to each other.
  */
 
-// Importing python under the `py` namespace to avoid importing `CallNode` from `Flow.qll` and thereby having a naming conflict with `API::CallNode`.
+// Importing python under the `PY` namespace to avoid pulling in `CallNode` from `Flow.qll` (via `import python`) and thereby having a naming conflict with `API::CallNode`.
 private import python as PY
+private import semmle.python.controlflow.internal.Cfg as Cfg
 import semmle.python.dataflow.new.DataFlow
 private import semmle.python.internal.CachedStages
 
@@ -282,15 +283,15 @@ module API {
       index = this.getIndex() and
       (
         // subscripting
-        exists(PY::SubscriptNode subscript |
+        exists(Cfg::SubscriptNode subscript |
           subscript.getObject() = this.getAValueReachableFromSource().asCfgNode() and
           subscript.getIndex() = index.asSink().asCfgNode()
         |
           // reading
           subscript = result.asSource().asCfgNode()
           or
           // writing
-          subscript.(PY::DefinitionNode).getValue() = result.asSink().asCfgNode()
+          subscript.(Cfg::DefinitionNode).getValue() = result.asSink().asCfgNode()
         )
         or
         // dictionary literals
@@ -684,7 +685,7 @@ module API {
      * Ignores relative imports, such as `from ..foo.bar import baz`.
      */
     private predicate imports(DataFlow::CfgNode imp, string name) {
-      exists(PY::ImportExprNode iexpr |
+      exists(Cfg::ImportExprNode iexpr |
         imp.getNode() = iexpr and
         not iexpr.getNode().isRelative() and
         name = iexpr.getNode().getImportedModuleName()
@@ -775,7 +776,7 @@ module API {
         // list literals, from `x` to `[x]`
         // TODO: once convenient, this should be done at a higher level than the AST,
         // at least at the CFG layer, to take splitting into account.
-        // Also consider `SequenceNode for generality.
+        // Also consider `Cfg::SequenceNode` for generality.
         exists(PY::List list | list = pred.(DataFlow::ExprNode).getNode().getNode() |
           rhs.(DataFlow::ExprNode).getNode().getNode() = list.getAnElt() and
           lbl = Label::subscript()
@@ -805,7 +806,7 @@ module API {
         subscript = trackUseNode(src).getSubscript(index)
       |
         // from `x` to a definition of `x[...]`
-        rhs.asCfgNode() = subscript.asCfgNode().(PY::DefinitionNode).getValue() and
+        rhs.asCfgNode() = subscript.asCfgNode().(Cfg::DefinitionNode).getValue() and
         lbl = Label::subscript()
         or
         // from `x` to `"key"` in `x["key"]`

python/ql/lib/semmle/python/Concepts.qll

@@ -5,6 +5,7 @@
  */
 
 private import python
+private import semmle.python.controlflow.internal.Cfg as Cfg
 private import semmle.python.dataflow.new.DataFlow
 private import semmle.python.dataflow.new.internal.DataFlowImplSpecific
 private import semmle.python.dataflow.new.RemoteFlowSources
@@ -214,7 +215,7 @@ module Path {
     SafeAccessCheck() { this = DataFlow::BarrierGuard<safeAccessCheck/3>::getABarrierNode() }
   }
 
-  private predicate safeAccessCheck(DataFlow::GuardNode g, ControlFlowNode node, boolean branch) {
+  private predicate safeAccessCheck(DataFlow::GuardNode g, Cfg::ControlFlowNode node, boolean branch) {
     g.(SafeAccessCheck::Range).checks(node, branch)
   }
 
@@ -223,7 +224,7 @@ module Path {
     /** A data-flow node that checks that a path is safe to access in some way, for example by having a controlled prefix. */
     abstract class Range extends DataFlow::GuardNode {
       /** Holds if this guard validates `node` upon evaluating to `branch`. */
-      abstract predicate checks(ControlFlowNode node, boolean branch);
+      abstract predicate checks(Cfg::ControlFlowNode node, boolean branch);
     }
   }
 }

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

@@ -1,11 +1,12 @@
 /** Provides commonly used BarrierGuards. */
 
 private import python
+private import semmle.python.controlflow.internal.Cfg as Cfg
 private import semmle.python.dataflow.new.DataFlow
 
-private predicate constCompare(DataFlow::GuardNode g, ControlFlowNode node, boolean branch) {
-  exists(CompareNode cn | cn = g |
-    exists(ImmutableLiteral const, Cmpop op, ControlFlowNode c |
+private predicate constCompare(DataFlow::GuardNode g, Cfg::ControlFlowNode node, boolean branch) {
+  exists(Cfg::CompareNode cn | cn = g |
+    exists(ImmutableLiteral const, Cmpop op, Cfg::ControlFlowNode c |
       c.getNode() = const and
       (
         op = any(Eq eq) and branch = true
@@ -18,7 +19,7 @@ private predicate constCompare(DataFlow::GuardNode g, ControlFlowNode node, bool
       cn.operands(node, op, c)
     )
     or
-    exists(NameConstant const, Cmpop op, ControlFlowNode c |
+    exists(NameConstant const, Cmpop op, Cfg::ControlFlowNode c |
       c.getNode() = const and
       (
         op = any(Is is_) and branch = true
@@ -31,12 +32,12 @@ private predicate constCompare(DataFlow::GuardNode g, ControlFlowNode node, bool
       cn.operands(node, op, c)
     )
     or
-    exists(IterableNode const_iterable, Cmpop op |
+    exists(Cfg::IterableNode const_iterable, Cmpop op |
       op = any(In in_) and branch = true
       or
       op = any(NotIn ni) and branch = false
     |
-      forall(ControlFlowNode elem | elem = const_iterable.getAnElement() |
+      forall(Cfg::ControlFlowNode elem | elem = const_iterable.getAnElement() |
         elem.getNode() instanceof ImmutableLiteral
       ) and
       cn.operands(node, op, const_iterable)

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

@@ -4,6 +4,7 @@
  */
 
 private import python
+private import semmle.python.controlflow.internal.Cfg as Cfg
 private import semmle.python.dataflow.new.DataFlow
 // Need to import `semmle.python.Frameworks` since frameworks can extend `SensitiveDataSource::Range`
 private import semmle.python.Frameworks
@@ -105,7 +106,7 @@ private module SensitiveDataModeling {
       or
       // to cover functions that we don't have the definition for, and where the
       // reference to the function has not already been marked as being sensitive
-      this.getFunction().asCfgNode().(NameNode).getId() = sensitiveString(classification)
+      this.getFunction().asCfgNode().(Cfg::NameNode).getId() = sensitiveString(classification)
     }
 
     override SensitiveDataClassification getClassification() { result = classification }
@@ -251,12 +252,12 @@ private module SensitiveDataModeling {
     SensitiveDataClassification classification;
 
     SensitiveVariableAssignment() {
-      exists(DefinitionNode def |
-        def.(NameNode).getId() = sensitiveString(classification) and
+      exists(Cfg::DefinitionNode def |
+        def.(Cfg::NameNode).getId() = sensitiveString(classification) and
         (
           this.asCfgNode() = def.getValue()
           or
-          this.asCfgNode() = def.getValue().(ForNode).getSequence()
+          this.asCfgNode() = def.getValue().(Cfg::ForNode).getSequence()
         ) and
         not this.asExpr() instanceof FunctionExpr and
         not this.asExpr() instanceof ClassExpr
@@ -293,7 +294,7 @@ private module SensitiveDataModeling {
     SensitiveDataClassification classification;
 
     SensitiveSubscript() {
-      this.asCfgNode().(SubscriptNode).getIndex() =
+      this.asCfgNode().(Cfg::SubscriptNode).getIndex() =
         sensitiveLookupStringConst(classification).asCfgNode()
     }
 

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

@@ -3,6 +3,7 @@ overlay[local]
 module;
 
 private import python
+private import semmle.python.controlflow.internal.Cfg as Cfg
 import DataFlowUtil
 import DataFlowPublic
 private import DataFlowPrivate
@@ -83,9 +84,9 @@ abstract class AttrWrite extends AttrRef {
  * ```python
  * object.attr = value
  * ```
- * Also gives access to the `value` being written, by extending `DefinitionNode`.
+ * Also gives access to the `value` being written, by extending `Cfg::DefinitionNode`.
  */
-private class AttributeAssignmentNode extends DefinitionNode, AttrNode { }
+private class AttributeAssignmentNode extends Cfg::DefinitionNode, Cfg::AttrNode { }
 
 /** A simple attribute assignment: `object.attr = value`. */
 private class AttributeAssignmentAsAttrWrite extends AttrWrite, CfgNode {
@@ -131,21 +132,21 @@ private class GlobalAttributeAssignmentAsAttrWrite extends AttrWrite, CfgNode {
   override string getAttributeName() { result = node.getName() }
 }
 
-/** Represents `CallNode`s that may refer to calls to built-in functions or classes. */
-private class BuiltInCallNode extends CallNode {
+/** Represents `Cfg::CallNode`s that may refer to calls to built-in functions or classes. */
+private class BuiltInCallNode extends Cfg::CallNode {
   string name;
 
   BuiltInCallNode() {
     // TODO disallow instances where the name of the built-in may refer to an in-scope variable of that name.
-    exists(NameNode id |
+    exists(Cfg::NameNode id |
       name = Builtins::getBuiltinName() and
       this.getFunction() = id and
       id.getId() = name and
       id.isGlobal()
     )
   }
 
-  /** Gets the name of the built-in function that is called at this `CallNode` */
+  /** Gets the name of the built-in function that is called at this `Cfg::CallNode` */
   string getBuiltinName() { result = name }
 }
 
@@ -157,20 +158,20 @@ private class BuiltinAttrCallNode extends BuiltInCallNode {
   BuiltinAttrCallNode() { name in ["setattr", "getattr", "hasattr", "delattr"] }
 
   /** Gets the control flow node for object on which the attribute is accessed. */
-  ControlFlowNode getObject() { result in [this.getArg(0), this.getArgByName("object")] }
+  Cfg::ControlFlowNode getObject() { result in [this.getArg(0), this.getArgByName("object")] }
 
   /**
    * Gets the control flow node for the value that is being written to the attribute.
    * Only relevant for `setattr` calls.
    */
-  ControlFlowNode getValue() {
+  Cfg::ControlFlowNode getValue() {
     // only valid for `setattr`
     name = "setattr" and
     result in [this.getArg(2), this.getArgByName("value")]
   }
 
   /** Gets the control flow node that defines the name of the attribute being accessed. */
-  ControlFlowNode getName() { result in [this.getArg(1), this.getArgByName("name")] }
+  Cfg::ControlFlowNode getName() { result in [this.getArg(1), this.getArgByName("name")] }
 }
 
 /** Represents calls to the built-in `setattr`. */
@@ -205,10 +206,10 @@ private class SetAttrCallAsAttrWrite extends AttrWrite, CfgNode {
  *     attr = value
  *     ...
  * ```
- * Instances of this class correspond to the `NameNode` for `attr`, and also gives access to `value` by
- * virtue of being a `DefinitionNode`.
+ * Instances of this class correspond to the `Cfg::NameNode` for `attr`, and also gives access to `value` by
+ * virtue of being a `Cfg::DefinitionNode`.
  */
-private class ClassAttributeAssignmentNode extends DefinitionNode, NameNode {
+private class ClassAttributeAssignmentNode extends Cfg::DefinitionNode, Cfg::NameNode {
   ClassAttributeAssignmentNode() { this.getScope() = any(ClassExpr c).getInnerScope() }
 }
 
@@ -248,7 +249,7 @@ abstract class AttrRead extends AttrRef, Node, LocalSourceNode {
 
 /** A simple attribute read, e.g. `object.attr` */
 private class AttributeReadAsAttrRead extends AttrRead, CfgNode {
-  override AttrNode node;
+  override Cfg::AttrNode node;
 
   AttributeReadAsAttrRead() { node.isLoad() }
 
@@ -285,7 +286,7 @@ private class GetAttrCallAsAttrRead extends AttrRead, CfgNode {
  * is treated as if it is a read of the attribute `module.attr`, even if `module` is not imported directly.
  */
 private class ModuleAttributeImportAsAttrRead extends AttrRead, CfgNode {
-  override ImportMemberNode node;
+  override Cfg::ImportMemberNode node;
 
   override Node getObject() { result.asCfgNode() = node.getModule(_) }
 

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

@@ -3,6 +3,7 @@ overlay[local]
 module;
 
 private import python
+private import semmle.python.controlflow.internal.Cfg as Cfg
 private import semmle.python.dataflow.new.DataFlow
 private import semmle.python.dataflow.new.internal.ImportStar
 
@@ -67,7 +68,7 @@ module Builtins {
   DataFlow::CfgNode likelyBuiltin(string name) {
     exists(Module m |
       result.getNode() =
-        any(NameNode n |
+        any(Cfg::NameNode n |
           possible_builtin_accessed_in_module(n, name, m) and
           not possible_builtin_defined_in_module(name, m)
         )
@@ -87,7 +88,7 @@ module Builtins {
    * Holds if `n` is an access of a global variable called `name` (which is also the name of a
    * built-in) inside the module `m`.
    */
-  private predicate possible_builtin_accessed_in_module(NameNode n, string name, Module m) {
+  private predicate possible_builtin_accessed_in_module(Cfg::NameNode n, string name, Module m) {
     n.isGlobal() and
     n.isLoad() and
     name = n.getId() and