Merge pull request #14706 from RasmusWL/class-attribute-flow

Python: Add basic flow for class attributes

Author: RasmusWL

python/ql/lib/change-notes/2023-11-07-class-attribute-flow.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Added basic flow for attributes defined on classes, when the attribute lookup is on a direct reference to that class (so not instance, cls parameter, or self parameter). Example: class definition `class Foo: my_tuples = (dangerous, safe)` and usage `SINK(Foo.my_tuples[0])`.

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

@@ -846,10 +846,27 @@ predicate comprehensionStoreStep(CfgNode nodeFrom, Content c, CfgNode nodeTo) {
  * ```
  * data flows from `x` to the attribute `foo` of  (the post-update node for) `obj`.
  */
-predicate attributeStoreStep(Node nodeFrom, AttributeContent c, PostUpdateNode nodeTo) {
-  exists(AttrWrite write |
-    write.accesses(nodeTo.getPreUpdateNode(), c.getAttribute()) and
-    nodeFrom = write.getValue()
+predicate attributeStoreStep(Node nodeFrom, AttributeContent c, Node nodeTo) {
+  exists(Node object |
+    // Normally we target a PostUpdateNode. However, for class definitions the class
+    // is only constructed after evaluating its' entire scope, so in terms of python
+    // evaluations there is no post or pre update nodes, just one node for the class
+    // expression. Therefore we target the class expression directly.
+    //
+    // Note: Due to the way we handle decorators, using a class decorator will result in
+    // there being a post-update node for the class (argument to the decorator). We do
+    // not want to differentiate between these two cases, so still target the class
+    // expression directly.
+    object = nodeTo.(PostUpdateNode).getPreUpdateNode() and
+    not object.asExpr() instanceof ClassExpr
+    or
+    object = nodeTo and
+    object.asExpr() instanceof ClassExpr
+  |
+    exists(AttrWrite write |
+      write.accesses(object, c.getAttribute()) and
+      nodeFrom = write.getValue()
+    )
   )
 }
 

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

@@ -338,9 +338,9 @@ class C:
 
 @expects(2)
 def test_attribute_reference():
-    SINK(C.a) #$ MISSING:flow="SOURCE, l:-4 -> C.a"
+    SINK(C.a) # $ flow="SOURCE, l:-5 -> C.a"
     c = C()
-    SINK(c.a) #$ MISSING:flow="SOURCE, l:-6 -> c.a"
+    SINK(c.a) # $ MISSING: flow="SOURCE, l:-7 -> c.a"
 
 # overriding __getattr__ should be tested by the class coverage tests
 

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

@@ -237,6 +237,124 @@ def set_attr(obj):
     SINK(y.attr) # $ MISSING: flow
     SINK_F(z.attr) # $ MISSING: flow
 
+# ------------------------------------------------------------------------------
+# Content in class attribute
+# ------------------------------------------------------------------------------
+
+class WithTuple:
+    my_tuple = (SOURCE, NONSOURCE)
+
+    def test_inst(self):
+        SINK(self.my_tuple[0]) # $ MISSING: flow
+        SINK_F(self.my_tuple[1])
+
+    def test_inst_no_call(self):
+        SINK(self.my_tuple[0]) # $ MISSING: flow
+        SINK_F(self.my_tuple[1])
+
+    @classmethod
+    def test_cm(cls):
+        SINK(cls.my_tuple[0]) # $ flow="SOURCE, l:-12 -> cls.my_tuple[0]"
+        SINK_F(cls.my_tuple[1])
+
+    @classmethod
+    def test_cm_no_call(cls):
+        SINK(cls.my_tuple[0]) # $ MISSING: flow="SOURCE, l:-8 -> cls.my_tuple[0]"
+        SINK_F(cls.my_tuple[1])
+
+
+@expects(2*4) # $ unresolved_call=expects(..) unresolved_call=expects(..)(..)
+def test_WithTuple():
+    SINK(WithTuple.my_tuple[0]) # $ flow="SOURCE, l:-23 -> WithTuple.my_tuple[0]"
+    SINK_F(WithTuple.my_tuple[1])
+
+    WithTuple.test_cm()
+
+    inst = WithTuple()
+    inst.test_inst()
+
+    SINK(inst.my_tuple[0]) # $ MISSING: flow
+    SINK_F(inst.my_tuple[1])
+
+
+@expects(4) # $ unresolved_call=expects(..) unresolved_call=expects(..)(..)
+def test_inst_override():
+    inst = WithTuple()
+
+    # setting attribute on instance does not override class attribute, it's only on the
+    # instance!
+    inst.my_tuple = (NONSOURCE, SOURCE)
+
+    SINK_F(inst.my_tuple[0])
+    SINK(inst.my_tuple[1]) # $ flow="SOURCE, l:-3 -> inst.my_tuple[1]"
+
+    SINK(WithTuple.my_tuple[0]) # $ flow="SOURCE, l:-46 -> WithTuple.my_tuple[0]"
+    SINK_F(WithTuple.my_tuple[1])
+
+
+class WithTuple2:
+    my_tuple = (NONSOURCE,)
+
+def set_to_source():
+    WithTuple2.my_tuple = (SOURCE,)
+
+@expects(4) # $ unresolved_call=expects(..) unresolved_call=expects(..)(..)
+def test_global_flow_to_class_attribute():
+    inst = WithTuple2()
+    SINK_F(WithTuple2.my_tuple[0])
+    SINK_F(inst.my_tuple[0])
+
+    set_to_source()
+
+    SINK(WithTuple2.my_tuple[0]) # $ MISSING: flow="SOURCE, l:-10 -> WithTuple2.my_tuple[0]"
+    SINK(inst.my_tuple[0]) # $ MISSING: flow="SOURCE, l:-11 -> inst.my_tuple[0]"
+
+
+class Outer:
+    src = SOURCE
+    class Inner:
+        src = SOURCE
+
+@expects(2) # $ unresolved_call=expects(..) unresolved_call=expects(..)(..)
+def test_nested_class():
+    SINK(Outer.src) # $ flow="SOURCE, l:-6 -> Outer.src"
+    SINK(Outer.Inner.src) # $ flow="SOURCE, l:-5 -> Outer.Inner.src"
+
+# --------------------------------------
+# unique classes from functions
+# --------------------------------------
+def make_class():
+    # a fresh class is returned each time this function is called
+    class C:
+        my_tuple = (NONSOURCE,)
+    return C
+
+@expects(8) # $ unresolved_call=expects(..) unresolved_call=expects(..)(..)
+def test_unique_class():
+    # This test highlights that if we use the _ClassExpr_ itself as the target/source
+    # for jumpsteps, we will end up with spurious flow (that is, we will think that
+    # x_cls and y_cls are the same, so by updating .my_tuple on x_cls we might propagate
+    # that to y_cls as well -- it might not matter too much in reality, but certainly an
+    # interesting corner case)
+    x_cls = make_class()
+    y_cls = make_class()
+
+    assert x_cls != y_cls
+
+    x_inst = x_cls()
+    y_inst = y_cls()
+
+    SINK_F(x_cls.my_tuple[0])
+    SINK_F(x_inst.my_tuple[0])
+    SINK_F(y_cls.my_tuple[0])
+    SINK_F(y_inst.my_tuple[0])
+
+    x_cls.my_tuple = (SOURCE,)
+    SINK(x_cls.my_tuple[0]) # $ flow="SOURCE, l:-1 -> x_cls.my_tuple[0]"
+    SINK(x_inst.my_tuple[0]) # $ MISSING: flow="SOURCE, l:-2 -> x_inst.my_tuple[0]"
+    SINK_F(y_cls.my_tuple[0])
+    SINK_F(y_inst.my_tuple[0])
+
 # ------------------------------------------------------------------------------
 # Crosstalk test -- using different function based on conditional
 # ------------------------------------------------------------------------------

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

@@ -64,7 +64,7 @@ class Unsafe:
 def test_value_pattern():
     match SOURCE:
         case Unsafe.VALUE as x:
-            SINK(x) #$ flow="SOURCE, l:-2 -> x" MISSING: flow="SOURCE, l:-5 -> x"
+            SINK(x) #$ flow="SOURCE, l:-2 -> x" flow="SOURCE, l:-5 -> x"
 
 @expects(2)
 def test_sequence_pattern_tuple():

python/ql/test/library-tests/PointsTo/new/ImpliesDataflow.expected

@@ -3,9 +3,5 @@
 | code/l_calls.py:12:1:12:20 | ControlFlowNode for ClassExpr | code/l_calls.py:16:16:16:18 | ControlFlowNode for cls |
 | code/l_calls.py:12:1:12:20 | ControlFlowNode for ClassExpr | code/l_calls.py:24:13:24:22 | ControlFlowNode for Attribute() |
 | code/l_calls.py:12:1:12:20 | ControlFlowNode for ClassExpr | code/l_calls.py:25:16:25:16 | ControlFlowNode for a |
-| code/l_calls.py:33:5:33:23 | ControlFlowNode for FunctionExpr | code/l_calls.py:39:1:39:3 | ControlFlowNode for Attribute |
-| code/l_calls.py:48:5:48:30 | ControlFlowNode for FunctionExpr | code/l_calls.py:53:1:53:3 | ControlFlowNode for Attribute |
-| code/q_super.py:48:5:48:17 | ControlFlowNode for ClassExpr | code/q_super.py:51:25:51:29 | ControlFlowNode for Attribute |
-| code/q_super.py:63:5:63:17 | ControlFlowNode for ClassExpr | code/q_super.py:66:19:66:23 | ControlFlowNode for Attribute |
 | code/t_type.py:3:1:3:16 | ControlFlowNode for ClassExpr | code/t_type.py:6:1:6:9 | ControlFlowNode for type() |
 | code/t_type.py:3:1:3:16 | ControlFlowNode for ClassExpr | code/t_type.py:13:5:13:13 | ControlFlowNode for type() |