diff --git a/infer/src/quandary/TaintAnalysis.ml b/infer/src/quandary/TaintAnalysis.ml index cdb6a8bde..d267b5a9a 100644 --- a/infer/src/quandary/TaintAnalysis.ml +++ b/infer/src/quandary/TaintAnalysis.ml @@ -146,7 +146,24 @@ module Make (TaintSpec : TaintSpec.S) = struct let id_ap = AccessPath.Exact (AccessPath.of_id ret_id ret_typ) in TaintDomain.add_trace id_ap trace access_tree - let add_sinks sinks actuals ({ Domain.access_tree; id_map; } as astate) proc_data loc = + (* log any reportable source-sink flows in [trace] and remove logged sinks from the trace *) + let report_and_filter_trace trace callee_site (proc_data : formal_map ProcData.t) = + match TraceDomain.get_reportable_exns trace with + | [] -> + trace + | reportable_exns -> + let caller_pname = Cfg.Procdesc.get_proc_name proc_data.pdesc in + let reported_sinks = + IList.map + (fun (_, sink, exn) -> + Reporting.log_error caller_pname ~loc:(CallSite.loc callee_site) exn; + sink) + reportable_exns 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. *) + TraceDomain.filter_sinks trace reported_sinks + + let add_sinks sinks actuals ({ Domain.access_tree; id_map; } as astate) proc_data callee_site = let f_resolve_id = resolve_id id_map in (* add [sink] to the trace associated with the [formal_num]th actual *) let add_sink_to_actual access_tree_acc (sink_param : TraceDomain.Sink.t Sink.parameter) = @@ -166,21 +183,16 @@ module Make (TaintSpec : TaintSpec.S) = struct then AccessPath.Abstracted actual_ap_raw else AccessPath.Exact actual_ap_raw in begin - match access_path_get_node actual_ap access_tree_acc proc_data loc with + match access_path_get_node + actual_ap access_tree_acc proc_data (CallSite.loc callee_site) with | Some (actual_trace, _) -> (* add callee_pname to actual trace as a sink *) - let actual_trace' = TraceDomain.add_sink sink_param.sink actual_trace in - begin - match TraceDomain.get_reportable_exns actual_trace' with - | [] -> - TaintDomain.add_trace actual_ap actual_trace' access_tree_acc - | reportable_exns -> - (* got new source -> sink flow. report it, but don't update the trace. if we - update the trace, we will double-report later on. *) - let pname = Cfg.Procdesc.get_proc_name proc_data.ProcData.pdesc in - IList.iter (Reporting.log_error pname ~loc) reportable_exns; - access_tree_acc - end + let actual_trace' = + report_and_filter_trace + (TraceDomain.add_sink sink_param.sink actual_trace) + callee_site + proc_data in + TaintDomain.add_trace actual_ap actual_trace' access_tree_acc | None -> access_tree_acc end @@ -250,10 +262,8 @@ module Make (TaintSpec : TaintSpec.S) = struct let output_trace = TaintSpec.of_summary_trace in_out_summary.output_trace in let appended_trace = TraceDomain.append in_trace output_trace callee_site in let joined_trace = TraceDomain.join caller_trace appended_trace in - IList.iter - (Reporting.log_error (CallSite.pname callee_site) ~loc:callee_loc) - (TraceDomain.get_reportable_exns joined_trace); - TaintDomain.add_trace caller_ap joined_trace access_tree + let filtered_trace = report_and_filter_trace joined_trace callee_site proc_data in + TaintDomain.add_trace caller_ap filtered_trace access_tree | None -> access_tree in @@ -325,7 +335,7 @@ module Make (TaintSpec : TaintSpec.S) = struct let sinks = TraceDomain.Sink.get call_site actuals in let astate_with_sink = match sinks with | [] -> astate - | sinks -> add_sinks sinks actuals astate proc_data callee_loc in + | sinks -> add_sinks sinks actuals astate proc_data call_site in let source = TraceDomain.Source.get call_site in let astate_with_source = diff --git a/infer/src/quandary/Trace.ml b/infer/src/quandary/Trace.ml index f557b17e2..b201eb855 100644 --- a/infer/src/quandary/Trace.ml +++ b/infer/src/quandary/Trace.ml @@ -46,7 +46,7 @@ module type S = sig val get_reports : t -> (Source.t * Sink.t * Passthroughs.t) list (** get logging-ready exceptions for the reportable source-sink flows in this trace *) - val get_reportable_exns : t -> exn list + val get_reportable_exns : t -> (Source.t * Sink.t * exn) list (** create a trace from a source *) val of_source : Source.t -> t @@ -57,6 +57,9 @@ module type S = sig (** add a sink to the current trace. *) val add_sink : Sink.t -> t -> t + (** remove the given sinks from the current trace *) + val filter_sinks : t -> Sink.t list -> t + (** append the trace for given call site to the current caller trace *) val append : t -> t -> CallSite.t -> t @@ -126,7 +129,8 @@ module Make (Spec : Spec) = struct let get_reportable_exns t = IList.map - (fun (source, sink, passthroughs) -> Spec.get_reportable_exn source sink passthroughs) + (fun (source, sink, passthroughs) -> + source, sink, Spec.get_reportable_exn source sink passthroughs) (get_reports t) let of_source source = @@ -143,6 +147,10 @@ module Make (Spec : Spec) = struct let sinks = Sinks.add sink t.sinks in { t with sinks; } + let filter_sinks t sinks_to_filter = + let sinks = Sinks.diff t.sinks (Sinks.of_list sinks_to_filter) in + { t with sinks; } + (** compute caller_trace + callee_trace *) let append caller_trace callee_trace callee_site = if is_empty callee_trace diff --git a/infer/src/quandary/Trace.mli b/infer/src/quandary/Trace.mli index 9ff5c75fd..51521267b 100644 --- a/infer/src/quandary/Trace.mli +++ b/infer/src/quandary/Trace.mli @@ -46,7 +46,7 @@ module type S = sig val get_reports : t -> (Source.t * Sink.t * Passthroughs.t) list (** get logging-ready exceptions for the reportable source-sink flows in this trace *) - val get_reportable_exns : t -> exn list + val get_reportable_exns : t -> (Source.t * Sink.t * exn) list (** create a trace from a source *) val of_source : Source.t -> t @@ -57,6 +57,9 @@ module type S = sig (** add a sink to the current trace. *) val add_sink : Sink.t -> t -> t + (** remove the given sinks from the current trace *) + val filter_sinks : t -> Sink.t list -> t + (** append the trace for given call site to the current caller trace *) val append : t -> t -> CallSite.t -> t diff --git a/infer/tests/codetoanalyze/java/quandary/Interprocedural.java b/infer/tests/codetoanalyze/java/quandary/Interprocedural.java index afed65ebc..5f626f216 100644 --- a/infer/tests/codetoanalyze/java/quandary/Interprocedural.java +++ b/infer/tests/codetoanalyze/java/quandary/Interprocedural.java @@ -195,6 +195,13 @@ class Interprocedural { getGlobalThenCallSink(); } + // this should report two alarms, not three + public void callSinkNoTripleReportBad() { + Object source = InferTaint.inferSecretSource(); + callSinkParam1(source, null); + callSinkParam2(null, source); + } + /** passthrough tests */ public static void singlePassthroughBad() { diff --git a/infer/tests/codetoanalyze/java/quandary/issues.exp b/infer/tests/codetoanalyze/java/quandary/issues.exp index 36a8b621c..6f88bf328 100644 --- a/infer/tests/codetoanalyze/java/quandary/issues.exp +++ b/infer/tests/codetoanalyze/java/quandary/issues.exp @@ -35,8 +35,6 @@ DynamicDispatch.java:140: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferT DynamicDispatch.java:146: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 144]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 146]) via { Object DynamicDispatch$BadSubtype.propagate(Object) at [line 145] } DynamicDispatch.java:154: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 119]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 154]) via { Object DynamicDispatch$BadSubtype.returnSource() at [line 153] } DynamicDispatch.java:157: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 156]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 124]) via { void DynamicDispatch$BadSubtype.callSink(Object) at [line 157] } -DynamicDispatch.java:159: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 156]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 124]) via { void DynamicDispatch$BadSubtype.callSink(Object) at [line 157], Object DynamicDispatch$BadSubtype.propagate(Object) at [line 159] } -DynamicDispatch.java:160: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 156]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 124]) via { void DynamicDispatch$BadSubtype.callSink(Object) at [line 157], Object DynamicDispatch$BadSubtype.propagate(Object) at [line 159] } DynamicDispatch.java:160: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 156]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 160]) via { void DynamicDispatch$BadSubtype.callSink(Object) at [line 157], Object DynamicDispatch$BadSubtype.propagate(Object) at [line 159] } Exceptions.java:23: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 19]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 23]) via { } Exceptions.java:33: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 30]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 33]) via { } @@ -97,12 +95,14 @@ Interprocedural.java:157: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferT Interprocedural.java:166: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 165]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 161]) via { void Interprocedural.callSinkOnGlobal() at [line 166] } Interprocedural.java:181: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 180]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 161]) via { void Interprocedural.callSinkOnGlobal() at [line 181], void Interprocedural.setGlobal(Object) at [line 180] } Interprocedural.java:195: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 194]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 190]) via { void Interprocedural.getGlobalThenCallSink() at [line 195] } -Interprocedural.java:203: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 201]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 203]) via { Object Interprocedural.id(Object) at [line 202] } -Interprocedural.java:210: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 207]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 210]) via { Object Interprocedural.id(Object) at [line 208], Object Interprocedural.id(Object) at [line 209] } -Interprocedural.java:221: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 216]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 221]) via { Object Interprocedural.returnSourceConditional(boolean) at [line 221] } -Interprocedural.java:232: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 230]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 232]) via { } -Interprocedural.java:244: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 244]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 240]) via { void Interprocedural.callSinkVariadic(java.lang.Object[]) at [line 244] } -Interprocedural.java:255: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 253]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 255]) via { } +Interprocedural.java:201: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 200]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 104]) via { void Interprocedural.callSinkParam1(Object,Object) at [line 201] } +Interprocedural.java:202: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 200]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 116]) via { void Interprocedural.callSinkParam1(Object,Object) at [line 201], void Interprocedural.callSinkParam2(Object,Object) at [line 202] } +Interprocedural.java:210: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 208]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 210]) via { Object Interprocedural.id(Object) at [line 209] } +Interprocedural.java:217: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 214]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 217]) via { Object Interprocedural.id(Object) at [line 215], Object Interprocedural.id(Object) at [line 216] } +Interprocedural.java:228: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 223]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 228]) via { Object Interprocedural.returnSourceConditional(boolean) at [line 228] } +Interprocedural.java:239: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 237]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 239]) via { } +Interprocedural.java:251: ERROR: QUANDARY_TAINT_ERROR Error: Other(Object InferTaint.inferSecretSource() at [line 251]) -> Other(void InferTaint.inferSensitiveSink(Object) at [line 247]) via { 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]) via { } LoggingPrivateData.java:20: ERROR: QUANDARY_TAINT_ERROR Error: PrivateData(String SharedPreferences.getString(String,String) at [line 20]) -> Logging(int Log.e(String,String) at [line 20]) via { } LoggingPrivateData.java:24: ERROR: QUANDARY_TAINT_ERROR Error: PrivateData(String SharedPreferences.getString(String,String) at [line 24]) -> Logging(int Log.e(String,String) at [line 24]) via { } LoggingPrivateData.java:39: ERROR: QUANDARY_TAINT_ERROR Error: PrivateData(String SharedPreferences.getString(String,String) at [line 38]) -> Logging(int Log.w(String,Throwable) at [line 39]) via { }