[quandary] optimize handling of unknown code by adding notion of 'taintable types'

Reviewed By: jeremydubreil

Differential Revision: D4846886

fbshipit-source-id: d8364cc

infer/src/checkers/Trace.ml

@@ -72,6 +72,8 @@ module type S = sig
   (** add a sink to the current trace. *)
   val add_sink : Sink.t -> t -> t
 
+  val update_sources : t -> Sources.t -> t
+
   val update_sinks : t -> Sinks.t -> t
 
   (** append the trace for given call site to the current caller trace *)
@@ -339,6 +341,8 @@ module Make (Spec : Spec) = struct
     let sinks = Sinks.add sink t.sinks in
     { t with sinks; }
 
+  let update_sources t sources = { t with sources }
+
   let update_sinks t sinks = { t with sinks }
 
   (** compute caller_trace + callee_trace *)

infer/src/checkers/Trace.mli

@@ -73,6 +73,8 @@ module type S = sig
   (** add a sink to the current trace. *)
   val add_sink : Sink.t -> t -> t
 
+  val update_sources : t -> Sources.t -> t
+
   (** replace sinks with new ones *)
   val update_sinks : t -> Sinks.t -> t
 

infer/src/quandary/ClangTaintAnalysis.ml

@@ -25,4 +25,6 @@ include
 
     let handle_unknown_call _ _ _ _ =
       []
+
+    let is_taintable_type _ = true
   end)

infer/src/quandary/JavaTaintAnalysis.ml

@@ -68,4 +68,19 @@ include
           []
       | pname ->
           failwithf "Non-Java procname %a in Java analysis@." Typ.Procname.pp pname
+
+    let is_taintable_type = function
+      | Typ.Tptr (Tstruct (JavaClass typename), _) | Tstruct (JavaClass typename) ->
+          begin
+            match Mangled.to_string_full typename with
+            | "android.content.Intent"
+            | "android.net.Uri"
+            | "java.lang.String"
+            | "java.net.URI" ->
+                true
+            | _ ->
+                false
+          end
+      | _ ->
+          false
   end)

infer/src/quandary/TaintAnalysis.ml

@@ -367,6 +367,15 @@ module Make (TaintSpecification : TaintSpec.S) = struct
       | Sil.Call (ret, Const (Cfun called_pname), actuals, callee_loc, call_flags) ->
 
           let handle_unknown_call callee_pname astate =
+            let is_variadic = match callee_pname with
+              | Typ.Procname.Java pname ->
+                  begin
+                    match List.rev (Typ.Procname.java_get_parameters pname) with
+                    | (_, "java.lang.Object[]") :: _ -> true
+                    | _ -> false
+                  end
+              | _ -> false in
+            let should_taint_typ typ = is_variadic || TaintSpecification.is_taintable_type typ in
             let exp_join_traces trace_acc (exp, typ) =
               match exp_get_node ~abstracted:true exp typ astate proc_data with
               | Some (trace, _) -> TraceDomain.join trace trace_acc
@@ -375,9 +384,23 @@ module Make (TaintSpecification : TaintSpec.S) = struct
               let initial_trace = access_path_get_trace access_path astate.access_tree proc_data in
               let trace_with_propagation =
                 List.fold ~f:exp_join_traces ~init:initial_trace actuals in
-              let access_tree =
-                TaintDomain.add_trace access_path trace_with_propagation astate.access_tree in
-              { astate with access_tree; } in
+              let filtered_sources =
+                TraceDomain.Sources.filter (fun source ->
+                    match TraceDomain.Source.get_footprint_access_path source with
+                    | Some access_path ->
+                        Option.exists
+                          (AccessPath.Raw.get_typ (AccessPath.extract access_path) proc_data.tenv)
+                          ~f:should_taint_typ
+                    | None -> true)
+                  (TraceDomain.sources trace_with_propagation) in
+              if TraceDomain.Sources.is_empty filtered_sources
+              then
+                astate
+              else
+                let trace' = TraceDomain.update_sources trace_with_propagation filtered_sources in
+                let access_tree =
+                  TaintDomain.add_trace access_path trace' astate.access_tree in
+                { astate with Domain.access_tree; } in
             let handle_unknown_call_ astate_acc propagation =
               match propagation, actuals, ret with
               | _, [], _ ->

infer/src/quandary/TaintSpec.ml

@@ -25,6 +25,9 @@ module type S = sig
   val handle_unknown_call :
     Typ.Procname.t -> Typ.t option -> (Exp.t * Typ.t) list -> Tenv.t -> handle_unknown list
 
+  (** return true if the given typ can be tainted *)
+  val is_taintable_type : Typ.t -> bool
+
   val to_summary_access_tree : AccessTree.t -> QuandarySummary.AccessTree.t
 
   val of_summary_access_tree : QuandarySummary.AccessTree.t -> AccessTree.t

infer/src/unit/TaintTests.ml

@@ -46,6 +46,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 is_taintable_type _ = true
   end)
 
 module TestInterpreter = AnalyzerTester.Make (ProcCfg.Normal) (MockTaintAnalysis.TransferFunctions)

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

@@ -63,4 +63,19 @@ public class Strings {
     InferTaint.inferSensitiveSink(formatter.toString());
   }
 
+  void viaStringFormatVarArgsDirectBad() {
+    Object source = InferTaint.inferSecretSource();
+    String tainted = String.format("%s%s", "hi", source);
+    InferTaint.inferSensitiveSink(tainted);
+  }
+
+  void viaStringFormatVarArgsIndirect(Object param) {
+    String tainted = String.format("%s%s", "hi", param);
+    InferTaint.inferSensitiveSink(tainted);
+  }
+
+  void FN_viaStringFormatVarArgsIndirectBad() {
+    viaStringFormatVarArgsIndirect(InferTaint.inferSecretSource());
+  }
+
 }

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

@@ -171,6 +171,7 @@ codetoanalyze/java/quandary/Strings.java, void Strings.viaStringBufferIgnoreRetu
 codetoanalyze/java/quandary/Strings.java, void Strings.viaStringBuilderBad(), 3, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to void InferTaint.inferSensitiveSink(Object)]
 codetoanalyze/java/quandary/Strings.java, void Strings.viaStringBuilderIgnoreReturnBad(), 5, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to void InferTaint.inferSensitiveSink(Object)]
 codetoanalyze/java/quandary/Strings.java, void Strings.viaStringBuilderSugarBad(), 2, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to void InferTaint.inferSensitiveSink(Object)]
+codetoanalyze/java/quandary/Strings.java, void Strings.viaStringFormatVarArgsDirectBad(), 3, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to void InferTaint.inferSensitiveSink(Object)]
 codetoanalyze/java/quandary/TaintedFormals.java, void TaintedFormals.callTaintedContextBad1(String), 2, QUANDARY_TAINT_ERROR, [return from Object TaintedFormals.taintedContextBad(String),return from Object TaintedFormals.taintedContextBad(String),call to void InferTaint.inferSensitiveSink(Object)]
 codetoanalyze/java/quandary/TaintedFormals.java, void TaintedFormals.callTaintedContextBad2(), 1, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to void TaintedFormals.taintedContextBad(String,Intent,Integer),call to ComponentName ContextWrapper.startService(Intent)]
 codetoanalyze/java/quandary/TaintedFormals.java, void TaintedFormals.taintedContextBad(String,Intent,Integer), 3, QUANDARY_TAINT_ERROR, [return from void TaintedFormals.taintedContextBad(String,Intent,Integer),call to void InferTaint.inferSensitiveSink(Object)]