Merge pull request #5015 from yoff/python-add-missing-postupdate-nodes

Python: add missing postupdate nodes

python/ql/src/semmle/python/dataflow/new/internal/DataFlowPrivate.qll

@@ -69,13 +69,27 @@ class StorePreUpdateNode extends NeedsSyntheticPostUpdateNode, CfgNode {
   override string label() { result = "store" }
 }
 
-/** A node marking the state change of an object after a read. */
+/**
+ * A node marking the state change of an object after a read.
+ *
+ * A reverse read happens when the result of a read is modified, e.g. in
+ * ```python
+ * l = [ mutable ]
+ * l[0].mutate()
+ * ```
+ * we may now have changed the content of `l`. To track this, there must be
+ * a postupdate node for `l`.
+ */
 class ReadPreUpdateNode extends NeedsSyntheticPostUpdateNode, CfgNode {
   ReadPreUpdateNode() {
     exists(Attribute a |
       node = a.getObject().getAFlowNode() and
       a.getCtx() instanceof Load
     )
+    or
+    node = any(SubscriptNode s).getObject()
+    or
+    node.getNode() = any(Call call).getKwargs()
   }
 
   override string label() { result = "read" }

python/ql/test/experimental/dataflow/coverage/test.py

@@ -591,3 +591,35 @@ def return_from_inner_scope(x):
 
 def test_return_from_inner_scope():
     SINK(return_from_inner_scope([]))  #$ flow="SOURCE, l:-3 -> return_from_inner_scope(..)"
+
+
+# Inspired by reverse read inconsistency check
+def insertAtA(d):
+    d["a"] = SOURCE
+
+def test_reverse_read_subscript():
+    d = {"a": NONSOURCE}
+    l = [d]
+    insertAtA(l[0])
+    SINK(d["a"])  #$ MISSING:flow="SOURCE, l-6 -> d['a']""
+
+def test_reverse_read_dict_arg():
+    d = {"a": NONSOURCE}
+    dd = {"d": d}
+    insertAtA(**dd)
+    SINK(d["a"])  #$ MISSING:flow="SOURCE, l-12 -> d['a']""
+
+
+class WithA:
+    def setA(self, v):
+        self.a = v
+
+    def __init__(self):
+        self.a = ""
+
+
+def test_reverse_read_subscript_cls():
+    withA = WithA()
+    l = [withA]
+    l[0].setA(SOURCE)
+    SINK(withA.a) #$ MISSING:flow="SOURCE, l:-1 -> self.a"