diff --git a/infer/src/checkers/Trace.ml b/infer/src/checkers/Trace.ml index 15509a124..a6babf93b 100644 --- a/infer/src/checkers/Trace.ml +++ b/infer/src/checkers/Trace.ml @@ -76,6 +76,8 @@ module type S = sig val update_sinks : t -> Sinks.t -> t + val get_footprint_indexes : t -> IntSet.t + (** append the trace for given call site to the current caller trace *) val append : t -> t -> CallSite.t -> t @@ -343,6 +345,21 @@ module Make (Spec : Spec) = struct let update_sinks t sinks = { t with sinks } + let get_footprint_index source = + match Source.get_footprint_access_path source with + | Some access_path -> + AccessPath.get_footprint_index access_path + | None -> + None + + let get_footprint_indexes trace = + Sources.fold + (fun source acc -> match get_footprint_index source with + | Some footprint_index -> IntSet.add footprint_index acc + | None -> acc) + (sources trace) + IntSet.empty + (** compute caller_trace + callee_trace *) let append caller_trace callee_trace callee_site = if is_empty callee_trace diff --git a/infer/src/checkers/Trace.mli b/infer/src/checkers/Trace.mli index c331d8be6..e3238b676 100644 --- a/infer/src/checkers/Trace.mli +++ b/infer/src/checkers/Trace.mli @@ -78,6 +78,9 @@ module type S = sig (** replace sinks with new ones *) val update_sinks : t -> Sinks.t -> t + (** get the footprint indexes for all of the sources in the trace *) + val get_footprint_indexes : t -> IntSet.t + (** append the trace for given call site to the current caller trace *) val append : t -> t -> CallSite.t -> t diff --git a/infer/src/checkers/accessPath.ml b/infer/src/checkers/accessPath.ml index 162f51628..14f6c7835 100644 --- a/infer/src/checkers/accessPath.ml +++ b/infer/src/checkers/accessPath.ml @@ -207,6 +207,14 @@ let to_footprint formal_index access_path = let _, base_typ = fst (extract access_path) in with_base (Var.of_formal_index formal_index, base_typ) access_path +let get_footprint_index access_path = + let raw_access_path = extract access_path in + match raw_access_path with + | (Var.LogicalVar id, _), _ when Ident.is_footprint id -> + Some (Ident.get_stamp id) + | _ -> + None + let is_exact = function | Exact _ -> true | Abstracted _ -> false diff --git a/infer/src/checkers/accessPath.mli b/infer/src/checkers/accessPath.mli index b5fdb7025..ad75f1d06 100644 --- a/infer/src/checkers/accessPath.mli +++ b/infer/src/checkers/accessPath.mli @@ -77,6 +77,10 @@ val of_lhs_exp : Exp.t -> Typ.t -> f_resolve_id:(Var.t -> Raw.t option) -> Raw.t (** replace the base var with a footprint variable rooted at formal index [formal_index] *) val to_footprint : int -> t -> t +(** return the formal index associated with the base of this access path if there is one, or None + otherwise *) +val get_footprint_index : t -> int option + (** append new accesses to an existing access path; e.g., `append_access x.f [g, h]` produces `x.f.g.h` *) val append : Raw.t -> access list -> Raw.t diff --git a/infer/src/quandary/TaintAnalysis.ml b/infer/src/quandary/TaintAnalysis.ml index ebad92b50..90ea167a7 100644 --- a/infer/src/quandary/TaintAnalysis.ml +++ b/infer/src/quandary/TaintAnalysis.ml @@ -103,7 +103,7 @@ module Make (TaintSpecification : TaintSpec.S) = struct false (** log any new reportable source-sink flows in [trace] *) - let report_trace trace cur_site (proc_data : extras ProcData.t) = + let report_trace ?(sink_indexes=IntSet.empty) trace cur_site (proc_data : extras ProcData.t) = let get_summary pname = if Typ.Procname.equal pname (Procdesc.get_proc_name proc_data.pdesc) then @@ -148,14 +148,13 @@ module Make (TaintSpecification : TaintSpec.S) = struct expand_source matching_source (choice :: report_acc, seen_acc') | [] -> acc in - let rec expand_sink sink0 ((report_acc, seen_acc) as acc) = + let rec expand_sink sink0 indexes0 ((report_acc, seen_acc) as acc) = let kind = Sink.kind sink0 in let call_site = Sink.call_site sink0 in let seen_acc' = CallSite.Set.add call_site seen_acc in let is_recursive sink = CallSite.Set.mem (Sink.call_site sink) seen_acc' in let matching_sinks = - (* TODO: use index info *) TaintDomain.trace_fold (fun acc _ trace -> match List.find @@ -164,17 +163,35 @@ module Make (TaintSpecification : TaintSpec.S) = struct kind (Sink.kind sink) && not (is_recursive sink)) (Sinks.elements (sinks trace)) with - | Some matching_sink -> matching_sink :: acc - | None -> acc) + | Some matching_sink -> + let indexes_match = not + (IntSet.is_empty (IntSet.inter indexes0 (get_footprint_indexes trace))) in + (matching_sink, indexes_match) :: acc + | None -> + acc) (get_summary (CallSite.pname call_site)) [] in - match matching_sinks with - | matching_sink :: _ -> - expand_sink matching_sink (matching_sink :: report_acc, seen_acc') - | [] -> - acc in + try + (* try to find a sink whose indexes match the current sink *) + let matching_sink, _ = List.find_exn ~f:snd matching_sinks in + expand_sink + matching_sink (Sink.indexes matching_sink) (matching_sink :: report_acc, seen_acc') + with Not_found -> + (* didn't find a sink whose indexes match; this can happen when taint flows in via a + global. pick any sink whose kind matches *) + begin + match matching_sinks with + | (matching_sink, _) :: _ -> + expand_sink + matching_sink + (Sink.indexes matching_sink) + (matching_sink :: report_acc, seen_acc') + | [] -> + acc + end in let expanded_sources, _ = expand_source source ([None, source], CallSite.Set.empty) in - let expanded_sinks, _ = expand_sink sink ([sink], CallSite.Set.empty) in + let expanded_sinks, _ = + expand_sink sink sink_indexes ([sink], CallSite.Set.empty) in let source_trace = let pp_access_path_opt fmt = function | None -> F.fprintf fmt "" @@ -223,7 +240,10 @@ module Make (TaintSpecification : TaintSpec.S) = struct begin match access_path_get_node actual_ap access_tree_acc proc_data with | Some (actual_trace, _) -> - let actual_trace' = TraceDomain.add_sink sink actual_trace in + let sink' = + let indexes = TraceDomain.get_footprint_indexes actual_trace in + TraceDomain.Sink.make ~indexes (TraceDomain.Sink.kind sink) callee_site in + let actual_trace' = TraceDomain.add_sink sink' actual_trace in report_trace actual_trace' callee_site proc_data; TaintDomain.add_trace actual_ap actual_trace' access_tree_acc | None -> @@ -299,8 +319,9 @@ module Make (TaintSpecification : TaintSpec.S) = struct let instantiate_and_report callee_trace caller_trace access_tree = let caller_trace' = replace_footprint_sources callee_trace caller_trace access_tree in + let sink_indexes = TraceDomain.get_footprint_indexes callee_trace in let appended_trace = TraceDomain.append caller_trace' callee_trace callee_site in - report_trace appended_trace callee_site proc_data; + report_trace appended_trace callee_site ~sink_indexes proc_data; appended_trace in let add_to_caller_tree access_tree_acc callee_ap callee_trace = diff --git a/infer/tests/codetoanalyze/java/quandary/Traces.java b/infer/tests/codetoanalyze/java/quandary/Traces.java new file mode 100644 index 000000000..ae49bd564 --- /dev/null +++ b/infer/tests/codetoanalyze/java/quandary/Traces.java @@ -0,0 +1,35 @@ +/* + * Copyright (c) 2017 - present Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +package codetoanalyze.java.quandary; + +import com.facebook.infer.builtins.InferTaint; + +class Traces { + void sourceMethod() { + Obj source = (Obj) InferTaint.inferSecretSource(); + callSameSink(null, source, null, null); + } + + void callSameSink(Obj o1, Obj o2, Obj o3, Obj o4) { + callMySink(o1); + callMySinkIndirect(o2); // test that we expand this sink in the trace + callMySink(o3); + callMySink(o4); + } + + void callMySinkIndirect(Obj o) { + callMySink(o); + } + + void callMySink(Obj o) { + InferTaint.inferSensitiveSink(o); + } + +} diff --git a/infer/tests/codetoanalyze/java/quandary/issues.exp b/infer/tests/codetoanalyze/java/quandary/issues.exp index 895267237..b4996deb4 100644 --- a/infer/tests/codetoanalyze/java/quandary/issues.exp +++ b/infer/tests/codetoanalyze/java/quandary/issues.exp @@ -208,6 +208,7 @@ codetoanalyze/java/quandary/TaintedFormals.java, void TaintedFormals.taintedCont 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,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,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/Traces.java, void Traces.sourceMethod(), 2, QUANDARY_TAINT_ERROR, [Return from Object InferTaint.inferSecretSource(),Call to void Traces.callSameSink(Obj,Obj,Obj,Obj),Call to void Traces.callMySinkIndirect(Obj),Call to void Traces.callMySink(Obj),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)]