[quandary] fix bug in summary application

Summary: In some cases where a function is called directly on a formal (e.g, `def foo(o) { callSomething(o) }`, we were failing to propagate the footprint trace to the caller.

Reviewed By: jeremydubreil

Differential Revision: D4502404

fbshipit-source-id: d4d632f

infer/src/quandary/TaintAnalysis.ml

@@ -225,7 +225,7 @@ module Make (TaintSpecification : TaintSpec.S) = struct
         actuals
         summary
         (astate_in : Domain.astate)
-        proc_data
+        (proc_data : FormalMap.t ProcData.t)
         callee_site =
       let callee_loc = CallSite.loc callee_site in
       let caller_access_tree = astate_in.access_tree in
@@ -264,46 +264,35 @@ module Make (TaintSpecification : TaintSpec.S) = struct
                   None
             end in
 
-      let get_caller_ap_node ap =
+      let get_caller_ap_node ap access_tree =
         match get_caller_ap ap with
         | Some caller_ap ->
             let caller_node_opt =
-              access_path_get_node caller_ap caller_access_tree proc_data callee_loc in
+              access_path_get_node caller_ap access_tree proc_data callee_loc in
             let caller_node = match caller_node_opt with
               | Some caller_node -> caller_node | None -> TaintDomain.empty_node in
             caller_ap, caller_node
         | None ->
             ap, TaintDomain.empty_node in
 
-      let replace_footprint_sources callee_trace caller_trace =
+      let replace_footprint_sources callee_trace caller_trace access_tree =
         let replace_footprint_source source acc =
           match TraceDomain.Source.get_footprint_access_path source with
           | Some footprint_access_path ->
-              let _, (caller_ap_trace, _) = get_caller_ap_node footprint_access_path in
+              let _, (caller_ap_trace, _) = get_caller_ap_node footprint_access_path access_tree in
               TraceDomain.join caller_ap_trace acc
           | None ->
               acc in
         TraceDomain.Sources.fold
           replace_footprint_source (TraceDomain.sources callee_trace) caller_trace in
 
-      let add_to_caller_tree acc callee_ap callee_trace =
+      let add_to_caller_tree access_tree_acc callee_ap callee_trace =
-        let can_assign ap =
+        let caller_ap, (caller_trace, caller_tree) = get_caller_ap_node callee_ap access_tree_acc in
-          let (base, _), accesses = AccessPath.extract ap in
+        let caller_trace' = replace_footprint_sources callee_trace caller_trace access_tree_acc in
-          match base with
-          | Var.LogicalVar id ->
-              (* Java is pass-by-value, so we can't assign directly to a formal *)
-              Ident.is_footprint id && accesses <> []
-          | Var.ProgramVar pvar ->
-              (* can't assign to callee locals in the caller *)
-              Pvar.is_global pvar || Pvar.is_return pvar in
-        let caller_ap, (caller_trace, caller_tree) = get_caller_ap_node callee_ap in
-        let caller_trace' = replace_footprint_sources callee_trace caller_trace in
         let appended_trace =
           TraceDomain.append caller_trace' callee_trace callee_site in
         report_trace appended_trace callee_site proc_data;
-        if can_assign callee_ap
+        TaintDomain.add_node caller_ap (appended_trace, caller_tree) access_tree_acc in
-        then TaintDomain.add_node caller_ap (appended_trace, caller_tree) acc
-        else acc in
 
       let access_tree =
         TaintDomain.fold

infer/tests/codetoanalyze/java/quandary/Interprocedural.java

@@ -326,26 +326,61 @@ class Interprocedural {
     callSinkThenDiverge(InferTaint.inferSecretSource());
   }
 
-  public static void assignSourceToParam(Object o) {
+  public void callSinkOnParam(Object o) {
-    o = InferTaint.inferSecretSource();
+    InferTaint.inferSensitiveSink(o);
   }
 
-  // Java is call-by-value; this is fine
+  public void callSinkIndirectOnParam(Object o) {
-  public static void assignSourceToParamOk() {
+    callSinkOnParam(o);
-    Object o = null;
+  }
-    assignSourceToParam(o);
+
-    InferTaint.inferSensitiveSink(o);
+  public void callDeepSinkBad1() {
+    Object source = InferTaint.inferSecretSource();
+    callSinkIndirectOnParam(source);
+  }
+
+  Obj propagate(Object param) {
+    Obj o = new Obj();
+    o.f = param;
+    return o;
+  }
+
+  public void FN_callSinkDeepBad2() {
+    Obj source = propagate(InferTaint.inferSecretSource());
+    callSink1(source);
+  }
+
+  void callSink1(Obj o) {
+    callSink2(o);
+  }
+
+  void callSink2(Obj o) {
+    InferTaint.inferSensitiveSink(id(o));
   }
 
   public static void swapParams(Object o1, Object o2) {
     o1 = o2;
   }
 
-  public static void swapParamsOk() {
+  public static void assignSourceToParam(Object o) {
+    o = InferTaint.inferSecretSource();
+  }
+
+  // need to understand that Java is call-by-value for this...
+  public static void FP_swapParamsOk() {
     Object notASource = null;
     Object source = InferTaint.inferSecretSource();
     swapParams(notASource, source);
     InferTaint.inferSensitiveSink(notASource);
   }
 
+  // ...and this
+  public static void FP_assignSourceToParamOk() {
+    Object o = null;
+    assignSourceToParam(o);
+    InferTaint.inferSensitiveSink(o);
+  }
+
+
+
 }

infer/tests/codetoanalyze/java/quandary/TaintedFormals.java

@@ -9,6 +9,9 @@
 
 package codetoanalyze.java.quandary;
 
+import android.app.Activity;
+import android.content.Intent;
+
 import com.facebook.infer.builtins.InferTaint;
 
 class Obj {
@@ -17,18 +20,23 @@ class Obj {
 
 public class TaintedFormals {
 
+  public Activity mActivity;
+
   public void callSink(Object formal) {
     InferTaint.inferSensitiveSink(formal);
   }
 
   // taintedFormal1 and taintedFormal2 were are modeled as tainted
-  public void taintedContextBad(String taintedFormal1, Boolean untaintedFormal, Integer taintedFormal2) {
+  public void taintedContextBad(String taintedFormal1,
+                                Intent untaintedFormal,
+                                Integer taintedFormal2) {
     InferTaint.inferSensitiveSink(taintedFormal1); // should report here
     InferTaint.inferSensitiveSink(taintedFormal2); // should report here
     callSink(taintedFormal1); // should report here
     callSink(taintedFormal2); // should report here
 
-    InferTaint.inferSensitiveSink(untaintedFormal); // should not report here
+    // using different sink to avoid confusion with the above
+    mActivity.startService(untaintedFormal); // should not report here
   }
 
   public Object taintedContextBad(String taintedFormal) {
@@ -41,7 +49,7 @@ public class TaintedFormals {
   }
 
   public void callTaintedContextBad2() {
-    taintedContextBad(null, (Boolean) InferTaint.inferSecretSource(), null);
+    taintedContextBad(null, (Intent) InferTaint.inferSecretSource(), null);
   }
 
   public void callTaintedContextOk1() {

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

@@ -100,9 +100,12 @@ codetoanalyze/java/quandary/Interprocedural.java, Object Interprocedural.irrelev
 codetoanalyze/java/quandary/Interprocedural.java, Object Interprocedural.irrelevantPassthroughsSinkInterprocedural(Object), 3, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),flow through Object Interprocedural.relevantPassthrough(Object),call to Object Interprocedural.callSinkIrrelevantPassthrough(Object),flow through Object Interprocedural.relevantPassthrough(Object),call to void InferTaint.inferSensitiveSink(Object)]
 codetoanalyze/java/quandary/Interprocedural.java, Object Interprocedural.irrelevantPassthroughsSourceAndSinkInterprocedural(Object), 4, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),flow through Object Interprocedural.relevantPassthrough(Object),return from Object Interprocedural.returnSourceIrrelevantPassthrough(Object),flow through Object Interprocedural.relevantPassthrough(Object),call to Object Interprocedural.callSinkIrrelevantPassthrough(Object),flow through Object Interprocedural.relevantPassthrough(Object),call to void InferTaint.inferSensitiveSink(Object)]
 codetoanalyze/java/quandary/Interprocedural.java, Object Interprocedural.irrelevantPassthroughsSourceInterprocedural(Object), 4, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),flow through Object Interprocedural.relevantPassthrough(Object),return from Object Interprocedural.returnSourceIrrelevantPassthrough(Object),flow through Object Interprocedural.relevantPassthrough(Object),call to void InferTaint.inferSensitiveSink(Object)]
+codetoanalyze/java/quandary/Interprocedural.java, void Interprocedural.FP_assignSourceToParamOk(), 3, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),return from void Interprocedural.assignSourceToParam(Object),flow through void Interprocedural.assignSourceToParam(Object),call to void InferTaint.inferSensitiveSink(Object)]
 codetoanalyze/java/quandary/Interprocedural.java, void Interprocedural.FP_divergenceInCallee(), 3, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to void InferTaint.inferSensitiveSink(Object)]
 codetoanalyze/java/quandary/Interprocedural.java, void Interprocedural.FP_reassignInCallee(), 4, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to void InferTaint.inferSensitiveSink(Object)]
+codetoanalyze/java/quandary/Interprocedural.java, void Interprocedural.FP_swapParamsOk(), 4, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),flow through void Interprocedural.swapParams(Object,Object),call to void InferTaint.inferSensitiveSink(Object)]
 codetoanalyze/java/quandary/Interprocedural.java, void Interprocedural.FP_trackParamsOk(), 1, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),return from Object Interprocedural.returnSourceConditional(boolean),call to void InferTaint.inferSensitiveSink(Object)]
+codetoanalyze/java/quandary/Interprocedural.java, void Interprocedural.callDeepSinkBad1(), 2, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to void Interprocedural.callSinkIndirectOnParam(Object),call to void Interprocedural.callSinkOnParam(Object),call to void InferTaint.inferSensitiveSink(Object)]
 codetoanalyze/java/quandary/Interprocedural.java, void Interprocedural.callSinkNoTripleReportBad(), 2, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to void Interprocedural.callSinkParam1(Object,Object),call to void InferTaint.inferSensitiveSink(Object)]
 codetoanalyze/java/quandary/Interprocedural.java, void Interprocedural.callSinkNoTripleReportBad(), 3, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to void Interprocedural.callSinkParam2(Object,Object),call to void InferTaint.inferSensitiveSink(Object)]
 codetoanalyze/java/quandary/Interprocedural.java, void Interprocedural.callSinkOnFieldDirectBad(), 2, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to void Interprocedural.callSinkOnFieldDirect(),call to void InferTaint.inferSensitiveSink(Object)]
@@ -114,9 +117,9 @@ codetoanalyze/java/quandary/Interprocedural.java, void Interprocedural.callSinkP
 codetoanalyze/java/quandary/Interprocedural.java, void Interprocedural.callSinkVariadicBad(), 1, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to void Interprocedural.callSinkVariadic(java.lang.Object[]),call to void InferTaint.inferSensitiveSink(Object)]
 codetoanalyze/java/quandary/Interprocedural.java, void Interprocedural.doublePassthroughBad(), 4, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),flow through Object Interprocedural.id(Object),flow through Object Interprocedural.id(Object),call to void InferTaint.inferSensitiveSink(Object)]
 codetoanalyze/java/quandary/Interprocedural.java, void Interprocedural.getGlobalThenCallSinkBad(), 2, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to void Interprocedural.getGlobalThenCallSink(),flow through Object Interprocedural.getGlobal(),call to void InferTaint.inferSensitiveSink(Object)]
-codetoanalyze/java/quandary/Interprocedural.java, void Interprocedural.returnSourceDirectBad(), 1, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),return from Object Interprocedural.returnSourceDirect(),call to void InferTaint.inferSensitiveSink(Object)]
+codetoanalyze/java/quandary/Interprocedural.java, void Interprocedural.returnSourceDirectBad(), 1, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),return from Object Interprocedural.returnSourceDirect(),flow through Object Interprocedural.returnSourceDirect(),call to void InferTaint.inferSensitiveSink(Object)]
-codetoanalyze/java/quandary/Interprocedural.java, void Interprocedural.returnSourceDirectViaVarBad(), 2, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),return from Object Interprocedural.returnSourceDirect(),call to void InferTaint.inferSensitiveSink(Object)]
+codetoanalyze/java/quandary/Interprocedural.java, void Interprocedural.returnSourceDirectViaVarBad(), 2, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),return from Object Interprocedural.returnSourceDirect(),flow through Object Interprocedural.returnSourceDirect(),call to void InferTaint.inferSensitiveSink(Object)]
-codetoanalyze/java/quandary/Interprocedural.java, void Interprocedural.returnSourceIndirectBad(), 1, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),return from Object Interprocedural.returnSourceDirect(),return from Object Interprocedural.returnSourceIndirect(),call to void InferTaint.inferSensitiveSink(Object)]
+codetoanalyze/java/quandary/Interprocedural.java, void Interprocedural.returnSourceIndirectBad(), 1, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),return from Object Interprocedural.returnSourceDirect(),flow through Object Interprocedural.returnSourceDirect(),return from Object Interprocedural.returnSourceIndirect(),flow through Object Interprocedural.returnSourceIndirect(),call to void InferTaint.inferSensitiveSink(Object)]
 codetoanalyze/java/quandary/Interprocedural.java, void Interprocedural.returnSourceViaFieldBad(), 1, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),return from Interprocedural$Obj Interprocedural.returnSourceViaField(),call to void InferTaint.inferSensitiveSink(Object)]
 codetoanalyze/java/quandary/Interprocedural.java, void Interprocedural.returnSourceViaGlobalBad(), 2, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),return from void Interprocedural.returnSourceViaGlobal(),call to void InferTaint.inferSensitiveSink(Object)]
 codetoanalyze/java/quandary/Interprocedural.java, void Interprocedural.returnSourceViaParameter1Bad(Interprocedural$Obj), 2, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),return from void Interprocedural.returnSourceViaParameter1(Interprocedural$Obj),call to void InferTaint.inferSensitiveSink(Object)]
@@ -174,11 +177,11 @@ codetoanalyze/java/quandary/Strings.java, void Strings.viaStringBuilderBad(), 3,
 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/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,Boolean,Integer),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,Boolean,Integer), 1, QUANDARY_TAINT_ERROR, [return from void TaintedFormals.taintedContextBad(String,Boolean,Integer),call to void InferTaint.inferSensitiveSink(Object)]
+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)]
-codetoanalyze/java/quandary/TaintedFormals.java, void TaintedFormals.taintedContextBad(String,Boolean,Integer), 2, QUANDARY_TAINT_ERROR, [return from void TaintedFormals.taintedContextBad(String,Boolean,Integer),call to void InferTaint.inferSensitiveSink(Object)]
+codetoanalyze/java/quandary/TaintedFormals.java, void TaintedFormals.taintedContextBad(String,Intent,Integer), 4, QUANDARY_TAINT_ERROR, [return from void TaintedFormals.taintedContextBad(String,Intent,Integer),call to void InferTaint.inferSensitiveSink(Object)]
-codetoanalyze/java/quandary/TaintedFormals.java, void TaintedFormals.taintedContextBad(String,Boolean,Integer), 3, QUANDARY_TAINT_ERROR, [return from void TaintedFormals.taintedContextBad(String,Boolean,Integer),call to void TaintedFormals.callSink(Object),call to void InferTaint.inferSensitiveSink(Object)]
+codetoanalyze/java/quandary/TaintedFormals.java, void TaintedFormals.taintedContextBad(String,Intent,Integer), 5, QUANDARY_TAINT_ERROR, [return from void TaintedFormals.taintedContextBad(String,Intent,Integer),call to void TaintedFormals.callSink(Object),call to void InferTaint.inferSensitiveSink(Object)]
-codetoanalyze/java/quandary/TaintedFormals.java, void TaintedFormals.taintedContextBad(String,Boolean,Integer), 4, QUANDARY_TAINT_ERROR, [return from void TaintedFormals.taintedContextBad(String,Boolean,Integer),call to void TaintedFormals.callSink(Object),call to void InferTaint.inferSensitiveSink(Object)]
+codetoanalyze/java/quandary/TaintedFormals.java, void TaintedFormals.taintedContextBad(String,Intent,Integer), 6, QUANDARY_TAINT_ERROR, [return from void TaintedFormals.taintedContextBad(String,Intent,Integer),call to void TaintedFormals.callSink(Object),call to void InferTaint.inferSensitiveSink(Object)]
 codetoanalyze/java/quandary/UnknownCode.java, void UnknownCode.callPropagateFootprintBad(), 1, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to void UnknownCode.propagateFootprint(String),call to void InferTaint.inferSensitiveSink(Object)]
 codetoanalyze/java/quandary/UnknownCode.java, void UnknownCode.callUnknownSetterBad(Intent), 4, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to void InferTaint.inferSensitiveSink(Object)]
 codetoanalyze/java/quandary/UnknownCode.java, void UnknownCode.propagateEmptyBad(), 6, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),call to void InferTaint.inferSensitiveSink(Object)]