[quandary] fix false positives from procedures that are both sources and sinks

Summary: If a procedure is both a source and a sink for the same value, and it's a sink first, you will get a false positive when applying the summary for the procedure.

Reviewed By: cristianoc

Differential Revision: D4145246

fbshipit-source-id: 97f0022

infer/src/quandary/TaintAnalysis.ml

@@ -172,15 +172,23 @@ module Make (TaintSpec : TaintSpec.S) = struct
       | [] ->
           trace
       | paths ->
-          let caller_pname = Cfg.Procdesc.get_proc_name proc_data.pdesc in
+          let report_error path =
+            let caller_pname = Cfg.Procdesc.get_proc_name proc_data.pdesc in
+            let msg = Localise.to_string Localise.quandary_taint_error in
+            let trace_str = F.asprintf "%a" pp_path_short path in
+            let exn = Exceptions.Checkers (msg, Localise.verbatim_desc trace_str) in
+            Reporting.log_error caller_pname ~loc:(CallSite.loc callee_site) exn in
+
           let reported_sinks =
             IList.map
-              (fun ((_, _, sinks) as path) ->
-                 let msg = Localise.to_string Localise.quandary_taint_error in
-                 let trace_str = F.asprintf "%a" pp_path_short path in
-                 let exn = Exceptions.Checkers (msg, Localise.verbatim_desc trace_str) in
-                 Reporting.log_error caller_pname ~loc:(CallSite.loc callee_site) exn;
-                 fst (IList.hd (IList.rev sinks)))
+              (fun ((_, sources, sinks) as path) ->
+                 let source = fst (IList.hd (IList.rev (sources))) in
+                 let sink = fst (IList.hd (IList.rev sinks)) in
+                 if not (CallSite.equal
+                           (TraceDomain.Source.call_site source)
+                           (TraceDomain.Sink.call_site sink))
+                 then report_error path;
+                 sink)
               paths in
           (* got new source -> sink flow. report it, but don't add the sink to the trace. if we do,
              we will double-report later on. *)

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

@@ -295,4 +295,23 @@ class Interprocedural {
     InferTaint.inferSensitiveSink(notASource);
   }
 
+  static void sourceAndSink(Obj o) {
+    InferTaint.inferSensitiveSink(o.f);
+    o.f = InferTaint.inferSecretSource();
+  }
+
+  static void callSourceAndSinkOk(Obj o) {
+    sourceAndSink(o);
+  }
+
+  static void callSourceAndSinkBad1(Obj o) {
+    sourceAndSink(o);
+    InferTaint.inferSensitiveSink(InferTaint.inferSecretSource());
+  }
+
+  static void callSourceAndSinkBad2(Obj o) {
+    o.f = InferTaint.inferSecretSource();
+    sourceAndSink(o);
+  }
+
 }

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

@@ -103,6 +103,8 @@ Interprocedural.java:228: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object Interp
 Interprocedural.java:239: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 237]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 239]) 
 Interprocedural.java:251: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 251]) -> Other(void Interprocedural.callSinkVariadic(java.lang.Object[]) at [line 251]) 
 Interprocedural.java:262: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 260]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 262]) 
+Interprocedural.java:309: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 309]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 309]) 
+Interprocedural.java:314: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 313]) -> Other(void Interprocedural.sourceAndSink(Interprocedural$Obj) at [line 314]) 
 LoggingPrivateData.java:57: ERROR: QUANDARY_TAINT_ERROR Error: PrivateData(String TelephonyManager.getDeviceId() at [line 40]) -> Logging(int Log.e(String,String) at [line 57]) (via { String String.valueOf(double) at [line 25], String String.valueOf(double) at [line 31], String String.valueOf(double) at [line 34], String String.valueOf(float) at [line 28], String String.valueOf(float) at [line 37] })
 LoggingPrivateData.java:57: ERROR: QUANDARY_TAINT_ERROR Error: PrivateData(String TelephonyManager.getLine1Number() at [line 43]) -> Logging(int Log.e(String,String) at [line 57]) (via { String String.valueOf(double) at [line 25], String String.valueOf(double) at [line 31], String String.valueOf(double) at [line 34], String String.valueOf(float) at [line 28], String String.valueOf(float) at [line 37] })
 LoggingPrivateData.java:57: ERROR: QUANDARY_TAINT_ERROR Error: PrivateData(String TelephonyManager.getSimSerialNumber() at [line 46]) -> Logging(int Log.e(String,String) at [line 57]) (via { String String.valueOf(double) at [line 25], String String.valueOf(double) at [line 31], String String.valueOf(double) at [line 34], String String.valueOf(float) at [line 28], String String.valueOf(float) at [line 37] })