[taint] Remove taint duplicates

Summary: Sometimes we generate many reports in the same line. Here we add a dedup where we report only one issue per line per taint policy.

Reviewed By: jvillard

Differential Revision: D49832692

fbshipit-source-id: 436766f9c0b4c968a764da404e1733e314878af5

infer/src/pulse/PulseDiagnostic.ml

@@ -150,6 +150,7 @@ type t =
       ; location: Location.t
       ; flow_kind: flow_kind
       ; policy_description: string
+      ; policy_id: int
       ; policy_privacy_effect: string option }
   | UnnecessaryCopy of
       { copied_into: PulseAttribute.CopiedInto.t

infer/src/pulse/PulseDiagnostic.mli

@@ -96,6 +96,7 @@ type t =
       ; location: Location.t
       ; flow_kind: flow_kind
       ; policy_description: string
+      ; policy_id: int
       ; policy_privacy_effect: string option }
   | UnnecessaryCopy of
       { copied_into: PulseAttribute.CopiedInto.t

infer/src/pulse/PulseReport.ml

@@ -284,6 +284,7 @@ let report_errors tenv proc_desc err_log location errors =
 
 
 let report_exec_results tenv proc_desc err_log location results =
+  let results = PulseTaintOperations.dedup_reports results in
   List.filter_map results ~f:(fun exec_result ->
       match PulseResult.to_result exec_result with
       | Ok post ->

infer/src/pulse/PulseTaintOperations.ml

@@ -490,6 +490,42 @@ let has_taint_in_history hist =
   else true
 
 
+module LocationIntSet = PrettyPrintable.MakePPSet (struct
+  type nonrec t = Location.t * int [@@deriving compare]
+
+  let pp = Pp.pair ~fst:Location.pp ~snd:Int.pp
+end)
+
+let dedup_reports results =
+  let find_duplicate_taint loc policy_id (location, taint_policy) =
+    Location.equal location loc && Int.equal policy_id taint_policy
+  in
+  let dedup_result result =
+    match result with
+    | Ok _ | FatalError _ ->
+        result
+    | Recoverable (a, errors) ->
+        let dedup_errors error (acc, taint_found) =
+          match error with
+          | ReportableError {diagnostic= TaintFlow {location; policy_id} as diagnostic} ->
+              let found_duplicate =
+                LocationIntSet.exists (find_duplicate_taint location policy_id) taint_found
+              in
+              L.d_printfln_escaped ~color:Red "Taint error before deduplication %a" Diagnostic.pp
+                diagnostic ;
+              let taint_found = LocationIntSet.add (location, policy_id) taint_found in
+              if found_duplicate then (acc, taint_found) else (error :: acc, taint_found)
+          | _ ->
+              (error :: acc, taint_found)
+        in
+        let new_errors, _ =
+          List.fold_right ~f:dedup_errors ~init:([], LocationIntSet.empty) errors
+        in
+        Recoverable (a, new_errors)
+  in
+  List.map ~f:dedup_result results
+
+
 let check_flows_wrt_sink ?(policy_violations_reported = IntSet.empty) path location
     ~sink:(sink_item, sink_trace) ~source astate =
   let source_value, _ = source in
@@ -528,6 +564,7 @@ let check_flows_wrt_sink ?(policy_violations_reported = IntSet.empty) path locat
                      ; sink= ({sink_item with kinds= [sink_kind]}, sink_trace)
                      ; flow_kind
                      ; policy_description
+                     ; policy_id= violated_policy_id
                      ; policy_privacy_effect } ) )
         in
         PulseResult.list_fold potential_policy_violations ~init:policy_violations_reported

infer/src/pulse/PulseTaintOperations.mli

@@ -63,3 +63,7 @@ val check_flows_wrt_sink :
 val taint_initial : Tenv.t -> ProcAttributes.t -> AbductiveDomain.t -> AbductiveDomain.t
 
 val log_taint_config : unit -> unit [@@warning "-unused-value-declaration"]
+
+val dedup_reports :
+     ('a ExecutionDomain.base_t, AccessResult.error) pulse_result list
+  -> ('a ExecutionDomain.base_t, AccessResult.error) pulse_result list

infer/tests/codetoanalyze/objc/pulse-data-lineage/issues.exp

@@ -2,11 +2,7 @@ codetoanalyze/objc/pulse-data-lineage/DataFlowToSink.m, DataFlowToSink.test_simp
 codetoanalyze/objc/pulse-data-lineage/DataFlowToSink.m, DataFlowToSink.test_simple_inheritance, 2, DATA_FLOW_TO_SINK, no_bucket, ADVICE, [source of the taint here: allocation of type `NSObject` by `alloc` with kind `InitSource` (data flow only),when calling `DataFlowToSink.consume:` here,value passed as argument `#0` to `DataFlowToSink.__infer_taint_sink:` with kind `SimpleSink`], , sink: DataFlowToSink.__infer_taint_sink:, tainted expression: start
 codetoanalyze/objc/pulse-data-lineage/SensitiveDataFlow.m, SensitiveDataFlow.create_then_mutate, 2, SENSITIVE_DATA_FLOW, no_bucket, ADVICE, [in call to `SensitiveDataFlow.create_taint`,source of the taint here: value returned from `SensitiveDataFlow.__infer_taint_source` with kind `SimpleSource`,return from call to `SensitiveDataFlow.create_taint`,value passed as argument `#0` to `SensitiveDataFlow.mutate:` with kind `SensitiveSink` (data flow only)], source: SensitiveDataFlow.__infer_taint_source, tainted expression: SensitiveDataFlow.mutate:()
 codetoanalyze/objc/pulse-data-lineage/SensitiveDataFlow.m, SensitiveDataFlow.test, 2, SENSITIVE_DATA_FLOW, no_bucket, ADVICE, [in call to `SensitiveDataFlow.create_then_mutate`,in call to `SensitiveDataFlow.create_taint`,source of the taint here: value returned from `SensitiveDataFlow.__infer_taint_source` with kind `SimpleSource`,return from call to `SensitiveDataFlow.create_taint`,return from call to `SensitiveDataFlow.create_then_mutate`,when calling `SensitiveDataFlow.mutate_then_consume:` here,value passed as argument `#0` to `SensitiveDataFlow.consume:` with kind `SensitiveSink` (data flow only)], source: SensitiveDataFlow.__infer_taint_source, tainted expression: start
-codetoanalyze/objc/pulse-data-lineage/SensitiveDataFlow.m, SensitiveDataFlow.test, 2, SENSITIVE_DATA_FLOW, no_bucket, ADVICE, [in call to `SensitiveDataFlow.create_then_mutate`,in call to `SensitiveDataFlow.create_taint`,source of the taint here: value returned from `SensitiveDataFlow.__infer_taint_source` with kind `SimpleSource`,return from call to `SensitiveDataFlow.create_taint`,return from call to `SensitiveDataFlow.create_then_mutate`,when calling `SensitiveDataFlow.mutate_then_consume:` here,when calling `SensitiveDataFlow.consume:` here,value passed as argument `#0` to `SensitiveDataFlow.might_be_a_sink:` with kind `SensitiveSink` (data flow only)], source: SensitiveDataFlow.__infer_taint_source, tainted expression: start
-codetoanalyze/objc/pulse-data-lineage/SensitiveDataFlow.m, SensitiveDataFlow.test, 2, SENSITIVE_DATA_FLOW, no_bucket, ADVICE, [in call to `SensitiveDataFlow.create_then_mutate`,in call to `SensitiveDataFlow.create_taint`,source of the taint here: value returned from `SensitiveDataFlow.__infer_taint_source` with kind `SimpleSource`,return from call to `SensitiveDataFlow.create_taint`,return from call to `SensitiveDataFlow.create_then_mutate`,when calling `SensitiveDataFlow.mutate_then_consume:` here,value passed as argument `#0` to `SensitiveDataFlow.mutate:` with kind `SensitiveSink` (data flow only)], source: SensitiveDataFlow.__infer_taint_source, tainted expression: start
-codetoanalyze/objc/pulse-data-lineage/SensitiveDataFlow.m, SensitiveDataFlow.test, 2, SENSITIVE_DATA_FLOW, no_bucket, ADVICE, [in call to `SensitiveDataFlow.create_then_mutate`,in call to `SensitiveDataFlow.create_taint`,source of the taint here: value returned from `SensitiveDataFlow.__infer_taint_source` with kind `SimpleSource`,return from call to `SensitiveDataFlow.create_taint`,return from call to `SensitiveDataFlow.create_then_mutate`,value passed as argument `#0` to `SensitiveDataFlow.mutate_then_consume:` with kind `SensitiveSink` (data flow only)], source: SensitiveDataFlow.__infer_taint_source, tainted expression: start
 codetoanalyze/objc/pulse-data-lineage/SensitiveDataFlow.m, SensitiveDataFlow.test_flow_to_unknown, 2, SENSITIVE_DATA_FLOW, no_bucket, ADVICE, [source of the taint here: value returned from `SensitiveDataFlow.__infer_taint_source` with kind `SimpleSource`,value passed as argument `#0` to `unknown` with kind `SensitiveSink` (data flow only)], source: SensitiveDataFlow.__infer_taint_source, tainted expression: obj
-codetoanalyze/objc/pulse-data-lineage/SensitiveDataFlow.m, SensitiveDataFlow.test_taint_propagation, 2, SENSITIVE_DATA_FLOW, no_bucket, ADVICE, [source of the taint here: value returned from `SensitiveDataFlow.__infer_taint_source` with kind `SimpleSource`,value passed as argument `#0` to `SensitiveDataFlow.propagate_taint:` with kind `SensitiveSink` (data flow only)], source: SensitiveDataFlow.__infer_taint_source, tainted expression: obj
 codetoanalyze/objc/pulse-data-lineage/SensitiveDataFlow.m, SensitiveDataFlow.test_taint_propagation, 2, SENSITIVE_DATA_FLOW, no_bucket, ADVICE, [source of the taint here: value returned from `SensitiveDataFlow.__infer_taint_source` with kind `SimpleSource`,when calling `SensitiveDataFlow.propagate_taint:` here,value passed as argument `#0` to `unknown` with kind `SensitiveSink` (data flow only)], source: SensitiveDataFlow.__infer_taint_source, tainted expression: obj
 codetoanalyze/objc/pulse-data-lineage/SensitiveDataFlow.m, SensitiveDataFlow.test_taint_propagation, 3, SENSITIVE_DATA_FLOW, no_bucket, ADVICE, [source of the taint here: value returned from `SensitiveDataFlow.__infer_taint_source` with kind `SimpleSource`,value passed as argument `#0` to `SensitiveDataFlow.might_be_a_sink:` with kind `SensitiveSink` (data flow only)], source: SensitiveDataFlow.__infer_taint_source, tainted expression: ret
 codetoanalyze/objc/pulse-data-lineage/SensitiveDataFlow.m, SensitiveDataFlow.test_ignored_calls, 2, SENSITIVE_DATA_FLOW, no_bucket, ADVICE, [source of the taint here: value returned from `TaintedObject.__infer_taint_source` with kind `SimpleSource`,value passed as argument `#0` to `SensitiveDataFlow.might_be_a_sink:` with kind `SensitiveSink` (data flow only)], source: TaintedObject.__infer_taint_source, tainted expression: tainted

infer/tests/codetoanalyze/objc/pulse/issues.exp

@@ -182,7 +182,6 @@ codetoanalyze/objc/pulse/taint/basics.m, callTwoKindSinkDirectBad, 2, TAINT_ERRO
 codetoanalyze/objc/pulse/taint/basics.m, callTwoKindSinkOnTwiceTaintedDirectBad, 3, TAINT_ERROR, no_bucket, ERROR, [source of the taint here: value passed as argument `#0` to `InferTaint.taintsArg:` with kind `SecondSimpleSource`,value passed as argument `#0` to `InferTaint.twoKindSink:` with kind `SecondSimpleSink`], source: InferTaint.taintsArg:, sink: InferTaint.twoKindSink:, tainted expression: source
 codetoanalyze/objc/pulse/taint/basics.m, callTwoKindSinkOnTwiceTaintedDirectBad, 3, TAINT_ERROR, no_bucket, ERROR, [source of the taint here: value returned from `InferTaint.source` with kind `SimpleSource`,value passed as argument `#0` to `InferTaint.twoKindSink:` with kind `SecondSimpleSink`], source: InferTaint.source, sink: InferTaint.twoKindSink:, tainted expression: source
 codetoanalyze/objc/pulse/taint/basics.m, callTwoSinksIndirectBad, 2, TAINT_ERROR, no_bucket, ERROR, [source of the taint here: value returned from `InferTaint.source` with kind `SimpleSource`,when calling `InferTaint.twoSinks:` here,value passed as argument `#0` to `InferTaint.sink:` with kind `SimpleSink`], source: InferTaint.source, sink: InferTaint.sink:, tainted expression: source
-codetoanalyze/objc/pulse/taint/basics.m, callTwoSinksIndirectBad, 2, TAINT_ERROR, no_bucket, ERROR, [source of the taint here: value returned from `InferTaint.source` with kind `SimpleSource`,when calling `InferTaint.twoSinks:` here,value passed as argument `#0` to `InferTaint.twoKindSink:` with kind `SecondSimpleSink`], source: InferTaint.source, sink: InferTaint.twoKindSink:, tainted expression: source
 codetoanalyze/objc/pulse/taint/basics.m, taintSourceParameterBad, 0, TAINT_ERROR, no_bucket, ERROR, [source of the taint here: value passed as argument `#0` to `taintSourceParameterBad` with kind `SimpleSource`,value passed as argument `#0` to `InferTaint.sink:` with kind `SimpleSink`], source: taintSourceParameterBad, sink: InferTaint.sink:, tainted expression: source
 codetoanalyze/objc/pulse/taint/basics.m, objc_block_taintSourceParameterBlockBad_1, 1, TAINT_ERROR, no_bucket, ERROR, [source of the taint here: value passed as argument `#0` to a block passed to `InferTaint.callBlockUnknown:` with kind `SimpleSource`,value passed as argument `#0` to `InferTaint.sink:` with kind `SimpleSink`], source: a block passed to InferTaint.callBlockUnknown:, sink: InferTaint.sink:, tainted expression: source
 codetoanalyze/objc/pulse/taint/basics.m, sanitizerThenTaintDirectBad, 4, TAINT_ERROR, no_bucket, ERROR, [source of the taint here: value passed as argument `#0` to `InferTaint.taintsArg:` with kind `SecondSimpleSource`,value passed as argument `#0` to `InferTaint.twoKindSink:` with kind `SecondSimpleSink`], source: InferTaint.taintsArg:, sink: InferTaint.twoKindSink:, tainted expression: obj