distinguish instance attribute models from class attribute models

Summary:
In Python, the semantics of class and instance variables differ. If you declare `class C: x = 1` and write `self.x = 2` later, `C.x` will remain unchanged for the other instances of `C`.

Previously, we would allow accessing an attribute model through both the instance and class. This disallows us from expressing models only for class attributes vs. only for instance attributes.

Support the class attribute case via the module_name.ClassName.__class__.attribute, and keep instance attribute syntax as is (via module_name.ClassName.attribute). If we need models for both cases, we'll need to write both.

Reviewed By: fahndrich

Differential Revision: D16672643

fbshipit-source-id: 92ff7bcf0863514ab5da670ed6f9e8474e78799e

analysis/test/lookupTest.ml

@@ -533,7 +533,7 @@ let test_lookup_definitions context =
       "test.py:12:4-12:7 -> test.py:8:0-9:8";
       "test.py:13:12-13:18 -> test.py:2:0-3:13";
       "test.py:13:4-13:11 -> test.py:5:0-6:8";
-      "test.py:2:16-2:19 -> builtins.pyi:76:0-93:34" ];
+      "test.py:2:16-2:19 -> builtins.pyi:77:0-94:34" ];
   assert_definition ~position:{ Location.line = 12; column = 0 } ~definition:None;
   assert_definition
     ~position:{ Location.line = 12; column = 4 }

taint/model.ml

@@ -560,6 +560,17 @@ let model_compatible ~type_parameters ~normalized_model_parameters =
   errors
 
 
+let demangle_class_attribute name =
+  if String.is_substring ~substring:"__class__" name then
+    String.split name ~on:'.'
+    |> List.rev
+    |> function
+    | attribute :: "__class__" :: rest -> List.rev (attribute :: rest) |> String.concat ~sep:"."
+    | _ -> name
+  else
+    name
+
+
 let create ~resolution ?path ~configuration source =
   let global_resolution = Resolution.global_resolution resolution in
   let signatures =
@@ -697,6 +708,7 @@ let create ~resolution ?path ~configuration source =
         let model_compatibility_errors =
           (* Make self as an explicit parameter in type's parameter list *)
           let implicit_to_explicit_self { Type.Callable.name; implicit_annotation } =
+            let name = demangle_class_attribute name in
             let open Type.Callable.RecordParameter in
             Named { name; annotation = implicit_annotation; default = false }
           in
@@ -823,14 +835,22 @@ let get_global_model ~resolution ~expression =
     match Node.value expression, AccessPath.get_global ~resolution expression with
     | _, Some global -> Some global
     | Name (Name.Attribute { base; attribute; _ }), _ ->
-        let annotation =
-          match Resolution.resolve resolution base with
-          | Type.Optional annotation -> annotation
-          | annotation ->
-              if Type.is_meta annotation then
-                Type.single_parameter annotation
-              else
-                annotation
+        let is_meta, annotation =
+          let rec is_meta = function
+            | Type.Optional annotation -> is_meta annotation
+            | annotation ->
+                if Type.is_meta annotation then
+                  true, Type.single_parameter annotation
+                else
+                  false, annotation
+          in
+          is_meta (Resolution.resolve resolution base)
+        in
+        let attribute =
+          if is_meta then
+            Format.sprintf "__class__.%s" attribute
+          else
+            attribute
         in
         annotation
         |> Type.class_name

taint/model.mli

@@ -16,6 +16,9 @@ type t = {
 
 exception InvalidModel of string
 
+(* Exposed for testing. *)
+val demangle_class_attribute : string -> string
+
 val get_callsite_model
   :  call_target:[< Callable.t ] ->
   arguments:Expression.t Expression.Call.Argument.t list ->

taint/test/integration/class_flows.py

@@ -1,29 +1,51 @@
 # @nolint
 
-from typing import Type
+from typing import Type, Optional
 
 
 class C:
     tainted_attribute: List[int] = []
+    tainted_class_attribute: List[int] = []
     not_tainted = 2
 
 
 class D(C):
     pass
 
 
-def tainted_attribute_flow(c: C) -> None:
+def tainted_attribute_flow_issue(c: C) -> None:
     c.tainted_attribute = __test_source()
 
 
-def untainted_flow(c: C) -> None:
+def untainted_flow_not_issue(c: C) -> None:
     c.not_tainted = __test_source()
 
 
-def tainted_attribute_for_class(c: Type[C]) -> None:
+def tainted_attribute_for_class_not_issue(c: Type[C]) -> None:
     c.tainted_attribute = __test_source()
 
 
-def tainted_attribute_through_inheritance(d: D) -> None:
+def tainted_attribute_through_inheritance_not_issue(d: D) -> None:
     # TODO(T47337940): Support this.
     d.tainted_attribute = __test_source()
+
+
+def tainted_class_attribute_through_instance_not_issue(c: C) -> None:
+    c.tainted_class_attribute = __test_source()
+
+
+def tainted_class_attribute_through_class_issue(class_object: Type[C]) -> None:
+    class_object.tainted_class_attribute = __test_source()
+
+
+def tainted_class_attribute_through_double_underscore_class_issue(c: C) -> None:
+    c.__class__.tainted_class_attribute = __test_source()
+
+
+def tainted_class_attribute_through_optional_class_issue(class_object: Optional[Type[C]]) -> None:
+    if class_object is not None:
+        class_object.tainted_class_attribute = __test_source()
+
+
+def global_class_attribute_issue() -> None:
+    C.tainted_class_attribute = __test_source()

taint/test/integration/class_flows.py.cg

@@ -1,6 +1,11 @@
 @generated
 Call dependencies
-class_flows.tainted_attribute_flow (fun) -> [__test_source (fun)]
-class_flows.tainted_attribute_for_class (fun) -> [__test_source (fun)]
-class_flows.tainted_attribute_through_inheritance (fun) -> [__test_source (fun)]
-class_flows.untainted_flow (fun) -> [__test_source (fun)]
+class_flows.global_class_attribute_issue (fun) -> [__test_source (fun)]
+class_flows.tainted_attribute_flow_issue (fun) -> [__test_source (fun)]
+class_flows.tainted_attribute_for_class_not_issue (fun) -> [__test_source (fun)]
+class_flows.tainted_attribute_through_inheritance_not_issue (fun) -> [__test_source (fun)]
+class_flows.tainted_class_attribute_through_class_issue (fun) -> [__test_source (fun)]
+class_flows.tainted_class_attribute_through_double_underscore_class_issue (fun) -> [__test_source (fun) object::__class__ (method)]
+class_flows.tainted_class_attribute_through_instance_not_issue (fun) -> [__test_source (fun)]
+class_flows.tainted_class_attribute_through_optional_class_issue (fun) -> [__test_source (fun)]
+class_flows.untainted_flow_not_issue (fun) -> [__test_source (fun)]

taint/test/integration/class_flows.py.models

@@ -2,12 +2,13 @@
 {
   "kind": "issue",
   "data": {
-    "callable": "class_flows.tainted_attribute_flow",
-    "callable_line": 15,
+    "callable":
+      "class_flows.tainted_class_attribute_through_double_underscore_class_issue",
+    "callable_line": 41,
     "code": 5002,
-    "line": 16,
+    "line": 42,
     "start": 4,
-    "end": 23,
+    "end": 39,
     "filename": "class_flows.py",
     "message":
       "Test flow. Data from [Test] source(s) may reach [Test] sink(s)",
@@ -18,9 +19,9 @@
           {
             "root": {
               "filename": "class_flows.py",
-              "line": 16,
-              "start": 39,
-              "end": 41
+              "line": 42,
+              "start": 55,
+              "end": 57
             },
             "leaves": [ { "kind": "Test", "name": "__test_source" } ],
             "features": [ { "via": "special_source" } ]
@@ -33,12 +34,116 @@
           {
             "root": {
               "filename": "class_flows.py",
-              "line": 16,
+              "line": 42,
               "start": 4,
-              "end": 23
+              "end": 39
             },
             "leaves": [
-              { "kind": "Test", "name": "class_flows.C.tainted_attribute" }
+              {
+                "kind": "Test",
+                "name": "class_flows.C.__class__.tainted_class_attribute"
+              }
+            ]
+          }
+        ]
+      }
+    ]
+  }
+}
+{
+  "kind": "issue",
+  "data": {
+    "callable":
+      "class_flows.tainted_class_attribute_through_optional_class_issue",
+    "callable_line": 45,
+    "code": 5002,
+    "line": 47,
+    "start": 8,
+    "end": 44,
+    "filename": "class_flows.py",
+    "message":
+      "Test flow. Data from [Test] source(s) may reach [Test] sink(s)",
+    "traces": [
+      {
+        "name": "forward",
+        "roots": [
+          {
+            "root": {
+              "filename": "class_flows.py",
+              "line": 47,
+              "start": 60,
+              "end": 62
+            },
+            "leaves": [ { "kind": "Test", "name": "__test_source" } ],
+            "features": [ { "via": "special_source" } ]
+          }
+        ]
+      },
+      {
+        "name": "backward",
+        "roots": [
+          {
+            "root": {
+              "filename": "class_flows.py",
+              "line": 47,
+              "start": 8,
+              "end": 44
+            },
+            "leaves": [
+              {
+                "kind": "Test",
+                "name": "class_flows.C.__class__.tainted_class_attribute"
+              }
+            ]
+          }
+        ]
+      }
+    ]
+  }
+}
+{
+  "kind": "issue",
+  "data": {
+    "callable": "class_flows.global_class_attribute_issue",
+    "callable_line": 50,
+    "code": 5002,
+    "line": 51,
+    "start": 4,
+    "end": 29,
+    "filename": "class_flows.py",
+    "message":
+      "Test flow. Data from [Test] source(s) may reach [Test] sink(s)",
+    "traces": [
+      {
+        "name": "forward",
+        "roots": [
+          {
+            "root": {
+              "filename": "class_flows.py",
+              "line": 51,
+              "start": 45,
+              "end": 47
+            },
+            "leaves": [ { "kind": "Test", "name": "__test_source" } ],
+            "features": [ { "via": "special_source" } ]
+          }
+        ]
+      },
+      {
+        "name": "backward",
+        "roots": [
+          {
+            "root": {
+              "filename": "class_flows.py",
+              "line": 51,
+              "start": 4,
+              "end": 29
+            },
+            "leaves": [
+              {
+                "kind": "Test",
+                "name": "class_flows.C.__class__.tainted_class_attribute"
+              }
             ]
           }
         ]
@@ -49,10 +154,10 @@
 {
   "kind": "issue",
   "data": {
-    "callable": "class_flows.tainted_attribute_for_class",
-    "callable_line": 23,
+    "callable": "class_flows.tainted_attribute_flow_issue",
+    "callable_line": 16,
     "code": 5002,
-    "line": 24,
+    "line": 17,
     "start": 4,
     "end": 23,
     "filename": "class_flows.py",
@@ -65,7 +170,7 @@
           {
             "root": {
               "filename": "class_flows.py",
-              "line": 24,
+              "line": 17,
               "start": 39,
               "end": 41
             },
@@ -80,7 +185,7 @@
           {
             "root": {
               "filename": "class_flows.py",
-              "line": 24,
+              "line": 17,
               "start": 4,
               "end": 23
             },
@@ -93,3 +198,53 @@
     ]
   }
 }
+{
+  "kind": "issue",
+  "data": {
+    "callable": "class_flows.tainted_class_attribute_through_class_issue",
+    "callable_line": 37,
+    "code": 5002,
+    "line": 38,
+    "start": 4,
+    "end": 40,
+    "filename": "class_flows.py",
+    "message":
+      "Test flow. Data from [Test] source(s) may reach [Test] sink(s)",
+    "traces": [
+      {
+        "name": "forward",
+        "roots": [
+          {
+            "root": {
+              "filename": "class_flows.py",
+              "line": 38,
+              "start": 56,
+              "end": 58
+            },
+            "leaves": [ { "kind": "Test", "name": "__test_source" } ],
+            "features": [ { "via": "special_source" } ]
+          }
+        ]
+      },
+      {
+        "name": "backward",
+        "roots": [
+          {
+            "root": {
+              "filename": "class_flows.py",
+              "line": 38,
+              "start": 4,
+              "end": 40
+            },
+            "leaves": [
+              {
+                "kind": "Test",
+                "name": "class_flows.C.__class__.tainted_class_attribute"
+              }
+            ]
+          }
+        ]
+      }
+    ]
+  }
+}

taint/test/integration/class_flows.py.pysa

@@ -1,2 +1,3 @@
 class_flows.C.tainted_attribute: TaintSink[Test] = ...
+class_flows.C.__class__.tainted_class_attribute: TaintSink[Test] = ...
 def list.append(self, element: TaintInTaintOut[Updates[self]]): ...