[quandary] propagating taint from unknown procedures and constructors

Summary:
Right now, taint gets lost if it flows into a constructor or procedure whose implementation is missing.
Since the core Java (e.g., String) and Android classes (e.g, Intent) are among these, this is bad.
We could handle this by writing a bunch of models instead, but that would be a lot of work (plus we may still miss cases).

Reviewed By: jvillard

Differential Revision: D4051591

fbshipit-source-id: 65851c8

infer/src/quandary/CppTaintAnalysis.ml

@@ -22,6 +22,6 @@ include
       | QuandarySummary.Cpp trace -> trace
       | _ -> assert false
 
-    let handle_unknown_call _ _ =
+    let handle_unknown_call _ _ _ =
       []
   end)

infer/src/quandary/JavaTaintAnalysis.ml

@@ -22,34 +22,70 @@ include
       | QuandarySummary.Java trace -> trace
       | _ -> assert false
 
+    let make_nth_param_ap n pname ~propagate_all =
+      let raw_ap =
+        (* base of this access path is always ignored, so type/name don't matter *)
+        AccessPath.of_pvar
+          (Pvar.mk (Mangled.from_string ("fake_param" ^ string_of_int n)) pname) Typ.Tvoid in
+      if propagate_all then AccessPath.Abstracted raw_ap else AccessPath.Exact raw_ap
+
     (* propagate the trace from the nth parameter of [site.pname] to the return value of
-       [site.pname]. if [propagate_reachable] is true, all traces reachable from the parameter will
+       [site.pname]. if [propagate_all] is true, all traces reachable from the parameter will
        be propagated as well (e.g., for foo(x), we'll also propagate the traces associated with x.f,
        x.f.g, and so on) *)
     let propagate_nth_to_return n site ret_typ ~propagate_all =
       let pname = CallSite.pname site in
-      let dummy_param_ap =
-        let raw_ap =
-          (* base of this access path is always ignored, so type/name don't matter *)
-          AccessPath.of_pvar (Pvar.mk (Mangled.from_string "fake_param") pname) Typ.Tvoid in
-        if propagate_all then AccessPath.Abstracted raw_ap else AccessPath.Exact raw_ap in
-      let input = QuandarySummary.make_formal_input n dummy_param_ap in
+      let nth_param_ap = make_nth_param_ap n pname ~propagate_all in
+      let input = QuandarySummary.make_formal_input n nth_param_ap in
       let output =
         QuandarySummary.make_return_output
           (AccessPath.Exact (AccessPath.of_pvar (Pvar.get_ret_pvar pname) ret_typ)) in
-      let footprint_source = Trace.Source.make_footprint dummy_param_ap site in
+      let footprint_source = Trace.Source.make_footprint nth_param_ap site in
       let footprint_trace = Trace.of_source footprint_source in
       QuandarySummary.make_in_out_summary input output (to_summary_trace footprint_trace)
 
-    let handle_unknown_call site ret_typ_opt =
+    (* propagate the trace associated with each actual to the actual corresponding to the
+       constructed object *)
+    let propagate_to_constructor site actuals ~propagate_all =
+      match actuals with
+      | [] ->
+          failwithf
+            "Constructor %a has 0 actuals, which should never happen"
+            Procname.pp (CallSite.pname site)
+      | _ :: [] ->
+          (* constructor has no actuals, nothing to propagate *)
+          []
+      | _ :: actuals ->
+          let pname = CallSite.pname site in
+          let constructor_ap = make_nth_param_ap 0 pname ~propagate_all in
+          let output = QuandarySummary.make_formal_output 0 constructor_ap in
+          let make_propagation_summary acc n _ =
+            let n = n + 1 in (* skip the constructor actual *)
+            let nth_param_ap = make_nth_param_ap n pname ~propagate_all in
+            let input = QuandarySummary.make_formal_input n nth_param_ap in
+            let footprint_source = Trace.Source.make_footprint nth_param_ap site in
+            let footprint_trace = Trace.of_source footprint_source in
+            let summary =
+              QuandarySummary.make_in_out_summary input output (to_summary_trace footprint_trace) in
+            summary :: acc in
+          IList.fold_lefti make_propagation_summary [] actuals
+
+    let propagate_actuals_to_return site ret_type actuals ~propagate_all =
+      IList.mapi
+        (fun actual_num _-> propagate_nth_to_return actual_num site ret_type ~propagate_all)
+        actuals
+
+    let handle_unknown_call site ret_typ_opt actuals =
       match CallSite.pname site with
-      | Procname.Java pname ->
+      | (Procname.Java java_pname) as pname ->
           begin
-            match Procname.java_get_class_name pname,
-                  Procname.java_get_method pname,
+            match Procname.java_get_class_name java_pname,
+                  Procname.java_get_method java_pname,
                   ret_typ_opt with
-            | "java.lang.String", "valueOf", Some ret_typ ->
-                [propagate_nth_to_return 0 site ret_typ ~propagate_all:true]
+            | _ when Procname.is_constructor pname ->
+                propagate_to_constructor site actuals ~propagate_all:true
+            | _, _, Some ret_typ ->
+                propagate_actuals_to_return site ret_typ actuals ~propagate_all:true
             | _ ->
                 []
           end

infer/src/quandary/TaintAnalysis.ml

@@ -342,7 +342,7 @@ module Make (TaintSpec : TaintSpec.S) = struct
                 let summary =
                   match Summary.read_summary proc_data.tenv proc_data.pdesc callee_pname with
                   | Some summary -> summary
-                  | None -> TaintSpec.handle_unknown_call call_site (Option.map snd ret) in
+                  | None -> TaintSpec.handle_unknown_call call_site (Option.map snd ret) actuals in
                 apply_summary ret actuals summary astate_with_source proc_data call_site in
 
             Domain.join astate_acc astate_with_summary in

infer/src/quandary/TaintSpec.ml

@@ -13,8 +13,9 @@
 module type S = sig
   module Trace : Trace.S
 
-  (** return a summary for handling an unknown call at the given site with the given return type *)
-  val handle_unknown_call : CallSite.t -> Typ.t option -> QuandarySummary.t
+  (** return a summary for handling an unknown call at the given site with the given return type
+      and actuals *)
+  val handle_unknown_call : CallSite.t -> Typ.t option -> (Exp.t * Typ.t) list -> QuandarySummary.t
 
   (** convert a trace type into a summary trace. can be killed if we functorize specs.ml *)
   val to_summary_trace : Trace.t -> QuandarySummary.summary_trace

infer/src/unit/TaintTests.ml

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

infer/tests/codetoanalyze/java/quandary/Makefile

@@ -20,7 +20,7 @@ FILES = \
   Interprocedural.java \
   LoggingPrivateData.java \
   Recursion.java \
-  Strings.java \
+  UnknownCode.java \
 
 compile:
 	javac -cp $(CLASSPATH) $(FILES)

infer/tests/codetoanalyze/java/quandary/Strings.java

@@ -1,35 +0,0 @@
-/*
- * Copyright (c) 2016 - present Facebook, Inc.
- * All rights reserved.
- *
- * This source code is licensed under the BSD style license found in the
- * LICENSE file in the root directory of this source tree. An additional grant
- * of patent rights can be found in the PATENTS file in the same directory.
- */
-
-package codetoanalyze.java.quandary;
-
-import com.facebook.infer.builtins.InferTaint;
-
-/** testing how the analysis handles strings and string manipulation functions */
-
-public class Strings {
-
-  static class Wrapper {
-    Object f;
-  }
-
-  static void valueOfStringBad() {
-    Object source = InferTaint.inferSecretSource();
-    String stringSource = String.valueOf(source);
-    InferTaint.inferSensitiveSink(stringSource);
-  }
-
-  static void valueOfStringWrapperBad() {
-    Wrapper w = new Wrapper();
-    w.f = InferTaint.inferSecretSource();
-    String stringSource = String.valueOf(w.f);
-    InferTaint.inferSensitiveSink(stringSource);
-  }
-
-}

infer/tests/codetoanalyze/java/quandary/UnknownCode.java

@@ -0,0 +1,46 @@
+/*
+ * Copyright (c) 2016 - present Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ */
+
+package codetoanalyze.java.quandary;
+
+import com.facebook.infer.builtins.InferTaint;
+
+/** testing how the analysis handles missing/unknown code */
+
+public class UnknownCode {
+
+  native static Object id(Object o);
+
+  public UnknownCode() {}
+
+  static void propagateViaUnknownCodeBad() {
+    Object source = InferTaint.inferSecretSource();
+    Object launderedSource = id(source);
+    InferTaint.inferSensitiveSink(launderedSource);
+  }
+
+  static void propagateViaUnknownConstructorBad() {
+    String source = (String) InferTaint.inferSecretSource();
+    // we don't analyze the code for the core Java libraries, so this constructor will be unknown
+    String unknownConstructor = new String(source);
+    InferTaint.inferSensitiveSink(unknownConstructor);
+  }
+
+  static void propagateViaUnknownConstructorOk() {
+    String unknownConstructor = new String("");
+    InferTaint.inferSensitiveSink(unknownConstructor);
+  }
+
+  static void propagateViaUnknownCodeOk() {
+    Object notASource = new UnknownCode();
+    Object launderedSource = id(notASource);
+    InferTaint.inferSensitiveSink(launderedSource);
+  }
+
+}

infer/tests/codetoanalyze/java/quandary/issues.exp

@@ -109,5 +109,5 @@ LoggingPrivateData.java:37: ERROR: QUANDARY_TAINT_ERROR Error: SharedPreferences
 Recursion.java:26: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 26]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 21]) via { void Recursion.callSinkThenDiverge(Object) at [line 26] }
 Recursion.java:36: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 36]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 31]) via { void Recursion.safeRecursionCallSink(int,Object) at [line 36] }
 Recursion.java:42: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 42]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 41]) via { void Recursion.recursionBad(int,Object) at [line 42] }
-Strings.java:25: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 23]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 25]) via { String String.valueOf(Object) at [line 24] }
-Strings.java:32: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 30]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 32]) via { String String.valueOf(Object) at [line 31] }
+UnknownCode.java:25: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 23]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 25]) via { Object UnknownCode.id(Object) at [line 24] }
+UnknownCode.java:32: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 29]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 32]) via { String.<init>(String) at [line 31] }