Infer tito to self by default

Summary:
A while ago, we implemented inference of taint propagation from arguments to the `self` argument.
This was gated by the `--infer-self-tito` flag for a while.
Since we use this in production for all our runs, let's just enable it by default.

Reviewed By: alexkassil

Differential Revision: D77671053

fbshipit-source-id: 7f76f05854b534a718767ca880a6d9ddd9501bb5

Author: arthaud

client/command_arguments.py

@@ -299,7 +299,7 @@ class AnalyzeArguments:
     enable_memory_profiling: bool = False
     enable_profiling: bool = False
     find_missing_flows: Optional[MissingFlowsKind] = None
-    infer_self_tito: bool = False
+    infer_self_tito: bool = True
     infer_argument_tito: bool = False
     log_identifier: Optional[str] = None
     maximum_model_source_tree_width: Optional[int] = None

client/commands/analyze.py

@@ -43,7 +43,7 @@ class Arguments:
     dump_call_graph: Optional[str] = None
     dump_model_query_results: Optional[str] = None
     find_missing_flows: Optional[str] = None
-    infer_self_tito: bool = False
+    infer_self_tito: bool = True
     infer_argument_tito: bool = False
     maximum_model_source_tree_width: Optional[int] = None
     maximum_model_sink_tree_width: Optional[int] = None

client/commands/tests/analyze_test.py

@@ -91,7 +91,7 @@ def assert_serialized(
                 ("dump_call_graph", "/call-graph"),
                 ("dump_model_query_results", "/model-query"),
                 ("find_missing_flows", "obscure"),
-                ("infer_self_tito", False),
+                ("infer_self_tito", True),
                 ("infer_argument_tito", False),
                 ("maximum_model_source_tree_width", 10),
                 ("maximum_model_sink_tree_width", 11),
@@ -177,7 +177,7 @@ def test_create_analyze_arguments(self) -> None:
                         dump_call_graph="/call-graph",
                         dump_model_query_results="/model-query",
                         find_missing_flows=command_arguments.MissingFlowsKind.TYPE,
-                        infer_self_tito=True,
+                        infer_self_tito=False,
                         infer_argument_tito=True,
                         maximum_model_source_tree_width=10,
                         maximum_model_sink_tree_width=11,
@@ -247,7 +247,7 @@ def test_create_analyze_arguments(self) -> None:
                     dump_call_graph="/call-graph",
                     dump_model_query_results="/model-query",
                     find_missing_flows="type",
-                    infer_self_tito=True,
+                    infer_self_tito=False,
                     infer_argument_tito=True,
                     maximum_model_source_tree_width=10,
                     maximum_model_sink_tree_width=11,

client/pyre.py

@@ -493,13 +493,13 @@ def pyre(
     help="Build the cache and exit without computing results..",
 )
 @click.option(
-    "--infer-self-tito",
+    "--infer-self-tito/--no-infer-self-tito",
     is_flag=True,
-    default=False,
+    default=True,
     help="Infer taint propagations (tito) from arguments to `self` for all methods.",
 )
 @click.option(
-    "--infer-argument-tito",
+    "--infer-argument-tito/--no-infer-argument-tito",
     is_flag=True,
     default=False,
     help="Infer taint propagations (tito) from arguments to other arguments.",

documentation/website/docs/pysa_advanced.md

@@ -212,8 +212,8 @@ def MyClass.updates_foo(self, x: TaintInTaintOut[Updates[self], UpdatePath[_.foo
 
 ## Taint propagation from arguments to self
 
-By default, Pysa only infers taint propagation from arguments to self for
-constructors, property setters and the special `__setitem__` method.
+By default, Pysa infers taint propagation from arguments to self for all methods,
+including constructors, property setters and the special `__setitem__` method.
 
 For instance:
 ```python
@@ -226,22 +226,26 @@ class Foo:
 
 def issue():
   foo = Foo(source())
-  sink(foo)  # Issue found.
+  sink(foo)  # Issue (1) found.
 
   foo = Foo("")
   foo.set_x(source())
-  sink(foo)  # Issue NOT found.
+  sink(foo)  # Issue (2) found.
 ```
 
-To enable the inference of propagations from arguments to self for all methods,
-one can provide the command line argument `--infer-self-tito` or use the taint
-annotation `@InferSelfTito` in a `.pysa` file:
+To disable the inference of propagations from arguments to self for all methods - except
+constructors, property setters and `__setitem__` - one can provide the command
+line argument `--no-infer-self-tito`. This should reduce the analysis time, as well
+as reduce false positives. The counterpart is that it can also lead to false negatives.
+For instance, Pysa wouldn't find issue (2) anymore.
+
+When using `--no-infer-self-tito`, we can use the taint annotation `@InferSelfTito`
+in a `.pysa` file to enable inference for specific methods:
 ```python
 @InferSelfTito
 def my_module.Foo.set_x(): ...
 ```
-Pysa would now find the second issue properly. Note that `--infer-self-tito` can
-significantly increase the analysis time as well as the amount of false positives.
+Pysa would now find the second issue properly.
 
 ## Taint propagation between arguments
 

source/command/analyzeCommand.ml

@@ -92,7 +92,7 @@ module AnalyzeConfiguration = struct
           let maximum_parameterized_targets_at_call_site =
             optional_int_member "maximum_parameterized_targets_at_call_site" json
           in
-          let infer_self_tito = bool_member "infer_self_tito" ~default:false json in
+          let infer_self_tito = bool_member "infer_self_tito" ~default:true json in
           let infer_argument_tito = bool_member "infer_argument_tito" ~default:false json in
           let maximum_model_source_tree_width =
             optional_int_member "maximum_model_source_tree_width" json

source/command/test/analyzeTest.ml

@@ -30,7 +30,7 @@ let test_json_parsing context =
       dump_call_graph = None;
       dump_model_query_results = None;
       find_missing_flows = None;
-      infer_self_tito = false;
+      infer_self_tito = true;
       infer_argument_tito = false;
       maximum_model_source_tree_width = None;
       maximum_model_sink_tree_width = None;
@@ -92,8 +92,8 @@ let test_json_parsing context =
         find_missing_flows = Some Configuration.MissingFlowKind.Obscure;
       };
   assert_parsed
-    (`Assoc (("infer_self_tito", `Bool true) :: BaseConfigurationTest.dummy_base_json))
-    ~expected:{ dummy_analyze_configuration with infer_self_tito = true };
+    (`Assoc (("infer_self_tito", `Bool false) :: BaseConfigurationTest.dummy_base_json))
+    ~expected:{ dummy_analyze_configuration with infer_self_tito = false };
   assert_parsed
     (`Assoc (("infer_argument_tito", `Bool true) :: BaseConfigurationTest.dummy_base_json))
     ~expected:{ dummy_analyze_configuration with infer_argument_tito = true };

source/configuration.ml

@@ -834,7 +834,7 @@ module StaticAnalysis = struct
       ?dump_model_query_results
       ?(use_cache = false)
       ?(build_cache_only = false)
-      ?(infer_self_tito = false)
+      ?(infer_self_tito = true)
       ?(infer_argument_tito = false)
       ?maximum_model_source_tree_width
       ?maximum_model_sink_tree_width

source/interprocedural_analyses/taint/taintConfiguration.ml

@@ -519,7 +519,7 @@ module Heap = struct
       string_combine_partial_sinks = StringOperationPartialSinks.empty;
       find_missing_flows = None;
       dump_model_query_results_path = None;
-      infer_self_tito = false;
+      infer_self_tito = true;
       infer_argument_tito = false;
       analysis_model_constraints = ModelConstraints.default;
       source_sink_filter = SourceSinkFilter.all;
@@ -697,7 +697,7 @@ module Heap = struct
       string_combine_partial_sinks = StringOperationPartialSinks.empty;
       find_missing_flows = None;
       dump_model_query_results_path = None;
-      infer_self_tito = false;
+      infer_self_tito = true;
       infer_argument_tito = false;
       analysis_model_constraints = ModelConstraints.default;
       source_sink_filter =
@@ -1635,7 +1635,7 @@ let from_json_list source_json_list =
     string_combine_partial_sinks;
     find_missing_flows = None;
     dump_model_query_results_path = None;
-    infer_self_tito = false;
+    infer_self_tito = true;
     infer_argument_tito = false;
     analysis_model_constraints =
       ModelConstraints.override_with
@@ -1979,7 +1979,7 @@ let with_command_line_options
     rules;
     implicit_sources;
     implicit_sinks;
-    infer_self_tito = configuration.infer_self_tito || infer_self_tito;
+    infer_self_tito = configuration.infer_self_tito && infer_self_tito;
     infer_argument_tito = configuration.infer_argument_tito || infer_argument_tito;
     source_sink_filter;
   }

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

@@ -1058,6 +1058,20 @@
       }
     ],
     "tito": [
+      {
+        "port": "formal(feature, position=1)",
+        "taint": [
+          {
+            "kinds": [
+              {
+                "return_paths": { "[feature]": 3 },
+                "kind": "ParameterUpdate[formal(self, position=0)]"
+              }
+            ],
+            "tito": {}
+          }
+        ]
+      },
       {
         "port": "formal(self, position=0)[inner]",
         "taint": [