Python points-to: Don't track non-descriptor class attributes on instances. Update more tests.

2025-12-17 01:03:14 +01:00 · 2019-05-13 11:24:50 +01:00
parent beebd0e15c
commit d74c76510f
9 changed files with 34 additions and 23 deletions
--- a/python/ql/src/semmle/python/objects/Instances.qll
+++ b/python/ql/src/semmle/python/objects/Instances.qll
@@ -82,7 +82,12 @@ class SpecificInstanceInternal extends TSpecificInstance, ObjectInternal {
        exists(ObjectInternal cls_attr, CfgOrigin attr_orig |
            this.getClass().(ClassObjectInternal).lookup(name, cls_attr, attr_orig)
            |
-            cls_attr.isDescriptor() = false and value = cls_attr and origin = attr_orig
+            /* If class attribute is not a descriptor, that usually means it is some sort of
+             * default value and likely overridden by an instance attribute. In that case
+             * use `unknown` to signal that an attribute exists but to avoid false positives
+             * for due to using the default value.
+             */
+            cls_attr.isDescriptor() = false and value = ObjectInternal::unknown() and origin = CfgOrigin::unknown()
            or
            cls_attr.isDescriptor() = true and cls_attr.descriptorGetInstance(this, value, origin)
        )
--- a/python/ql/test/3/library-tests/PointsTo/attributes/Test.expected
+++ b/python/ql/test/3/library-tests/PointsTo/attributes/Test.expected
@@ -77,6 +77,7 @@
 | 56 | ControlFlowNode for Attribute | int 20 | 56 |
 | 56 | ControlFlowNode for IntegerLiteral | int 20 | 56 |
 | 56 | ControlFlowNode for c2 | C() | 52 |
+| 57 | ControlFlowNode for Attribute | int 1 | 10 |
 | 57 | ControlFlowNode for Attribute | int 10 | 54 |
 | 57 | ControlFlowNode for c1 | C() | 51 |
 | 58 | ControlFlowNode for Attribute | int 1 | 10 |
@@ -114,6 +115,7 @@
 | 72 | ControlFlowNode for Attribute | int 2 | 72 |
 | 72 | ControlFlowNode for IntegerLiteral | int 2 | 72 |
 | 72 | ControlFlowNode for self | self | 70 |
+| 73 | ControlFlowNode for Attribute | Attribute | 73 |
 | 73 | ControlFlowNode for Attribute() | NoneType None | 64 |
 | 73 | ControlFlowNode for self | self | 70 |
 | 74 | ControlFlowNode for Attribute | int 0 | 65 |
@@ -174,6 +176,7 @@
 | 100 | ControlFlowNode for self | self | 98 |
 | 100 | ControlFlowNode for setattr | Builtin-function setattr | 100 |
 | 100 | ControlFlowNode for setattr() | NoneType None | 100 |
+| 101 | ControlFlowNode for Attribute | Attribute | 101 |
 | 101 | ControlFlowNode for Attribute() | NoneType None | 92 |
 | 101 | ControlFlowNode for self | self | 98 |
 | 102 | ControlFlowNode for Str | str u'a' | 102 |
@@ -198,5 +201,6 @@
 | 109 | ControlFlowNode for self | self | 108 |
 | 109 | ControlFlowNode for setattr | Builtin-function setattr | 109 |
 | 109 | ControlFlowNode for setattr() | NoneType None | 109 |
+| 111 | ControlFlowNode for Attribute | int 0 | 109 |
 | 111 | ControlFlowNode for G | class G | 106 |
 | 111 | ControlFlowNode for G() | G() | 111 |
--- a/python/ql/test/3/library-tests/PointsTo/attributes/TestWithType.expected
+++ b/python/ql/test/3/library-tests/PointsTo/attributes/TestWithType.expected
@@ -77,6 +77,7 @@
 | 56 | ControlFlowNode for Attribute | int 20 | builtin-class int | 56 |
 | 56 | ControlFlowNode for IntegerLiteral | int 20 | builtin-class int | 56 |
 | 56 | ControlFlowNode for c2 | C() | class C | 52 |
+| 57 | ControlFlowNode for Attribute | int 1 | builtin-class int | 10 |
 | 57 | ControlFlowNode for Attribute | int 10 | builtin-class int | 54 |
 | 57 | ControlFlowNode for c1 | C() | class C | 51 |
 | 58 | ControlFlowNode for Attribute | int 1 | builtin-class int | 10 |
@@ -114,6 +115,7 @@
 | 72 | ControlFlowNode for Attribute | int 2 | builtin-class int | 72 |
 | 72 | ControlFlowNode for IntegerLiteral | int 2 | builtin-class int | 72 |
 | 72 | ControlFlowNode for self | self | class D | 70 |
+| 73 | ControlFlowNode for Attribute | Attribute | builtin-class method | 73 |
 | 73 | ControlFlowNode for Attribute() | NoneType None | builtin-class NoneType | 64 |
 | 73 | ControlFlowNode for self | self | class D | 70 |
 | 74 | ControlFlowNode for Attribute | int 0 | builtin-class int | 65 |
@@ -174,6 +176,7 @@
 | 100 | ControlFlowNode for self | self | class F | 98 |
 | 100 | ControlFlowNode for setattr | Builtin-function setattr | builtin-class builtin_function_or_method | 100 |
 | 100 | ControlFlowNode for setattr() | NoneType None | builtin-class NoneType | 100 |
+| 101 | ControlFlowNode for Attribute | Attribute | builtin-class method | 101 |
 | 101 | ControlFlowNode for Attribute() | NoneType None | builtin-class NoneType | 92 |
 | 101 | ControlFlowNode for self | self | class F | 98 |
 | 102 | ControlFlowNode for Str | str u'a' | builtin-class str | 102 |
@@ -198,5 +201,6 @@
 | 109 | ControlFlowNode for self | self | class G | 108 |
 | 109 | ControlFlowNode for setattr | Builtin-function setattr | builtin-class builtin_function_or_method | 109 |
 | 109 | ControlFlowNode for setattr() | NoneType None | builtin-class NoneType | 109 |
+| 111 | ControlFlowNode for Attribute | int 0 | builtin-class int | 109 |
 | 111 | ControlFlowNode for G | class G | builtin-class type | 106 |
 | 111 | ControlFlowNode for G() | G() | class G | 111 |
--- a/python/ql/test/3/library-tests/PointsTo/attributes/test.py
+++ b/python/ql/test/3/library-tests/PointsTo/attributes/test.py
@@ -54,7 +54,7 @@ def k(cond):
    c1.z = 10
    if cond:
        c2.z = 20
-    c1.z
+    c1.z # FP here due to self.attribute and local attribute
    c2.z
    c3.z
    c3.z = 30
@@ -85,7 +85,7 @@ class E(object):

 E().x

-#Make sure that we handle getattr and setattr as well as they are needed for protobuf stubs.
+#Make sure that we handle getattr and setattr as well

 class F(object):

--- a/python/ql/test/3/library-tests/classes/attr/list_attr.expected
+++ b/python/ql/test/3/library-tests/classes/attr/list_attr.expected
@@ -1,12 +1,8 @@
 | builtin-class list | __add__ | Builtin-method __add__ |
-| builtin-class list | __class__ | Property __class__ |
 | builtin-class list | __contains__ | Builtin-method __contains__ |
-| builtin-class list | __delattr__ | Builtin-method __delattr__ |
 | builtin-class list | __delitem__ | Builtin-method __delitem__ |
-| builtin-class list | __dir__ | Builtin-method __dir__ |
 | builtin-class list | __doc__ | str u'list() -> new empty list\nlist(iterable) -> new list initialized from iterable's items' |
 | builtin-class list | __eq__ | Builtin-method __eq__ |
-| builtin-class list | __format__ | Builtin-method __format__ |
 | builtin-class list | __ge__ | Builtin-method __ge__ |
 | builtin-class list | __getattribute__ | Builtin-method __getattribute__ |
 | builtin-class list | __getitem__ | Builtin-method __getitem__ |
@@ -21,17 +17,12 @@
 | builtin-class list | __lt__ | Builtin-method __lt__ |
 | builtin-class list | __mul__ | Builtin-method __mul__ |
 | builtin-class list | __ne__ | Builtin-method __ne__ |
-| builtin-class list | __new__ | Builtin-method __new__ |
-| builtin-class list | __reduce__ | Builtin-method __reduce__ |
-| builtin-class list | __reduce_ex__ | Builtin-method __reduce_ex__ |
+| builtin-class list | __new__ | builtin_function_or_method __new__ |
 | builtin-class list | __repr__ | Builtin-method __repr__ |
 | builtin-class list | __reversed__ | Builtin-method __reversed__ |
 | builtin-class list | __rmul__ | Builtin-method __rmul__ |
-| builtin-class list | __setattr__ | Builtin-method __setattr__ |
 | builtin-class list | __setitem__ | Builtin-method __setitem__ |
 | builtin-class list | __sizeof__ | Builtin-method __sizeof__ |
-| builtin-class list | __str__ | Builtin-method __str__ |
-| builtin-class list | __subclasshook__ | classmethod_descriptor __subclasshook__ |
 | builtin-class list | append | Builtin-method append |
 | builtin-class list | clear | Builtin-method clear |
 | builtin-class list | copy | Builtin-method copy |
@@ -60,13 +51,14 @@
 | class DerivedFromBuiltin | __iadd__ | Builtin-method __iadd__ |
 | class DerivedFromBuiltin | __imul__ | Builtin-method __imul__ |
 | class DerivedFromBuiltin | __init__ | Builtin-method __init__ |
+| class DerivedFromBuiltin | __init_subclass__ | classmethod_descriptor __init_subclass__ |
 | class DerivedFromBuiltin | __iter__ | Builtin-method __iter__ |
 | class DerivedFromBuiltin | __le__ | Builtin-method __le__ |
 | class DerivedFromBuiltin | __len__ | Builtin-method __len__ |
 | class DerivedFromBuiltin | __lt__ | Builtin-method __lt__ |
 | class DerivedFromBuiltin | __mul__ | Builtin-method __mul__ |
 | class DerivedFromBuiltin | __ne__ | Builtin-method __ne__ |
-| class DerivedFromBuiltin | __new__ | Builtin-method __new__ |
+| class DerivedFromBuiltin | __new__ | builtin_function_or_method __new__ |
 | class DerivedFromBuiltin | __reduce__ | Builtin-method __reduce__ |
 | class DerivedFromBuiltin | __reduce_ex__ | Builtin-method __reduce_ex__ |
 | class DerivedFromBuiltin | __repr__ | Builtin-method __repr__ |
--- a/python/ql/test/3/library-tests/six/test.expected
+++ b/python/ql/test/3/library-tests/six/test.expected
@@ -28,6 +28,7 @@
 | Module six | callable | Builtin-function callable |
 | Module six | callable | Function callable |
 | Module six | class_types | Tuple |
+| Module six | create_bound_method | builtin-class method |
 | Module six | get_function_closure | Attribute() |
 | Module six | get_function_code | Attribute() |
 | Module six | get_function_defaults | Attribute() |
@@ -45,8 +46,17 @@
 | Module six | iterlists | Function iterlists |
 | Module six | itervalues | Function itervalues |
 | Module six | moves | Module six.moves |
+| Module six | moves.__init__ | Module six.moves.__init__ |
+| Module six | moves.urllib | Module six.moves.urllib |
+| Module six | moves.urllib.__init__ | Module six.moves.urllib.__init__ |
+| Module six | moves.urllib_error | Module six.moves.urllib_error |
+| Module six | moves.urllib_parse | Module six.moves.urllib_parse |
+| Module six | moves.urllib_request | Module six.moves.urllib_request |
+| Module six | moves.urllib_response | Module six.moves.urllib_response |
+| Module six | moves.urllib_robotparser | Module six.moves.urllib_robotparser |
 | Module six | next | Builtin-function next |
 | Module six | operator | Module operator |
+| Module six | print_ | Function print_ |
 | Module six | remove_move | Function remove_move |
 | Module six | reraise | Function reraise |
 | Module six | string_types | Tuple |
@@ -86,6 +96,7 @@
 | Module six.__init__ | callable | Builtin-function callable |
 | Module six.__init__ | callable | Function callable |
 | Module six.__init__ | class_types | Tuple |
+| Module six.__init__ | create_bound_method | builtin-class method |
 | Module six.__init__ | get_function_closure | Attribute() |
 | Module six.__init__ | get_function_code | Attribute() |
 | Module six.__init__ | get_function_defaults | Attribute() |
@@ -105,6 +116,7 @@
 | Module six.__init__ | moves | Module six.moves |
 | Module six.__init__ | next | Builtin-function next |
 | Module six.__init__ | operator | Module operator |
+| Module six.__init__ | print_ | Function print_ |
 | Module six.__init__ | remove_move | Function remove_move |
 | Module six.__init__ | reraise | Function reraise |
 | Module six.__init__ | string_types | Tuple |
@@ -120,6 +132,7 @@
 | Module six.moves | PY3 | bool True |
 | Module six.moves | SimpleHTTPServer | Module http.server |
 | Module six.moves | StringIO | builtin-class _io.StringIO |
+| Module six.moves | UserString | class UserString |
 | Module six.moves | __name__ | str u'six.moves' |
 | Module six.moves | _thread | Module _thread |
 | Module six.moves | builtins | Module builtins |
@@ -161,6 +174,7 @@
 | Module six.moves | tkinter_tksimpledialog | Module tkinter.simpledialog |
 | Module six.moves | tkinter_ttk | Module tkinter.ttk |
 | Module six.moves | urllib | Module six.moves.urllib |
+| Module six.moves | urllib.__init__ | Module six.moves.urllib.__init__ |
 | Module six.moves | urllib_error | Module six.moves.urllib_error |
 | Module six.moves | urllib_parse | Module six.moves.urllib_parse |
 | Module six.moves | urllib_request | Module six.moves.urllib_request |
@@ -176,6 +190,7 @@
 | Module six.moves.__init__ | PY3 | bool True |
 | Module six.moves.__init__ | SimpleHTTPServer | Module http.server |
 | Module six.moves.__init__ | StringIO | builtin-class _io.StringIO |
+| Module six.moves.__init__ | UserString | class UserString |
 | Module six.moves.__init__ | __name__ | str u'six.moves' |
 | Module six.moves.__init__ | _thread | Module _thread |
 | Module six.moves.__init__ | builtins | Module builtins |
--- a/python/ql/test/library-tests/PointsTo/guarded/PointsTo.expected
+++ b/python/ql/test/library-tests/PointsTo/guarded/PointsTo.expected
@@ -49,14 +49,10 @@
 | test.py | 301 | ControlFlowNode for x | NoneType None | 291 |
 | test.py | 308 | ControlFlowNode for z | int 7 | 305 |
 | test.py | 314 | ControlFlowNode for b | NoneType None | 311 |
-| test.py | 330 | ControlFlowNode for Attribute | NoneType None | 324 |
-| test.py | 330 | ControlFlowNode for Attribute | int 3 | 324 |
 | test.py | 332 | ControlFlowNode for Attribute | NoneType None | 322 |
 | test.py | 332 | ControlFlowNode for Attribute | int 4 | 322 |
 | test.py | 337 | ControlFlowNode for Attribute | NoneType None | 324 |
 | test.py | 337 | ControlFlowNode for Attribute | int 3 | 324 |
-| test.py | 345 | ControlFlowNode for Attribute | NoneType None | 324 |
-| test.py | 345 | ControlFlowNode for Attribute | int 3 | 324 |
 | test.py | 347 | ControlFlowNode for Attribute | NoneType None | 322 |
 | test.py | 347 | ControlFlowNode for Attribute | int 4 | 322 |
 | test.py | 357 | ControlFlowNode for g1 | float 7.0 | 356 |
--- a/python/ql/test/library-tests/PointsTo/guarded/PointsToWithType.expected
+++ b/python/ql/test/library-tests/PointsTo/guarded/PointsToWithType.expected
@@ -49,14 +49,10 @@
 | test.py | 301 | ControlFlowNode for x | NoneType None | builtin-class NoneType | 291 |
 | test.py | 308 | ControlFlowNode for z | int 7 | builtin-class int | 305 |
 | test.py | 314 | ControlFlowNode for b | NoneType None | builtin-class NoneType | 311 |
-| test.py | 330 | ControlFlowNode for Attribute | NoneType None | builtin-class NoneType | 324 |
-| test.py | 330 | ControlFlowNode for Attribute | int 3 | builtin-class int | 324 |
 | test.py | 332 | ControlFlowNode for Attribute | NoneType None | builtin-class NoneType | 322 |
 | test.py | 332 | ControlFlowNode for Attribute | int 4 | builtin-class int | 322 |
 | test.py | 337 | ControlFlowNode for Attribute | NoneType None | builtin-class NoneType | 324 |
 | test.py | 337 | ControlFlowNode for Attribute | int 3 | builtin-class int | 324 |
-| test.py | 345 | ControlFlowNode for Attribute | NoneType None | builtin-class NoneType | 324 |
-| test.py | 345 | ControlFlowNode for Attribute | int 3 | builtin-class int | 324 |
 | test.py | 347 | ControlFlowNode for Attribute | NoneType None | builtin-class NoneType | 322 |
 | test.py | 347 | ControlFlowNode for Attribute | int 4 | builtin-class int | 322 |
 | test.py | 357 | ControlFlowNode for g1 | float 7.0 | builtin-class float | 356 |
--- a/python/ql/test/library-tests/PointsTo/metaclass/Failed.expected
+++ b/python/ql/test/library-tests/PointsTo/metaclass/Failed.expected
@@ -1,3 +1,2 @@
 | test.py:26:1:26:17 | class C5 | Failed to infer metaclass |
-| test.py:30:1:30:17 | class C6 | Decorator not understood |
 | test.py:30:1:30:17 | class C6 | Failed to infer metaclass |