[quandary] improve taint propagation for unknown calls

Summary:
When the receiver type and return type of an unknown call are the same, propagate taint to both the receiver and the return type.
This does the right thing for common "builder-style" methods that both update and return the receiver.
We already had custom models for a few such methods (e.g., `StringBuilder.append`), but we can remove them now.

Reviewed By: jeremydubreil

Differential Revision: D4490071

fbshipit-source-id: 325ea88

infer/src/quandary/ClangTaintAnalysis.ml

@@ -23,6 +23,6 @@ include
       | QuandarySummary.AccessTree.Clang tree -> tree
       | _ -> assert false
 
-    let handle_unknown_call _ _ =
+    let handle_unknown_call _ _ _ =
       []
   end)

infer/src/quandary/JavaTaintAnalysis.ml

@@ -23,25 +23,36 @@ include
       | QuandarySummary.AccessTree.Java access_tree -> access_tree
       | _ -> assert false
 
-    let handle_unknown_call pname ret_typ_opt =
+    let handle_unknown_call pname ret_typ_opt actuals =
+      let types_match typ class_string = match typ with
+        | Typ.Tptr (Tstruct typename, _) -> String.equal (Typename.name typename) class_string
+        | _ -> false in
       match pname with
       | (Procname.Java java_pname) as pname ->
+          let is_static = Procname.java_is_static pname in
           begin
             match Procname.java_get_class_name java_pname,
                   Procname.java_get_method java_pname,
                   ret_typ_opt with
             | _ when Procname.is_constructor pname ->
                 [TaintSpec.Propagate_to_receiver]
-            | ("java.lang.StringBuffer" | "java.lang.StringBuilder" | "java.util.Formatter"), _,
+            | _, _, (Some Typ.Tvoid | None) when not is_static ->
-              Some _
-              when not (Procname.java_is_static pname) ->
-                [TaintSpec.Propagate_to_receiver; TaintSpec.Propagate_to_return]
-            | _, _, (Some Typ.Tvoid | None) when not (Procname.java_is_static pname) ->
                 (* for instance methods with no return value, propagate the taint to the receiver *)
                 [TaintSpec.Propagate_to_receiver]
-            | _, _, Some _ ->
+            | classname, _, Some (Typ.Tptr _ | Tstruct _) ->
-                [TaintSpec.Propagate_to_return]
+                begin
-            | _, _, None ->
+                  match actuals with
+                  | (_, receiver_typ) :: _
+                    when not is_static && types_match receiver_typ classname ->
+                      (* if the receiver and return type are the same, propagate to both. we're
+                         assuming the call is one of the common "builder-style" methods that both
+                         updates and returns the receiver *)
+                      [TaintSpec.Propagate_to_receiver; TaintSpec.Propagate_to_return]
+                  | _ ->
+                      (* receiver doesn't match return type; just propagate to the return type *)
+                      [TaintSpec.Propagate_to_return]
+                end
+            | _ ->
                 []
           end
       | pname when BuiltinDecl.is_declared pname ->

infer/src/quandary/TaintAnalysis.ml

@@ -399,21 +399,19 @@ module Make (TaintSpecification : TaintSpec.S) = struct
               | TaintSpec.Propagate_to_receiver,
                 (receiver_exp, receiver_typ) :: (_ :: _ as other_actuals),
                 _ ->
-                  let receiver_ap =
+                  begin
                     match AccessPath.of_lhs_exp receiver_exp receiver_typ ~f_resolve_id with
                     | Some ap ->
-                        AccessPath.Exact ap
+                        propagate_to_access_path (AccessPath.Exact ap) other_actuals astate_acc
                     | None ->
-                        failwithf
+                        (* this can happen when (for example) the receiver is a string literal *)
-                          "Receiver for called procedure %a does not have an access path"
+                        astate_acc
-                          Procname.pp
+                  end
-                          callee_pname in
-                  propagate_to_access_path receiver_ap other_actuals astate_acc
               | _ ->
                   astate_acc in
 
             let propagations =
-              TaintSpecification.handle_unknown_call callee_pname (Option.map ~f:snd ret) in
+              TaintSpecification.handle_unknown_call callee_pname (Option.map ~f:snd ret) actuals in
             IList.fold_left handle_unknown_call_ astate propagations in
 
           let analyze_call astate_acc callee_pname =

infer/src/quandary/TaintSpec.ml

@@ -22,7 +22,8 @@ module type S = sig
 
   (** return a summary for handling an unknown call at the given site with the given return type
       and actuals *)
-  val handle_unknown_call : Procname.t -> Typ.t option -> handle_unknown list
+  val handle_unknown_call :
+    Procname.t -> Typ.t option -> (Exp.t * Typ.t) list -> handle_unknown list
 
   val to_summary_access_tree : AccessTree.t -> QuandarySummary.AccessTree.t
 

infer/src/unit/TaintTests.ml

@@ -45,7 +45,7 @@ module MockTaintAnalysis = TaintAnalysis.Make(struct
 
     let of_summary_access_tree _ = assert false
     let to_summary_access_tree _ = assert false
-    let handle_unknown_call _ _ = []
+    let handle_unknown_call _ _ _ = []
   end)
 
 module TestInterpreter = AnalyzerTester.Make