Skip to content

Commit

Permalink
Use a flow join instead of a sequence join for breadcrumbs when apply…
Browse files Browse the repository at this point in the history
…ing tito

Summary:
As pointed out in D33037819 (5480bf6), we should almost always use `joined_breadcrumbs` instead of `accumulated_breadcrumbs`. The first will perform a "flow" join (i.e, may join always = may) between breadcrumbs from different callees, while the second one uses a "sequence" join (i.e, may join always = always).

There are still a few places where we do use `accumulated_breadcrumbs`, i.e when gathering breadcrumbs from `AddFeaturesToArgument` or attribute models. Since those are user-specified, we want a sequence join instead of a flow join.

Reviewed By: dkgi

Differential Revision: D33488879

fbshipit-source-id: 312c8be77312b28345fc9a0fbe5f3fa44d639299
  • Loading branch information
arthaud authored and facebook-github-bot committed Jan 10, 2022
1 parent d3012ee commit 30c2fa4
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 4 deletions.
2 changes: 1 addition & 1 deletion source/interprocedural_analyses/taint/backwardAnalysis.ml
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ module State (FunctionContext : FUNCTION_CONTEXT) = struct
get_taint access_path initial_state |> BackwardState.Tree.join global_sink
in
let convert_tito_path_to_taint ~kind (tito_path, tito_taint) argument_taint =
let breadcrumbs = BackwardTaint.accumulated_breadcrumbs tito_taint in
let breadcrumbs = BackwardTaint.joined_breadcrumbs tito_taint in
let tito_depth =
BackwardTaint.fold TraceLength.Self tito_taint ~f:TraceLength.join ~init:TraceLength.bottom
in
Expand Down
2 changes: 1 addition & 1 deletion source/interprocedural_analyses/taint/callModel.ml
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ let taint_in_taint_out_mapping
if Model.ModeSet.contains Obscure modes then
let breadcrumbs =
TaintInTaintOutMap.get mapping ~kind:Sinks.LocalReturn
>>| BackwardState.Tree.accumulated_breadcrumbs
>>| BackwardState.Tree.joined_breadcrumbs
|> Option.value ~default:Features.BreadcrumbSet.empty
|> Features.BreadcrumbSet.add (Features.obscure_model ())
in
Expand Down
3 changes: 1 addition & 2 deletions source/interprocedural_analyses/taint/forwardAnalysis.ml
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,7 @@ module State (FunctionContext : FUNCTION_CONTEXT) = struct
accumulated_tito
=
let breadcrumbs =
BackwardTaint.accumulated_breadcrumbs tito_taint
|> Features.BreadcrumbSet.add (Features.tito ())
BackwardTaint.joined_breadcrumbs tito_taint |> Features.BreadcrumbSet.add (Features.tito ())
in
let taint_to_propagate =
if collapse_tito then
Expand Down

0 comments on commit 30c2fa4

Please sign in to comment.