Indirect function call chain false negative

Summary:
FN determined thanks to marat-turaev 's comment.

Thanks gbleaney for pointing me to a better place to add these tests/explaining the historical context behind the builder pattern.

Reviewed By: tianhan0

Differential Revision: D48344707

fbshipit-source-id: 920b30faf33492f050b63561a72a57d75296eb71

source/interprocedural_analyses/taint/test/integration/builder_pattern.py

@@ -34,6 +34,12 @@ def set_not_saved_through_typevar(self: T, not_saved: str) -> T:
         self._not_saved = not_saved
         return self
 
+    def return_self(self) -> "Builder":
+        return self
+
+    def set_saved_no_return(self, saved: str) -> None:
+        self._saved = saved
+
 
 def test_no_issue():
     builder = Builder()
@@ -59,6 +65,36 @@ def test_issue_with_type_var():
     ).async_save()
 
 
+def test_chained_class_setter():
+    # TODO(T161085814): False negative due to no model for
+    # set_saved_no_return, so no taint is propagated
+    builder = Builder()
+    builder.return_self().set_saved_no_return(_test_source())
+    _test_sink(builder)
+    _test_sink(builder._saved)
+
+
+def test_class_setter():
+    # TODO(T161085814): False negative due to no model for
+    # set_saved_no_return, so no taint is propagated
+    builder = Builder()
+    builder.set_saved_no_return(_test_source())
+    _test_sink(builder)
+    _test_sink(builder._saved)
+
+
+def test_taint_update_receiver_declaration():
+    # TODO(T161085814): False Negative due to not passing around
+    # the fact that builder and return_self() are the same for the
+    # chained builder pattern. return_self and set_saved both
+    # have valid models.
+    builder = Builder()
+    builder.return_self().set_saved(_test_source())
+    _test_sink(builder)
+    _test_sink(builder._saved)
+    _test_sink(builder.return_self())
+
+
 class SubBuilder(Builder):
     pass
 

source/interprocedural_analyses/taint/test/integration/builder_pattern.py.cg

@@ -1,17 +1,22 @@
 @generated
 Call dependencies
+builder_pattern.test_chained_class_setter (fun) -> [_test_sink (fun) _test_source (fun) builder_pattern.Builder::__init__ (method) builder_pattern.Builder::return_self (method) builder_pattern.Builder::set_saved_no_return (method) object::__new__ (method)]
 builder_pattern.test_issue_with_sub_builder (fun) -> [_test_source (fun) builder_pattern.Builder::__init__ (method) builder_pattern.Builder::async_save (method) builder_pattern.Builder::set_not_saved_through_typevar (method) builder_pattern.Builder::set_saved_through_typevar (method) object::__new__ (method)]
 builder_pattern.test_issue_with_type_var (fun) -> [_test_source (fun) builder_pattern.Builder::__init__ (method) builder_pattern.Builder::async_save (method) builder_pattern.Builder::set_not_saved_through_typevar (method) builder_pattern.Builder::set_saved_through_typevar (method) object::__new__ (method)]
 builder_pattern.test_no_issue_with_sub_builder (fun) -> [_test_source (fun) builder_pattern.Builder::__init__ (method) builder_pattern.Builder::async_save (method) builder_pattern.Builder::set_not_saved_through_typevar (method) builder_pattern.Builder::set_saved_through_typevar (method) object::__new__ (method)]
 builder_pattern.test_no_issue_with_type_var (fun) -> [_test_source (fun) builder_pattern.Builder::__init__ (method) builder_pattern.Builder::async_save (method) builder_pattern.Builder::set_not_saved_through_typevar (method) builder_pattern.Builder::set_saved_through_typevar (method) object::__new__ (method)]
+builder_pattern.test_taint_update_receiver_declaration (fun) -> [_test_sink (fun) _test_source (fun) builder_pattern.Builder::__init__ (method) builder_pattern.Builder::return_self (method) builder_pattern.Builder::set_saved (method) object::__new__ (method)]
 builder_pattern.$toplevel (fun) -> [builder_pattern.T (object)]
+builder_pattern.test_class_setter (fun) -> [_test_sink (fun) _test_source (fun) builder_pattern.Builder::__init__ (method) builder_pattern.Builder::set_saved_no_return (method) object::__new__ (method)]
 builder_pattern.test_issue (fun) -> [_test_source (fun) builder_pattern.Builder::__init__ (method) builder_pattern.Builder::async_save (method) builder_pattern.Builder::set_not_saved (method) builder_pattern.Builder::set_saved (method) object::__new__ (method)]
 builder_pattern.test_no_issue (fun) -> [_test_source (fun) builder_pattern.Builder::__init__ (method) builder_pattern.Builder::async_save (method) builder_pattern.Builder::set_not_saved (method) builder_pattern.Builder::set_saved (method) object::__new__ (method)]
 builder_pattern.Builder::set_not_saved_through_typevar (method) -> []
+builder_pattern.Builder::set_saved_no_return (method) -> []
 builder_pattern.Builder::set_saved_through_typevar (method) -> []
 builder_pattern.Builder::$class_toplevel (method) -> []
 builder_pattern.Builder::__init__ (method) -> []
 builder_pattern.Builder::async_save (method) -> [_test_sink (fun)]
+builder_pattern.Builder::return_self (method) -> []
 builder_pattern.Builder::set_not_saved (method) -> []
 builder_pattern.Builder::set_saved (method) -> []
 builder_pattern.SubBuilder::$class_toplevel (method) -> []

source/interprocedural_analyses/taint/test/integration/builder_pattern.py.models

@@ -3,9 +3,9 @@
   "kind": "issue",
   "data": {
     "callable": "builder_pattern.test_issue",
-    "callable_line": 43,
+    "callable_line": 49,
     "code": 5002,
-    "line": 45,
+    "line": 51,
     "start": 4,
     "end": 61,
     "filename": "builder_pattern.py",
@@ -25,10 +25,10 @@
               }
             ],
             "local_features": [ { "always-via": "tito" } ],
-            "tito_positions": [ { "line": 45, "start": 46, "end": 60 } ],
+            "tito_positions": [ { "line": 51, "start": 46, "end": 60 } ],
             "origin": {
               "filename": "builder_pattern.py",
-              "line": 45,
+              "line": 51,
               "start": 46,
               "end": 60
             }
@@ -52,7 +52,7 @@
             "call": {
               "position": {
                 "filename": "builder_pattern.py",
-                "line": 45,
+                "line": 51,
                 "start": 4,
                 "end": 61
               },
@@ -81,9 +81,9 @@
   "kind": "issue",
   "data": {
     "callable": "builder_pattern.test_issue_with_sub_builder",
-    "callable_line": 73,
+    "callable_line": 109,
     "code": 5002,
-    "line": 75,
+    "line": 111,
     "start": 4,
     "end": 5,
     "filename": "builder_pattern.py",
@@ -103,10 +103,10 @@
               }
             ],
             "local_features": [ { "always-via": "tito" } ],
-            "tito_positions": [ { "line": 76, "start": 8, "end": 22 } ],
+            "tito_positions": [ { "line": 112, "start": 8, "end": 22 } ],
             "origin": {
               "filename": "builder_pattern.py",
-              "line": 76,
+              "line": 112,
               "start": 8,
               "end": 22
             }
@@ -130,7 +130,7 @@
             "call": {
               "position": {
                 "filename": "builder_pattern.py",
-                "line": 75,
+                "line": 111,
                 "start": 4,
                 "end": 4
               },
@@ -159,9 +159,9 @@
   "kind": "issue",
   "data": {
     "callable": "builder_pattern.test_issue_with_type_var",
-    "callable_line": 55,
+    "callable_line": 61,
     "code": 5002,
-    "line": 57,
+    "line": 63,
     "start": 4,
     "end": 5,
     "filename": "builder_pattern.py",
@@ -181,10 +181,10 @@
               }
             ],
             "local_features": [ { "always-via": "tito" } ],
-            "tito_positions": [ { "line": 58, "start": 8, "end": 22 } ],
+            "tito_positions": [ { "line": 64, "start": 8, "end": 22 } ],
             "origin": {
               "filename": "builder_pattern.py",
-              "line": 58,
+              "line": 64,
               "start": 8,
               "end": 22
             }
@@ -208,7 +208,7 @@
             "call": {
               "position": {
                 "filename": "builder_pattern.py",
-                "line": 57,
+                "line": 63,
                 "start": 4,
                 "end": 4
               },
@@ -438,6 +438,23 @@
     ]
   }
 }
+{
+  "kind": "model",
+  "data": {
+    "callable": "builder_pattern.Builder.return_self",
+    "tito": [
+      {
+        "port": "formal(self)",
+        "taint": [
+          {
+            "kinds": [ { "return_paths": { "": 4 }, "kind": "LocalReturn" } ],
+            "tito": null
+          }
+        ]
+      }
+    ]
+  }
+}
 {
   "kind": "model",
   "data": {