Skip to content

Commit

Permalink
[taint] Add to the taint attribute that the taint sink may come from …
Browse files Browse the repository at this point in the history
…a field or pointed to by a value

Summary:
When we add a taint sink attribute to a value in the heap, we also add it to all the values the value points to recursively. However, so far, we were not keeping track of this fact. Now, we are able to express in the tainted attribute that it's a sink because of a predecessor in the heap.

Then, we can differentiate between basic and derived sinks in the next diff to avoid using the value history in the derived case, since this causes false negatives.

Reviewed By: geralt-encore

Differential Revision: D50176052

fbshipit-source-id: 659e8483eefa1e8e68bcbb3e0efd97282ed038e8
  • Loading branch information
dulmarod authored and facebook-github-bot committed Oct 12, 2023
1 parent c922ba0 commit 2a5055d
Showing 1 changed file with 18 additions and 5 deletions.
23 changes: 18 additions & 5 deletions infer/src/pulse/PulseTaintOperations.ml
Original file line number Diff line number Diff line change
Expand Up @@ -622,18 +622,30 @@ let taint_sinks path location tainted astate =
let sink_trace = Trace.Immediate {location; history} in
let visited = ref AbstractValue.Set.empty in
let open PulseResult.Let_syntax in
let rec mark_sinked policy_violations_reported v hist astate =
let rec mark_sinked policy_violations_reported ?access ~(sink : TaintItem.t) v hist astate
=
let is_closure = Option.is_some (AddressAttributes.get_closure_proc_name v astate) in
if AbstractValue.Set.mem v !visited || is_closure then
Ok (policy_violations_reported, astate)
else (
visited := AbstractValue.Set.add v !visited ;
let sink_value_tuple =
match access with
| Some (MemoryAccess.FieldAccess fieldname) ->
TaintItem.FieldOf
{name= Fieldname.get_field_name fieldname; value_tuple= sink.value_tuple}
| Some MemoryAccess.Dereference ->
TaintItem.PointedToBy {value_tuple= sink.value_tuple}
| _ ->
sink.value_tuple
in
let new_sink = {sink with value_tuple= sink_value_tuple} in
let astate =
AbductiveDomain.AddressAttributes.add_taint_sink path sink sink_trace v astate
AbductiveDomain.AddressAttributes.add_taint_sink path new_sink sink_trace v astate
in
let res =
check_flows_wrt_sink ~policy_violations_reported path location
~sink:(sink, sink_trace) ~source:(v, hist) astate
~sink:(new_sink, sink_trace) ~source:(v, hist) astate
in
AbductiveDomain.Memory.fold_edges v astate ~init:res
~f:(fun res (access, (v, hist)) ->
Expand All @@ -645,9 +657,10 @@ let taint_sinks path location tainted astate =
res
| _ ->
let* policy_violations_reported, astate = res in
mark_sinked policy_violations_reported v hist astate ) )
mark_sinked policy_violations_reported ~access ~sink:new_sink v hist astate )
)
in
let+ _, astate = mark_sinked IntSet.empty v history astate in
let+ _, astate = mark_sinked IntSet.empty ~sink v history astate in
astate )
in
L.d_with_indent "taint_sinks" ~f:aux
Expand Down

0 comments on commit 2a5055d

Please sign in to comment.