From 9137f04bd330712c306ad819641e9328b466da72 Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Wed, 19 May 2021 11:57:49 +0200
Subject: [PATCH 01/14] Python: Add getPostUpdateNode to DataFlow::Node

as discussed in https://github.com/github/codeql/pull/5864#discussion_r634675940
---
 .../semmle/python/dataflow/new/internal/DataFlowPrivate.qll    | 2 +-
 .../src/semmle/python/dataflow/new/internal/DataFlowPublic.qll | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/python/ql/src/semmle/python/dataflow/new/internal/DataFlowPrivate.qll b/python/ql/src/semmle/python/dataflow/new/internal/DataFlowPrivate.qll
index 074289b00fd4..d831bfbfbd2b 100644
--- a/python/ql/src/semmle/python/dataflow/new/internal/DataFlowPrivate.qll
+++ b/python/ql/src/semmle/python/dataflow/new/internal/DataFlowPrivate.qll
@@ -19,7 +19,7 @@ module syntheticPreUpdateNode {
     SyntheticPreUpdateNode() { this = TSyntheticPreUpdateNode(post) }
 
     /** Gets the node for which this is a synthetic pre-update node. */
-    Node getPostUpdateNode() { result = post }
+    override NeedsSyntheticPreUpdateNode getPostUpdateNode() { result = post }
 
     override string toString() { result = "[pre " + post.label() + "] " + post.toString() }
 
diff --git a/python/ql/src/semmle/python/dataflow/new/internal/DataFlowPublic.qll b/python/ql/src/semmle/python/dataflow/new/internal/DataFlowPublic.qll
index 03912ca1b867..c95e3da5973a 100644
--- a/python/ql/src/semmle/python/dataflow/new/internal/DataFlowPublic.qll
+++ b/python/ql/src/semmle/python/dataflow/new/internal/DataFlowPublic.qll
@@ -123,6 +123,9 @@ class Node extends TNode {
    * Gets a local source node from which data may flow to this node in zero or more local data-flow steps.
    */
   LocalSourceNode getALocalSource() { result.flowsTo(this) }
+
+  /** Gets the `PostUpdateNode` for this node, if any. */
+  PostUpdateNode getPostUpdateNode() { result.getPreUpdateNode() = this }
 }
 
 /** A data-flow node corresponding to an SSA variable. */

From b02fb908072f8c29f6b2cce7617c5e6a661b796f Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Wed, 19 May 2021 12:11:49 +0200
Subject: [PATCH 02/14] Python: Add getObject(string attrName) to AttrRef

Now that I got started adding small things that are nice, I've been
missing this one (that is available on an `AttrNode`).
---
 .../semmle/python/dataflow/new/internal/Attributes.qll   | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/python/ql/src/semmle/python/dataflow/new/internal/Attributes.qll b/python/ql/src/semmle/python/dataflow/new/internal/Attributes.qll
index 6270e35aa9f3..19a5943df6a6 100644
--- a/python/ql/src/semmle/python/dataflow/new/internal/Attributes.qll
+++ b/python/ql/src/semmle/python/dataflow/new/internal/Attributes.qll
@@ -17,6 +17,15 @@ abstract class AttrRef extends Node {
    */
   abstract Node getObject();
 
+  /**
+   * Gets the data flow node corresponding to the object whose attribute named
+   * `attrName` is being read or written.
+   */
+  Node getObject(string attrName) {
+    result = this.getObject() and
+    attrName = this.getAttributeName()
+  }
+
   /**
    * Gets the expression node that defines the attribute being accessed, if any. This is
    * usually an identifier or literal.

From 3f5602c048595c1324b5210ec6ae11f7bbd6815e Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Wed, 19 May 2021 12:13:04 +0200
Subject: [PATCH 03/14] Python: Refactoring of TaintTrackingPrivate

To use all the good new stuff :tada:
---
 .../new/internal/TaintTrackingPrivate.qll     | 23 +++++++++----------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll b/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll
index a6e169243db1..37ed456b0cf3 100644
--- a/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll
+++ b/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll
@@ -151,36 +151,35 @@ predicate containerStep(DataFlow::CfgNode nodeFrom, DataFlow::Node nodeTo) {
   storeStep(nodeFrom, _, nodeTo)
   or
   // constructor call
-  exists(CallNode call | call = nodeTo.asCfgNode() |
-    call.getFunction().(NameNode).getId() in [
+  exists(DataFlow::CallCfgNode call | call = nodeTo |
+    call.getFunction().asCfgNode().(NameNode).getId() in [
         "list", "set", "frozenset", "dict", "defaultdict", "tuple"
       ] and
-    call.getArg(0) = nodeFrom.getNode()
+    call.getArg(0) = nodeFrom
   )
   or
   // functions operating on collections
-  exists(CallNode call | call = nodeTo.asCfgNode() |
-    call.getFunction().(NameNode).getId() in ["sorted", "reversed", "iter", "next"] and
-    call.getArg(0) = nodeFrom.getNode()
+  exists(DataFlow::CallCfgNode call | call = nodeTo |
+    call.getFunction().asCfgNode().(NameNode).getId() in ["sorted", "reversed", "iter", "next"] and
+    call.getArg(0) = nodeFrom
   )
   or
   // methods
-  exists(CallNode call, string name | call = nodeTo.asCfgNode() |
+  exists(DataFlow::CallCfgNode call, string name | call = nodeTo |
     name in [
         // general
         "copy", "pop",
         // dict
         "values", "items", "get", "popitem"
       ] and
-    call.getFunction().(AttrNode).getObject(name) = nodeFrom.asCfgNode()
+    call.getFunction().(DataFlow::AttrRead).getObject(name) = nodeFrom
   )
   or
   // list.append, set.add
-  exists(CallNode call, string name |
+  exists(DataFlow::CallCfgNode call, string name |
     name in ["append", "add"] and
-    call.getFunction().(AttrNode).getObject(name) =
-      nodeTo.(DataFlow::PostUpdateNode).getPreUpdateNode().asCfgNode() and
-    call.getArg(0) = nodeFrom.getNode()
+    call.getFunction().(DataFlow::AttrRead).getObject(name).getPostUpdateNode() = nodeTo and
+    call.getArg(0) = nodeFrom
   )
 }
 

From 53f1d2342d6ecb09dfc8398ea5864165f1413722 Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Wed, 19 May 2021 12:19:18 +0200
Subject: [PATCH 04/14] Python: Small refactor of TaintTrackingPrivate

Highlight why we need to import `DataFlowPrivate`
---
 .../python/dataflow/new/internal/TaintTrackingPrivate.qll     | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll b/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll
index 37ed456b0cf3..09479c3aae52 100644
--- a/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll
+++ b/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll
@@ -1,6 +1,6 @@
 private import python
 private import semmle.python.dataflow.new.DataFlow
-private import semmle.python.dataflow.new.internal.DataFlowPrivate
+private import semmle.python.dataflow.new.internal.DataFlowPrivate as DataFlowPrivate
 private import semmle.python.dataflow.new.internal.TaintTrackingPublic
 
 /**
@@ -148,7 +148,7 @@ predicate stringManipulation(DataFlow::CfgNode nodeFrom, DataFlow::CfgNode nodeT
 predicate containerStep(DataFlow::CfgNode nodeFrom, DataFlow::Node nodeTo) {
   // construction by literal
   // TODO: Not limiting the content argument here feels like a BIG hack, but we currently get nothing for free :|
-  storeStep(nodeFrom, _, nodeTo)
+  DataFlowPrivate::storeStep(nodeFrom, _, nodeTo)
   or
   // constructor call
   exists(DataFlow::CallCfgNode call | call = nodeTo |

From a2e8417c11d1dd5cc13ea5e51b869e7a1afc5508 Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Wed, 19 May 2021 12:39:10 +0200
Subject: [PATCH 05/14] Python: Use API graphs in TaintTrackingPrivate

Some of this modeling could probably go to the standard lib modeling
file, but this chain of commits is already pretty feature creep :|
---
 .../new/internal/TaintTrackingPrivate.qll     | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll b/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll
index 09479c3aae52..9fe62fca8ad8 100644
--- a/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll
+++ b/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll
@@ -2,6 +2,7 @@ private import python
 private import semmle.python.dataflow.new.DataFlow
 private import semmle.python.dataflow.new.internal.DataFlowPrivate as DataFlowPrivate
 private import semmle.python.dataflow.new.internal.TaintTrackingPublic
+private import semmle.python.ApiGraphs
 
 /**
  * Holds if `node` should be a sanitizer in all global taint flow configurations
@@ -152,15 +153,14 @@ predicate containerStep(DataFlow::CfgNode nodeFrom, DataFlow::Node nodeTo) {
   or
   // constructor call
   exists(DataFlow::CallCfgNode call | call = nodeTo |
-    call.getFunction().asCfgNode().(NameNode).getId() in [
-        "list", "set", "frozenset", "dict", "defaultdict", "tuple"
-      ] and
+    call = API::builtin(["list", "set", "frozenset", "dict", "tuple"]).getACall() and
     call.getArg(0) = nodeFrom
+    // TODO: Properly handle defaultdict/namedtuple
   )
   or
   // functions operating on collections
   exists(DataFlow::CallCfgNode call | call = nodeTo |
-    call.getFunction().asCfgNode().(NameNode).getId() in ["sorted", "reversed", "iter", "next"] and
+    call = API::builtin(["sorted", "reversed", "iter", "next"]).getACall() and
     call.getArg(0) = nodeFrom
   )
   or
@@ -187,14 +187,9 @@ predicate containerStep(DataFlow::CfgNode nodeFrom, DataFlow::Node nodeTo) {
  * Holds if taint can flow from `nodeFrom` to `nodeTo` with a step related to copying.
  */
 predicate copyStep(DataFlow::CfgNode nodeFrom, DataFlow::CfgNode nodeTo) {
-  exists(CallNode call | call = nodeTo.getNode() |
-    // Fully qualified: copy.copy, copy.deepcopy
-    (
-      call.getFunction().(NameNode).getId() in ["copy", "deepcopy"]
-      or
-      call.getFunction().(AttrNode).getObject(["copy", "deepcopy"]).(NameNode).getId() = "copy"
-    ) and
-    call.getArg(0) = nodeFrom.getNode()
+  exists(DataFlow::CallCfgNode call | call = nodeTo |
+    call = API::moduleImport("copy").getMember(["copy", "deepcopy"]).getACall() and
+    call.getArg(0) = nodeFrom
   )
 }
 

From aa8b7306a3d3ccb8eb5c037ca443d4b6384f914d Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Wed, 19 May 2021 12:41:28 +0200
Subject: [PATCH 06/14] Python: Use more API graphs in TaintTrackingPrivate

But now we suddenly don't handle the call to `unicode` :O -- at least
not when I run the test locally (using Python 3).
---
 .../dataflow/new/internal/TaintTrackingPrivate.qll     | 10 +++-------
 .../defaultAdditionalTaintStep/test_string.py          |  4 ++--
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll b/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll
index 9fe62fca8ad8..979f3fe49a6f 100644
--- a/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll
+++ b/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll
@@ -76,13 +76,9 @@ predicate subscriptStep(DataFlow::CfgNode nodeFrom, DataFlow::CfgNode nodeTo) {
  */
 predicate stringManipulation(DataFlow::CfgNode nodeFrom, DataFlow::CfgNode nodeTo) {
   // transforming something tainted into a string will make the string tainted
-  exists(CallNode call | call = nodeTo.getNode() |
-    call.getFunction().(NameNode).getId() in ["str", "bytes", "unicode"] and
-    (
-      nodeFrom.getNode() = call.getArg(0)
-      or
-      nodeFrom.getNode() = call.getArgByName("object")
-    )
+  exists(DataFlow::CallCfgNode call | call = nodeTo |
+    call = API::builtin(["str", "bytes", "unicode"]).getACall() and
+    nodeFrom in [call.getArg(0), call.getArgByName("object")]
   )
   or
   // String methods. Note that this doesn't recognize `meth = "foo".upper; meth()`
diff --git a/python/ql/test/experimental/dataflow/tainttracking/defaultAdditionalTaintStep/test_string.py b/python/ql/test/experimental/dataflow/tainttracking/defaultAdditionalTaintStep/test_string.py
index eed3543bc170..559375d1edf5 100644
--- a/python/ql/test/experimental/dataflow/tainttracking/defaultAdditionalTaintStep/test_string.py
+++ b/python/ql/test/experimental/dataflow/tainttracking/defaultAdditionalTaintStep/test_string.py
@@ -32,7 +32,7 @@ def str_operations():
         ts[0], # $ tainted
         str(ts), # $ tainted
         bytes(tb), # $ tainted
-        unicode(ts), # $ tainted
+        unicode(ts), # $ MISSING: tainted
     )
 
     aug_assignment = "safe"
@@ -104,7 +104,7 @@ def non_syntactic():
     _str = str
     ensure_tainted(
         meth(), # $ MISSING: tainted
-        _str(ts), # $ MISSING: tainted
+        _str(ts), # $ tainted
     )
 
 

From c4987e94e02b9037f6901b64cbc11ae5d335b0fd Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Wed, 19 May 2021 12:44:26 +0200
Subject: [PATCH 07/14] Python: Re-introduce syntactic handling of
 str/bytes/unicode

I don't want to loose results on this, so until type-tracking/API graphs
can handle this, I want to keep our syntactic handling.
---
 .../python/dataflow/new/internal/TaintTrackingPrivate.qll   | 6 +++++-
 .../tainttracking/defaultAdditionalTaintStep/test_string.py | 2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll b/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll
index 979f3fe49a6f..73fd2cfebc5b 100644
--- a/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll
+++ b/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll
@@ -77,7 +77,11 @@ predicate subscriptStep(DataFlow::CfgNode nodeFrom, DataFlow::CfgNode nodeTo) {
 predicate stringManipulation(DataFlow::CfgNode nodeFrom, DataFlow::CfgNode nodeTo) {
   // transforming something tainted into a string will make the string tainted
   exists(DataFlow::CallCfgNode call | call = nodeTo |
-    call = API::builtin(["str", "bytes", "unicode"]).getACall() and
+    (
+      call = API::builtin(["str", "bytes", "unicode"]).getACall()
+      or
+      call.getFunction().asCfgNode().(NameNode).getId() in ["str", "bytes", "unicode"]
+    ) and
     nodeFrom in [call.getArg(0), call.getArgByName("object")]
   )
   or
diff --git a/python/ql/test/experimental/dataflow/tainttracking/defaultAdditionalTaintStep/test_string.py b/python/ql/test/experimental/dataflow/tainttracking/defaultAdditionalTaintStep/test_string.py
index 559375d1edf5..42ac758bfffa 100644
--- a/python/ql/test/experimental/dataflow/tainttracking/defaultAdditionalTaintStep/test_string.py
+++ b/python/ql/test/experimental/dataflow/tainttracking/defaultAdditionalTaintStep/test_string.py
@@ -32,7 +32,7 @@ def str_operations():
         ts[0], # $ tainted
         str(ts), # $ tainted
         bytes(tb), # $ tainted
-        unicode(ts), # $ MISSING: tainted
+        unicode(ts), # $ tainted
     )
 
     aug_assignment = "safe"

From 870389addb296fb1eb7ab223d39bcb42f99b066c Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Mon, 14 Jun 2021 14:18:07 +0200
Subject: [PATCH 08/14] Revert "Python: Re-introduce syntactic handling of
 str/bytes/unicode"

This reverts commit c4987e94e02b9037f6901b64cbc11ae5d335b0fd.

Hoping that our new handling of builtins would solve this problem... but
it did not :|
---
 .../python/dataflow/new/internal/TaintTrackingPrivate.qll   | 6 +-----
 .../defaultAdditionalTaintStep/InlineTaintTest.expected     | 2 ++
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll b/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll
index 73fd2cfebc5b..979f3fe49a6f 100644
--- a/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll
+++ b/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll
@@ -77,11 +77,7 @@ predicate subscriptStep(DataFlow::CfgNode nodeFrom, DataFlow::CfgNode nodeTo) {
 predicate stringManipulation(DataFlow::CfgNode nodeFrom, DataFlow::CfgNode nodeTo) {
   // transforming something tainted into a string will make the string tainted
   exists(DataFlow::CallCfgNode call | call = nodeTo |
-    (
-      call = API::builtin(["str", "bytes", "unicode"]).getACall()
-      or
-      call.getFunction().asCfgNode().(NameNode).getId() in ["str", "bytes", "unicode"]
-    ) and
+    call = API::builtin(["str", "bytes", "unicode"]).getACall() and
     nodeFrom in [call.getArg(0), call.getArgByName("object")]
   )
   or
diff --git a/python/ql/test/experimental/dataflow/tainttracking/defaultAdditionalTaintStep/InlineTaintTest.expected b/python/ql/test/experimental/dataflow/tainttracking/defaultAdditionalTaintStep/InlineTaintTest.expected
index 79d760d87f42..bacfd46f6509 100644
--- a/python/ql/test/experimental/dataflow/tainttracking/defaultAdditionalTaintStep/InlineTaintTest.expected
+++ b/python/ql/test/experimental/dataflow/tainttracking/defaultAdditionalTaintStep/InlineTaintTest.expected
@@ -1,3 +1,5 @@
 argumentToEnsureNotTaintedNotMarkedAsSpurious
 untaintedArgumentToEnsureTaintedNotMarkedAsMissing
+| test_string.py:35:9:35:19 | test_string.py:35 | ERROR, you should add `# $ MISSING: tainted` annotation | unicode(..) |
 failures
+| test_string.py:35:22:35:32 | Comment # $ tainted | Missing result:tainted= |

From cc311ac4cda352af99d663aed721d99adc1737bb Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Mon, 14 Jun 2021 14:23:12 +0200
Subject: [PATCH 09/14] Python: Re-introduce syntactic handling of
 str/bytes/unicode (again)

This reverts commit 870389addb296fb1eb7ab223d39bcb42f99b066c.
---
 .../python/dataflow/new/internal/TaintTrackingPrivate.qll   | 6 +++++-
 .../defaultAdditionalTaintStep/InlineTaintTest.expected     | 2 --
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll b/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll
index 979f3fe49a6f..73fd2cfebc5b 100644
--- a/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll
+++ b/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll
@@ -77,7 +77,11 @@ predicate subscriptStep(DataFlow::CfgNode nodeFrom, DataFlow::CfgNode nodeTo) {
 predicate stringManipulation(DataFlow::CfgNode nodeFrom, DataFlow::CfgNode nodeTo) {
   // transforming something tainted into a string will make the string tainted
   exists(DataFlow::CallCfgNode call | call = nodeTo |
-    call = API::builtin(["str", "bytes", "unicode"]).getACall() and
+    (
+      call = API::builtin(["str", "bytes", "unicode"]).getACall()
+      or
+      call.getFunction().asCfgNode().(NameNode).getId() in ["str", "bytes", "unicode"]
+    ) and
     nodeFrom in [call.getArg(0), call.getArgByName("object")]
   )
   or
diff --git a/python/ql/test/experimental/dataflow/tainttracking/defaultAdditionalTaintStep/InlineTaintTest.expected b/python/ql/test/experimental/dataflow/tainttracking/defaultAdditionalTaintStep/InlineTaintTest.expected
index bacfd46f6509..79d760d87f42 100644
--- a/python/ql/test/experimental/dataflow/tainttracking/defaultAdditionalTaintStep/InlineTaintTest.expected
+++ b/python/ql/test/experimental/dataflow/tainttracking/defaultAdditionalTaintStep/InlineTaintTest.expected
@@ -1,5 +1,3 @@
 argumentToEnsureNotTaintedNotMarkedAsSpurious
 untaintedArgumentToEnsureTaintedNotMarkedAsMissing
-| test_string.py:35:9:35:19 | test_string.py:35 | ERROR, you should add `# $ MISSING: tainted` annotation | unicode(..) |
 failures
-| test_string.py:35:22:35:32 | Comment # $ tainted | Missing result:tainted= |

From 3b41c2f2041ececbf1cf1869ac04d0f3ce92e6ad Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Tue, 22 Jun 2021 15:12:35 +0200
Subject: [PATCH 10/14] Python: Use new `MethodCallNode` in
 TaintTrackingPrivate

---
 .../dataflow/new/internal/TaintTrackingPrivate.qll   | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll b/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll
index 73fd2cfebc5b..2d907c419dd3 100644
--- a/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll
+++ b/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll
@@ -165,20 +165,20 @@ predicate containerStep(DataFlow::CfgNode nodeFrom, DataFlow::Node nodeTo) {
   )
   or
   // methods
-  exists(DataFlow::CallCfgNode call, string name | call = nodeTo |
-    name in [
+  exists(DataFlow::MethodCallNode call, string methodName | call = nodeTo |
+    methodName in [
         // general
         "copy", "pop",
         // dict
         "values", "items", "get", "popitem"
       ] and
-    call.getFunction().(DataFlow::AttrRead).getObject(name) = nodeFrom
+    call.calls(nodeFrom, methodName)
   )
   or
   // list.append, set.add
-  exists(DataFlow::CallCfgNode call, string name |
-    name in ["append", "add"] and
-    call.getFunction().(DataFlow::AttrRead).getObject(name).getPostUpdateNode() = nodeTo and
+  exists(DataFlow::MethodCallNode call, DataFlow::Node obj |
+    call.calls(obj, ["append", "add"]) and
+    obj.getPostUpdateNode() = nodeTo and
     call.getArg(0) = nodeFrom
   )
 }

From e56dfe75bdd30e7254abd79fa56411eeabc64707 Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Tue, 22 Jun 2021 15:21:40 +0200
Subject: [PATCH 11/14] Python: AttrRef getOjbect/1 -> accesses/2

See this thread for discussion:
https://github.com/github/codeql/pull/5926#discussion_r635384981
---
 .../semmle/python/dataflow/new/internal/Attributes.qll    | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/python/ql/src/semmle/python/dataflow/new/internal/Attributes.qll b/python/ql/src/semmle/python/dataflow/new/internal/Attributes.qll
index b96b3d80858b..b85b8f213fc8 100644
--- a/python/ql/src/semmle/python/dataflow/new/internal/Attributes.qll
+++ b/python/ql/src/semmle/python/dataflow/new/internal/Attributes.qll
@@ -18,12 +18,10 @@ abstract class AttrRef extends Node {
   abstract Node getObject();
 
   /**
-   * Gets the data flow node corresponding to the object whose attribute named
-   * `attrName` is being read or written.
+   * Holds if this data flow node accesses attribute named `attrName` on object `object`.
    */
-  Node getObject(string attrName) {
-    result = this.getObject() and
-    attrName = this.getAttributeName()
+  predicate accesses(Node object, string attrName) {
+    this.getObject() = object and getAttributeName() = attrName
   }
 
   /**

From 7a6eee50ff8db0c284031186a6d157b1aca4532d Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Fri, 2 Jul 2021 13:23:02 +0200
Subject: [PATCH 12/14] Revert "Python: Add getPostUpdateNode to
 DataFlow::Node"

This reverts commit 9137f04bd330712c306ad819641e9328b466da72.
---
 .../semmle/python/dataflow/new/internal/DataFlowPrivate.qll    | 2 +-
 .../src/semmle/python/dataflow/new/internal/DataFlowPublic.qll | 3 ---
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/python/ql/src/semmle/python/dataflow/new/internal/DataFlowPrivate.qll b/python/ql/src/semmle/python/dataflow/new/internal/DataFlowPrivate.qll
index d831bfbfbd2b..074289b00fd4 100644
--- a/python/ql/src/semmle/python/dataflow/new/internal/DataFlowPrivate.qll
+++ b/python/ql/src/semmle/python/dataflow/new/internal/DataFlowPrivate.qll
@@ -19,7 +19,7 @@ module syntheticPreUpdateNode {
     SyntheticPreUpdateNode() { this = TSyntheticPreUpdateNode(post) }
 
     /** Gets the node for which this is a synthetic pre-update node. */
-    override NeedsSyntheticPreUpdateNode getPostUpdateNode() { result = post }
+    Node getPostUpdateNode() { result = post }
 
     override string toString() { result = "[pre " + post.label() + "] " + post.toString() }
 
diff --git a/python/ql/src/semmle/python/dataflow/new/internal/DataFlowPublic.qll b/python/ql/src/semmle/python/dataflow/new/internal/DataFlowPublic.qll
index c6039e64d80c..928082d7e8aa 100644
--- a/python/ql/src/semmle/python/dataflow/new/internal/DataFlowPublic.qll
+++ b/python/ql/src/semmle/python/dataflow/new/internal/DataFlowPublic.qll
@@ -123,9 +123,6 @@ class Node extends TNode {
    * Gets a local source node from which data may flow to this node in zero or more local data-flow steps.
    */
   LocalSourceNode getALocalSource() { result.flowsTo(this) }
-
-  /** Gets the `PostUpdateNode` for this node, if any. */
-  PostUpdateNode getPostUpdateNode() { result.getPreUpdateNode() = this }
 }
 
 /** A data-flow node corresponding to an SSA variable. */

From 22c155687e70f2e6accca7d861c89c4c6ed9adf1 Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Fri, 2 Jul 2021 13:25:25 +0200
Subject: [PATCH 13/14] Python: Fix code after removing `getPostUpdateNode`

---
 .../python/dataflow/new/internal/TaintTrackingPrivate.qll       | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll b/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll
index 2d907c419dd3..4d3353705eef 100644
--- a/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll
+++ b/python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll
@@ -178,7 +178,7 @@ predicate containerStep(DataFlow::CfgNode nodeFrom, DataFlow::Node nodeTo) {
   // list.append, set.add
   exists(DataFlow::MethodCallNode call, DataFlow::Node obj |
     call.calls(obj, ["append", "add"]) and
-    obj.getPostUpdateNode() = nodeTo and
+    obj = nodeTo.(DataFlow::PostUpdateNode).getPreUpdateNode() and
     call.getArg(0) = nodeFrom
   )
 }

From 81fab487a41bbb20d84ac32301e199377e4d94ec Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Fri, 2 Jul 2021 13:27:41 +0200
Subject: [PATCH 14/14] Python: Apply suggestions from code review

Co-authored-by: Taus <tausbn@github.com>
---
 .../ql/src/semmle/python/dataflow/new/internal/Attributes.qll   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/ql/src/semmle/python/dataflow/new/internal/Attributes.qll b/python/ql/src/semmle/python/dataflow/new/internal/Attributes.qll
index b85b8f213fc8..9d3e55c507b0 100644
--- a/python/ql/src/semmle/python/dataflow/new/internal/Attributes.qll
+++ b/python/ql/src/semmle/python/dataflow/new/internal/Attributes.qll
@@ -21,7 +21,7 @@ abstract class AttrRef extends Node {
    * Holds if this data flow node accesses attribute named `attrName` on object `object`.
    */
   predicate accesses(Node object, string attrName) {
-    this.getObject() = object and getAttributeName() = attrName
+    this.getObject() = object and this.getAttributeName() = attrName
   }
 
   /**