diff --git a/infer/src/checkers/Trace.ml b/infer/src/checkers/Trace.ml index a32765c51..430c47237 100644 --- a/infer/src/checkers/Trace.ml +++ b/infer/src/checkers/Trace.ml @@ -90,7 +90,7 @@ end the "original" trace element. The list is always non-empty. *) module Expander (TraceElem : TraceElem.S) = struct - let expand elem0 ~elems_passthroughs_of_pname = + let expand elem0 ~elems_passthroughs_of_pname ~filter_passthroughs = let rec expand_ elem (elems_passthroughs_acc, seen_acc) = let elem_site = TraceElem.call_site elem in let elem_kind = TraceElem.kind elem in @@ -105,11 +105,13 @@ module Expander (TraceElem : TraceElem.S) = struct TraceElem.Kind.compare (TraceElem.kind callee_elem) elem_kind = 0 && not (is_recursive callee_elem seen_acc')) elems in + (* arbitrarily pick one elem and explore it further *) match matching_elems with | callee_elem :: _ -> (* TODO: pick the shortest path to a sink here instead (t14242809) *) - (* arbitrarily pick one elem and explore it further *) - expand_ callee_elem ((elem, passthroughs) :: elems_passthroughs_acc, seen_acc') + let filtered_passthroughs = + filter_passthroughs elem_site (TraceElem.call_site callee_elem) passthroughs in + expand_ callee_elem ((elem, filtered_passthroughs) :: elems_passthroughs_acc, seen_acc') | _ -> (elem, Passthrough.Set.empty) :: elems_passthroughs_acc, seen_acc' in fst (expand_ elem0 ([], CallSite.Set.empty)) @@ -215,7 +217,26 @@ module Make (Spec : Spec) = struct pp_passthroughs cur_passthroughs pp_sinks (IList.rev sinks_passthroughs) + type passthrough_kind = + | Source (* passthroughs of a source *) + | Sink (* passthroughs of a sink *) + | Top_level (* passthroughs of a top-level source->sink path *) + let get_reportable_paths ?cur_site t ~trace_of_pname = + + let filter_passthroughs_ passthrough_kind start_site end_site passthroughs = + let line_number call_site = + (CallSite.loc call_site).Location.line in + let start_line = line_number start_site in + let end_line = line_number end_site in + let between_start_and_end passthrough = + let passthrough_line = line_number (Passthrough.site passthrough) in + match passthrough_kind with + | Source -> passthrough_line <= start_line + | Sink -> passthrough_line <= end_line + | Top_level -> passthrough_line >= start_line && passthrough_line <= end_line in + Passthrough.Set.filter between_start_and_end passthroughs in + let expand_path source sink = let sources_of_pname pname = let trace = trace_of_pname pname in @@ -224,15 +245,22 @@ module Make (Spec : Spec) = struct let trace = trace_of_pname pname in Sinks.elements (sinks trace), passthroughs trace in let sources_passthroughs = - SourceExpander.expand source ~elems_passthroughs_of_pname:sources_of_pname in + let filter_passthroughs = filter_passthroughs_ Source in + SourceExpander.expand + source ~elems_passthroughs_of_pname:sources_of_pname ~filter_passthroughs in let sinks_passthroughs = - SinkExpander.expand sink ~elems_passthroughs_of_pname:sinks_of_pname in + let filter_passthroughs = filter_passthroughs_ Sink in + SinkExpander.expand + sink ~elems_passthroughs_of_pname:sinks_of_pname ~filter_passthroughs in sources_passthroughs, sinks_passthroughs in IList.map (fun (source, sink, passthroughs) -> let sources_passthroughs, sinks_passthroughs = expand_path source sink in - passthroughs, sources_passthroughs, sinks_passthroughs) + let filtered_passthroughs = + filter_passthroughs_ + Top_level (Source.call_site source) (Sink.call_site sink) passthroughs in + filtered_passthroughs, sources_passthroughs, sinks_passthroughs) (get_reports ?cur_site t) let to_loc_trace diff --git a/infer/tests/codetoanalyze/java/quandary/Interprocedural.java b/infer/tests/codetoanalyze/java/quandary/Interprocedural.java index 75404b04b..81ea016ce 100644 --- a/infer/tests/codetoanalyze/java/quandary/Interprocedural.java +++ b/infer/tests/codetoanalyze/java/quandary/Interprocedural.java @@ -239,6 +239,59 @@ class Interprocedural { InferTaint.inferSensitiveSink(o.f); } + static Object relevantPassthrough(Object param) { + return param; + } + + static Object irrelevantPassthrough(Object param) { + return param; + } + + // the following tests should show only "relevantPassthrough" in their traces + public static Object irrelevantPassthroughsIntraprocedural(Object param) { + Object irrelevant = irrelevantPassthrough(param); + Object source = InferTaint.inferSecretSource(); + Object relevant = relevantPassthrough(source); + InferTaint.inferSensitiveSink(relevant); + return irrelevantPassthrough(relevant); + } + + public static Object returnSourceIrrelevantPassthrough(Object param) { + Object irrelevant = irrelevantPassthrough(param); + Object source = InferTaint.inferSecretSource(); + return relevantPassthrough(source); + } + + public static Object irrelevantPassthroughsSourceInterprocedural(Object o) { + Object irrelevant = irrelevantPassthrough(o); + Object source = returnSourceIrrelevantPassthrough(irrelevant); + Object relevant = relevantPassthrough(source); + InferTaint.inferSensitiveSink(relevant); + return irrelevantPassthrough(source); + } + + public static Object callSinkIrrelevantPassthrough(Object param) { + Object relevant = relevantPassthrough(param); + InferTaint.inferSensitiveSink(relevant); + Object irrelevant = irrelevantPassthrough(param); + return irrelevant; + } + + public static Object irrelevantPassthroughsSinkInterprocedural(Object o) { + Object source = InferTaint.inferSecretSource(); + Object relevant = relevantPassthrough(source); + callSinkIrrelevantPassthrough(relevant); + return irrelevantPassthrough(relevant); + } + + public static Object irrelevantPassthroughsSourceAndSinkInterprocedural(Object o) { + Object irrelevant = irrelevantPassthrough(o); + Object source = returnSourceIrrelevantPassthrough(irrelevant); + Object relevant = relevantPassthrough(source); + callSinkIrrelevantPassthrough(relevant); + return irrelevantPassthrough(relevant); + } + /** false negatives: an ideal analysis would report on these, but we do not */ // this gets modeled as Object[] params in Java. need to treat the array is tainted if its diff --git a/infer/tests/codetoanalyze/java/quandary/issues.exp b/infer/tests/codetoanalyze/java/quandary/issues.exp index 655fd949d..9dcc01eee 100644 --- a/infer/tests/codetoanalyze/java/quandary/issues.exp +++ b/infer/tests/codetoanalyze/java/quandary/issues.exp @@ -98,6 +98,10 @@ codetoanalyze/java/quandary/Intents.java, void Intents.callAllIntentSinksBad(Int codetoanalyze/java/quandary/Intents.java, void Intents.callAllIntentSinksBad(Intent), 20, QUANDARY_TAINT_ERROR, [return from Intent Intent.parseUri(String,int),call to void Intent.setSelector(Intent)] codetoanalyze/java/quandary/Intents.java, void Intents.callAllIntentSinksBad(Intent), 21, QUANDARY_TAINT_ERROR, [return from String Intent.getStringExtra(String),call to Intent Intent.setType(String)] codetoanalyze/java/quandary/Intents.java, void Intents.callAllIntentSinksBad(Intent), 22, QUANDARY_TAINT_ERROR, [return from String Intent.getStringExtra(String),call to Intent Intent.setTypeAndNormalize(String)] +codetoanalyze/java/quandary/Interprocedural.java, Object Interprocedural.irrelevantPassthroughsIntraprocedural(Object), 4, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),flow through Object Interprocedural.relevantPassthrough(Object),call to void InferTaint.inferSensitiveSink(Object)] +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_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_trackParamsOk(), 1, QUANDARY_TAINT_ERROR, [return from Object InferTaint.inferSecretSource(),return from Object Interprocedural.returnSourceConditional(boolean),call to void InferTaint.inferSensitiveSink(Object)]