Merge branch 'main' of https://github.com/github/codeql into python/captured-variables-basic

2026-04-26 09:15:12 +02:00 · 2023-12-20 13:16:42 +01:00
parent 1417c2cdd5 19813c8ba6
commit f8417b0dd8
73 changed files with 9964 additions and 533 deletions
--- a/python/ql/lib/change-notes/2023-12-20-add-scope-entry-definition-nodes.md
+++ b/python/ql/lib/change-notes/2023-12-20-add-scope-entry-definition-nodes.md
@@ -0,0 +1,5 @@
+---
+category: fix
+---
+
+- We would previously confuse all captured variables into a single scope entry node. Now they each get their own node so they can be tracked properly.
--- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll
+++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll
@@ -361,7 +361,10 @@ module LocalFlow {
    //   nodeFrom is `y` on first line
    //   nodeTo is `y` on second line
    exists(EssaDefinition def |
-      nodeFrom.(CfgNode).getNode() = def.(EssaNodeDefinition).getDefiningNode() and
+      nodeFrom.(CfgNode).getNode() = def.(EssaNodeDefinition).getDefiningNode()
+      or
+      nodeFrom.(ScopeEntryDefinitionNode).getDefinition() = def
+    |
      AdjacentUses::firstUse(def, nodeTo.(CfgNode).getNode())
    )
    or
@@ -492,8 +495,7 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
 * or at runtime when callables in the module are called.
 */
 predicate simpleLocalFlowStepForTypetracking(Node nodeFrom, Node nodeTo) {
-  IncludePostUpdateFlow<PhaseDependentFlow<LocalFlow::localFlowStep/2>::step/2>::step(nodeFrom,
-    nodeTo)
+  LocalFlow::localFlowStep(nodeFrom, nodeTo)
 }

 private predicate summaryLocalStep(Node nodeFrom, Node nodeTo) {
--- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll
+++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll
@@ -27,9 +27,12 @@ newtype TNode =
    isExpressionNode(node)
    or
    node.getNode() instanceof Pattern
-    or
-    node = any(ScopeEntryDefinition def | not def.getScope() instanceof Module).getDefiningNode()
  } or
+  /**
+   * A node corresponding to a scope entry definition. That is, the value of a variable
+   * as it enters a scope.
+   */
+  TScopeEntryDefinitionNode(ScopeEntryDefinition def) { not def.getScope() instanceof Module } or
  /**
   * A synthetic node representing the value of an object before a state change.
   *
@@ -265,6 +268,28 @@ class ExprNode extends CfgNode {
 /** Gets a node corresponding to expression `e`. */
 ExprNode exprNode(DataFlowExpr e) { result.getNode().getNode() = e }

+/**
+ * A node corresponding to a scope entry definition. That is, the value of a variable
+ * as it enters a scope.
+ */
+class ScopeEntryDefinitionNode extends Node, TScopeEntryDefinitionNode {
+  ScopeEntryDefinition def;
+
+  ScopeEntryDefinitionNode() { this = TScopeEntryDefinitionNode(def) }
+
+  /** Gets the `ScopeEntryDefinition` associated with this node. */
+  ScopeEntryDefinition getDefinition() { result = def }
+
+  /** Gets the source variable represented by this node. */
+  SsaSourceVariable getVariable() { result = def.getSourceVariable() }
+
+  override Location getLocation() { result = def.getLocation() }
+
+  override Scope getScope() { result = def.getScope() }
+
+  override string toString() { result = "Entry definition for " + this.getVariable().toString() }
+}
+
 /**
 * The value of a parameter at function entry, viewed as a node in a data
 * flow graph.
--- a/python/ql/lib/semmle/python/dataflow/new/internal/LocalSources.qll
+++ b/python/ql/lib/semmle/python/dataflow/new/internal/LocalSources.qll
@@ -71,7 +71,7 @@ class LocalSourceNode extends Node {
    or
    // We include all scope entry definitions, as these act as the local source within the scope they
    // enter.
-    this.asCfgNode() = any(ScopeEntryDefinition def).getDefiningNode()
+    this instanceof ScopeEntryDefinitionNode
    or
    this instanceof ParameterNode
  }
@@ -167,7 +167,7 @@ class LocalSourceNodeNotModuleVariableNode extends LocalSourceNode {
  LocalSourceNodeNotModuleVariableNode() {
    this instanceof ExprNode
    or
-    this.asCfgNode() = any(ScopeEntryDefinition def).getDefiningNode()
+    this instanceof ScopeEntryDefinitionNode
  }
 }

--- a/python/ql/lib/semmle/python/dataflow/new/internal/TypeTrackingImpl.qll
+++ b/python/ql/lib/semmle/python/dataflow/new/internal/TypeTrackingImpl.qll
@@ -251,7 +251,7 @@ module TypeTrackingInput implements Shared::TypeTrackingInput {
      e.getSourceVariable() = var and
      var.hasDefiningNode(def)
    |
-      nodeTo.asCfgNode() = e.getDefiningNode() and
+      nodeTo.(DataFlowPublic::ScopeEntryDefinitionNode).getDefinition() = e and
      nodeFrom.asCfgNode() = def.getValue() and
      var.getScope().getScope*() = nodeFrom.getScope()
    )
--- a/python/ql/test/experimental/dataflow/coverage/localFlow.expected
+++ b/python/ql/test/experimental/dataflow/coverage/localFlow.expected
@@ -1,12 +1,12 @@
-| test.py:41:1:41:33 | Entry node for Function test_tuple_with_local_flow | test.py:42:10:42:18 | ControlFlowNode for NONSOURCE |
-| test.py:41:1:41:33 | Entry node for Function test_tuple_with_local_flow | test.py:42:21:42:26 | ControlFlowNode for SOURCE |
-| test.py:41:1:41:33 | Entry node for Function test_tuple_with_local_flow | test.py:44:5:44:8 | ControlFlowNode for SINK |
+| test.py:41:1:41:33 | Entry definition for SsaSourceVariable NONSOURCE | test.py:42:10:42:18 | ControlFlowNode for NONSOURCE |
+| test.py:41:1:41:33 | Entry definition for SsaSourceVariable SINK | test.py:44:5:44:8 | ControlFlowNode for SINK |
+| test.py:41:1:41:33 | Entry definition for SsaSourceVariable SOURCE | test.py:42:21:42:26 | ControlFlowNode for SOURCE |
 | test.py:42:5:42:5 | ControlFlowNode for x | test.py:43:9:43:9 | ControlFlowNode for x |
 | test.py:42:10:42:26 | ControlFlowNode for Tuple | test.py:42:5:42:5 | ControlFlowNode for x |
 | test.py:43:5:43:5 | ControlFlowNode for y | test.py:44:10:44:10 | ControlFlowNode for y |
 | test.py:43:9:43:12 | ControlFlowNode for Subscript | test.py:43:5:43:5 | ControlFlowNode for y |
-| test.py:208:1:208:53 | Entry node for Function test_nested_comprehension_deep_with_local_flow | test.py:209:25:209:30 | ControlFlowNode for SOURCE |
-| test.py:208:1:208:53 | Entry node for Function test_nested_comprehension_deep_with_local_flow | test.py:210:5:210:8 | ControlFlowNode for SINK |
+| test.py:208:1:208:53 | Entry definition for SsaSourceVariable SINK | test.py:210:5:210:8 | ControlFlowNode for SINK |
+| test.py:208:1:208:53 | Entry definition for SsaSourceVariable SOURCE | test.py:209:25:209:30 | ControlFlowNode for SOURCE |
 | test.py:209:5:209:5 | ControlFlowNode for x | test.py:210:10:210:10 | ControlFlowNode for x |
 | test.py:209:9:209:68 | ControlFlowNode for .0 | test.py:209:9:209:68 | ControlFlowNode for .0 |
 | test.py:209:9:209:68 | ControlFlowNode for ListComp | test.py:209:5:209:5 | ControlFlowNode for x |
--- a/python/ql/test/experimental/dataflow/enclosing-callable/EnclosingCallable.expected
+++ b/python/ql/test/experimental/dataflow/enclosing-callable/EnclosingCallable.expected
@@ -18,7 +18,6 @@
 | generator.py:1:1:1:23 | Function generator_func | generator.py:2:12:2:26 | ControlFlowNode for .0 |
 | generator.py:1:1:1:23 | Function generator_func | generator.py:2:12:2:26 | ControlFlowNode for .0 |
 | generator.py:1:1:1:23 | Function generator_func | generator.py:2:12:2:26 | ControlFlowNode for ListComp |
-| generator.py:1:1:1:23 | Function generator_func | generator.py:2:12:2:26 | Entry node for Function listcomp |
 | generator.py:1:1:1:23 | Function generator_func | generator.py:2:13:2:13 | ControlFlowNode for Yield |
 | generator.py:1:1:1:23 | Function generator_func | generator.py:2:13:2:13 | ControlFlowNode for x |
 | generator.py:1:1:1:23 | Function generator_func | generator.py:2:19:2:19 | ControlFlowNode for x |
--- a/python/ql/test/experimental/dataflow/typetracking/moduleattr.expected
+++ b/python/ql/test/experimental/dataflow/typetracking/moduleattr.expected
@@ -6,6 +6,6 @@ module_attr_tracker
 | import_as_attr.py:1:28:1:35 | ControlFlowNode for attr_ref |
 | import_as_attr.py:3:1:3:1 | ControlFlowNode for x |
 | import_as_attr.py:3:5:3:12 | ControlFlowNode for attr_ref |
-| import_as_attr.py:5:1:5:10 | Entry node for Function fun |
+| import_as_attr.py:5:1:5:10 | Entry definition for SsaSourceVariable attr_ref |
 | import_as_attr.py:6:5:6:5 | ControlFlowNode for y |
 | import_as_attr.py:6:9:6:16 | ControlFlowNode for attr_ref |
--- a/python/ql/test/experimental/dataflow/typetracking/tracked.ql
+++ b/python/ql/test/experimental/dataflow/typetracking/tracked.ql
@@ -26,7 +26,7 @@ module TrackedTest implements TestSig {
      not e.getLocation().getStartLine() = 0 and
      // We do not wish to annotate scope entry definitions,
      // as they do not appear in the source code.
-      not e.asCfgNode() = any(ScopeEntryDefinition def).getDefiningNode() and
+      not e instanceof DataFlow::ScopeEntryDefinitionNode and
      // ...same for `SynthCaptureNode`s
      not e instanceof DataFlow::SynthCaptureNode and
      tag = "tracked" and
--- a/python/ql/test/experimental/meta/debug/InlineTaintTestPaths.ql
+++ b/python/ql/test/experimental/meta/debug/InlineTaintTestPaths.ql
@@ -23,7 +23,7 @@ module Flows = TaintTracking::Global<Config>;
 import Flows::PathGraph

 // int explorationLimit() { result = 5 }
-// module FlowsPartial = Flows::FlowExploration<explorationLimit/0>;
+// module FlowsPartial = Flows::FlowExplorationFwd<explorationLimit/0>;
 // import FlowsPartial::PartialPathGraph
 from Flows::PathNode source, Flows::PathNode sink
 where Flows::flowPath(source, sink)
--- a/python/ql/test/library-tests/ApiGraphs/py3/test_crosstalk.expected
+++ b/python/ql/test/library-tests/ApiGraphs/py3/test_crosstalk.expected
@@ -0,0 +1,2 @@
+| test_crosstalk.py:8:16:8:18 | ControlFlowNode for f() | bar |
+| test_crosstalk.py:13:16:13:18 | ControlFlowNode for g() | baz |
--- a/python/ql/test/library-tests/ApiGraphs/py3/test_crosstalk.py
+++ b/python/ql/test/library-tests/ApiGraphs/py3/test_crosstalk.py
@@ -0,0 +1,13 @@
+
+def outer():
+    from foo import bar, baz
+    
+    def inner_bar():
+        f = bar
+        g = baz
+        return f()
+    
+    def inner_baz():
+        f = bar
+        g = baz
+        return g()
--- a/python/ql/test/library-tests/ApiGraphs/py3/test_crosstalk.ql
+++ b/python/ql/test/library-tests/ApiGraphs/py3/test_crosstalk.ql
@@ -0,0 +1,8 @@
+import python
+import semmle.python.ApiGraphs
+
+from API::CallNode callNode, string member
+where
+  callNode = API::moduleImport("foo").getMember(member).getACall() and
+  callNode.getLocation().getFile().getBaseName() = "test_crosstalk.py"
+select callNode, member