Skip to content

Commit 5b3099e

Browse files
arthaudfacebook-github-bot
authored andcommitted
Move ViaTypeOf and ViaValueOf features out of the breadcrumb set
Summary: One of the most expensive operations in the analysis is to handle a call site: we need to get the model of the callee and propagate all sources, sinks and tito. During this step, we currently iterate on all breadcrumbs of the callee model in order to find `ViaValueOf` and `ViaTypeOf` features, and instantiate those `via-type:..` breadcrumbs. This can be quite costly since the breadcrumb set can get quite large. To prevent this, this patch moves the `ViaTypeOf` and `ViaValueOf` features into a different set in `FlowDetails`, so that we only iterate on those, and not the whole set of breadcrumbs. This might bring performance benefits. It also makes more sense to use a normal set instead of an over-under set for these, since we do not care whether `ViaXXXOf` is present on all flows or not. Reviewed By: dkgi Differential Revision: D30761555 fbshipit-source-id: 99a56072b8ad65833cc23905267d31338e181625
1 parent ba2d79b commit 5b3099e

File tree

14 files changed

+376
-236
lines changed

14 files changed

+376
-236
lines changed

source/interprocedural_analyses/taint/backwardAnalysis.ml

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ module AnalysisInstance (FunctionContext : FUNCTION_CONTEXT) = struct
313313
BackwardState.Tree.filter_by_kind ~kind:Sinks.AddFeatureToArgument sink_taint
314314
|> BackwardTaint.breadcrumbs
315315
in
316-
if Features.SimpleSet.is_bottom breadcrumbs_to_add then
316+
if Features.BreadcrumbSet.is_bottom breadcrumbs_to_add then
317317
state
318318
else
319319
let taint =
@@ -336,7 +336,7 @@ module AnalysisInstance (FunctionContext : FUNCTION_CONTEXT) = struct
336336
Features.type_breadcrumbs
337337
~resolution:(Resolution.global_resolution resolution)
338338
(Some annotation)
339-
|> Features.SimpleSet.add Features.obscure
339+
|> Features.BreadcrumbSet.add Features.obscure
340340
in
341341
BackwardState.Tree.collapse
342342
~transform:(BackwardTaint.add_breadcrumbs Features.tito_broadening)
@@ -1089,10 +1089,10 @@ module AnalysisInstance (FunctionContext : FUNCTION_CONTEXT) = struct
10891089
let location = Location.with_module ~qualifier:FunctionContext.qualifier location in
10901090
let global_model = Model.get_global_model ~resolution ~expression ~location in
10911091
let add_tito_features taint =
1092-
let attribute_features =
1093-
global_model |> Model.GlobalModel.get_tito |> BackwardState.Tree.features
1092+
let attribute_breadcrumbs =
1093+
global_model |> Model.GlobalModel.get_tito |> BackwardState.Tree.breadcrumbs
10941094
in
1095-
BackwardState.Tree.add_breadcrumbs attribute_features taint
1095+
BackwardState.Tree.add_breadcrumbs attribute_breadcrumbs taint
10961096
in
10971097

10981098
let apply_attribute_sanitizers taint =
@@ -1381,7 +1381,7 @@ let extract_tito_and_sink_models define ~is_constructor ~resolution ~existing_ba
13811381
|> BackwardState.Tree.partition BackwardTaint.kind By ~f:Fn.id
13821382
in
13831383
let taint_in_taint_out =
1384-
let features_to_attach =
1384+
let breadcrumbs_to_attach, via_features_to_attach =
13851385
BackwardState.extract_features_to_attach
13861386
~root:parameter
13871387
~attach_to_kind:Sinks.Attach
@@ -1398,7 +1398,11 @@ let extract_tito_and_sink_models define ~is_constructor ~resolution ~existing_ba
13981398
BackwardState.Tree.prune_maximum_length maximum_tito_depth candidate_tree
13991399
| _ -> candidate_tree
14001400
in
1401-
let candidate_tree = BackwardState.Tree.add_breadcrumbs features_to_attach candidate_tree in
1401+
let candidate_tree =
1402+
candidate_tree
1403+
|> BackwardState.Tree.add_breadcrumbs breadcrumbs_to_attach
1404+
|> BackwardState.Tree.add_via_features via_features_to_attach
1405+
in
14021406
let number_of_paths =
14031407
BackwardState.Tree.fold
14041408
BackwardState.Tree.Path
@@ -1434,13 +1438,15 @@ let extract_tito_and_sink_models define ~is_constructor ~resolution ~existing_ba
14341438
Map.Poly.fold ~init:BackwardState.Tree.empty ~f:simplify_sink_taint partition
14351439
in
14361440
let sink_taint =
1437-
let features_to_attach =
1441+
let breadcrumbs_to_attach, via_features_to_attach =
14381442
BackwardState.extract_features_to_attach
14391443
~root:parameter
14401444
~attach_to_kind:Sinks.Attach
14411445
existing_backward.TaintResult.Backward.sink_taint
14421446
in
1443-
BackwardState.Tree.add_breadcrumbs features_to_attach sink_taint
1447+
sink_taint
1448+
|> BackwardState.Tree.add_breadcrumbs breadcrumbs_to_attach
1449+
|> BackwardState.Tree.add_via_features via_features_to_attach
14441450
in
14451451
TaintResult.Backward.
14461452
{

source/interprocedural_analyses/taint/domains.ml

Lines changed: 90 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,8 @@ module FlowDetails = struct
176176
let name = "flow details"
177177

178178
type 'a slot =
179-
| SimpleFeature : Features.SimpleSet.t slot
179+
| Breadcrumb : Features.BreadcrumbSet.t slot
180+
| ViaFeature : Features.ViaFeatureSet.t slot
180181
| ReturnAccessPath : Features.ReturnAccessPathSet.t slot
181182
| TraceLength : TraceLength.t slot
182183
| TitoPosition : Features.TitoPositionSet.t slot
@@ -185,11 +186,12 @@ module FlowDetails = struct
185186
| FirstField : Features.FirstFieldSet.t slot
186187

187188
(* Must be consistent with above variants *)
188-
let slots = 7
189+
let slots = 8
189190

190191
let slot_name (type a) (slot : a slot) =
191192
match slot with
192-
| SimpleFeature -> "SimpleFeature"
193+
| Breadcrumb -> "Breadcrumb"
194+
| ViaFeature -> "ViaFeature"
193195
| ReturnAccessPath -> "ReturnAccessPath"
194196
| TraceLength -> "TraceLength"
195197
| TitoPosition -> "TitoPosition"
@@ -200,7 +202,8 @@ module FlowDetails = struct
200202

201203
let slot_domain (type a) (slot : a slot) =
202204
match slot with
203-
| SimpleFeature -> (module Features.SimpleSet : Abstract.Domain.S with type t = a)
205+
| Breadcrumb -> (module Features.BreadcrumbSet : Abstract.Domain.S with type t = a)
206+
| ViaFeature -> (module Features.ViaFeatureSet : Abstract.Domain.S with type t = a)
204207
| ReturnAccessPath ->
205208
(module Features.ReturnAccessPathSet : Abstract.Domain.S with type t = a)
206209
| TraceLength -> (module TraceLength : Abstract.Domain.S with type t = a)
@@ -216,7 +219,8 @@ module FlowDetails = struct
216219
include Abstract.ProductDomain.Make (Slots)
217220

218221
let initial =
219-
create [Part (Features.SimpleSet.Self, Features.SimpleSet.empty); Part (TraceLength.Self, 0)]
222+
create
223+
[Part (Features.BreadcrumbSet.Self, Features.BreadcrumbSet.empty); Part (TraceLength.Self, 0)]
220224

221225

222226
let strip_tito_positions =
@@ -225,11 +229,9 @@ module FlowDetails = struct
225229

226230
let add_tito_position position = transform Features.TitoPositionSet.Element Add ~f:position
227231

228-
let add_breadcrumb breadcrumb = transform Features.SimpleSet.Element Add ~f:breadcrumb
232+
let add_breadcrumb breadcrumb = transform Features.BreadcrumbSet.Element Add ~f:breadcrumb
229233

230-
let add_breadcrumbs breadcrumbs = transform Features.SimpleSet.Self Add ~f:breadcrumbs
231-
232-
let features = get Slots.SimpleFeature
234+
let add_breadcrumbs breadcrumbs = transform Features.BreadcrumbSet.Self Add ~f:breadcrumbs
233235

234236
let product_pp = pp (* shadow *)
235237

@@ -260,13 +262,13 @@ module type TAINT_DOMAIN = sig
260262

261263
val trace_info : TraceInfo.t Abstract.Domain.part
262264

263-
val add_breadcrumb : Features.Simple.t -> t -> t
265+
val add_breadcrumb : Features.Breadcrumb.t -> t -> t
264266

265-
val add_breadcrumbs : Features.SimpleSet.t -> t -> t
267+
val add_breadcrumbs : Features.BreadcrumbSet.t -> t -> t
266268

267-
val breadcrumbs : t -> Features.SimpleSet.t
269+
val breadcrumbs : t -> Features.BreadcrumbSet.t
268270

269-
val features : t -> Features.SimpleSet.t
271+
val via_features : t -> Features.ViaFeatureSet.t
270272

271273
val transform_on_widening_collapse : t -> t
272274

@@ -364,23 +366,16 @@ end = struct
364366
let kind_json = `String (Kind.show kind) in
365367
let breadcrumbs, leaf_json =
366368
let gather_json { Abstract.OverUnderSetDomain.element; in_under } breadcrumbs =
367-
match element with
368-
| Features.Simple.ViaValueOf _
369-
| ViaTypeOf _ ->
370-
(* The taint analysis creates breadcrumbs for ViaValueOf and ViaTypeOf features
371-
dynamically.*)
372-
breadcrumbs
373-
| Breadcrumb breadcrumb ->
374-
let breadcrumb_json = Features.Breadcrumb.to_json breadcrumb ~on_all_paths:in_under in
375-
breadcrumb_json :: breadcrumbs
369+
let breadcrumb_json = Features.Breadcrumb.to_json element ~on_all_paths:in_under in
370+
breadcrumb_json :: breadcrumbs
376371
in
377372
let gather_return_access_path path leaves =
378373
let path_name = Abstract.TreeDomain.Label.show_path path in
379374
`Assoc ["kind", kind_json; "name", `String path_name; "depth", `Int trace_length]
380375
:: leaves
381376
in
382377
let breadcrumbs =
383-
FlowDetails.fold Features.SimpleSet.ElementAndUnder ~f:gather_json ~init:[] features
378+
FlowDetails.fold Features.BreadcrumbSet.ElementAndUnder ~f:gather_json ~init:[] features
384379
in
385380
let leaves =
386381
FlowDetails.get FlowDetails.Slots.LeafName features
@@ -445,36 +440,39 @@ end = struct
445440
create_json ~trace_info_to_json:(TraceInfo.to_external_json ~filename_lookup)
446441

447442

448-
let add_breadcrumb breadcrumb = transform Features.SimpleSet.Element Add ~f:breadcrumb
443+
let add_breadcrumb breadcrumb = transform Features.BreadcrumbSet.Element Add ~f:breadcrumb
449444

450445
let add_breadcrumbs breadcrumbs taint =
451-
if Features.SimpleSet.is_bottom breadcrumbs || Features.SimpleSet.is_empty breadcrumbs then
446+
if Features.BreadcrumbSet.is_bottom breadcrumbs || Features.BreadcrumbSet.is_empty breadcrumbs
447+
then
452448
taint
453449
else
454-
transform Features.SimpleSet.Self Add ~f:breadcrumbs taint
450+
transform Features.BreadcrumbSet.Self Add ~f:breadcrumbs taint
455451

456452

457-
let features taint =
458-
let gather_features to_add features = Features.SimpleSet.add_set features ~to_add in
459-
fold Features.SimpleSet.Self ~f:gather_features ~init:Features.SimpleSet.bottom taint
453+
let breadcrumbs taint =
454+
let gather_breadcrumbs to_add breadcrumbs =
455+
Features.BreadcrumbSet.add_set breadcrumbs ~to_add
456+
in
457+
fold Features.BreadcrumbSet.Self ~f:gather_breadcrumbs ~init:Features.BreadcrumbSet.bottom taint
460458

461459

462-
let breadcrumbs taint =
460+
let via_features taint =
463461
fold
464-
Features.SimpleSet.Self
465-
~f:Features.gather_breadcrumbs
466-
~init:Features.SimpleSet.bottom
462+
Features.ViaFeatureSet.Self
463+
~f:Features.ViaFeatureSet.join
464+
~init:Features.ViaFeatureSet.bottom
467465
taint
468466

469467

470468
let transform_on_widening_collapse =
471469
(* using an always-feature here would break the widening invariant: a <= a widen b *)
472470
let open Features in
473471
let broadening =
474-
SimpleSet.of_approximation
472+
BreadcrumbSet.of_approximation
475473
[
476-
{ element = Simple.Breadcrumb Breadcrumb.Broadening; in_under = false };
477-
{ element = Simple.Breadcrumb Breadcrumb.IssueBroadening; in_under = false };
474+
{ element = Breadcrumb.Broadening; in_under = false };
475+
{ element = Breadcrumb.IssueBroadening; in_under = false };
478476
]
479477
in
480478
add_breadcrumbs broadening
@@ -491,12 +489,15 @@ end = struct
491489
let apply_call location ~callees ~port ~path ~element:taint =
492490
let apply (trace_info, kind_taint) =
493491
let open TraceInfo in
492+
let apply_flow_details flow_details =
493+
flow_details
494+
|> FlowDetails.transform Features.TitoPositionSet.Self Map ~f:(fun _ ->
495+
Features.TitoPositionSet.bottom)
496+
|> FlowDetails.transform Features.ViaFeatureSet.Self Map ~f:(fun _ ->
497+
Features.ViaFeatureSet.bottom)
498+
in
494499
let kind_taint =
495-
KindTaintDomain.transform
496-
Features.TitoPositionSet.Self
497-
Map
498-
~f:(fun _ -> Features.TitoPositionSet.bottom)
499-
kind_taint
500+
KindTaintDomain.transform FlowDetails.Self Map ~f:apply_flow_details kind_taint
500501
in
501502
match trace_info with
502503
| Origin _
@@ -561,12 +562,14 @@ module MakeTaintTree (Taint : TAINT_DOMAIN) () = struct
561562
let essential_trace_info = function
562563
| _ -> TraceInfo.Declaration { leaf_name_provided = false }
563564
in
564-
let essential_simple_features _ = Features.SimpleSet.bottom in
565+
let essential_breadcrumbs _ = Features.BreadcrumbSet.bottom in
566+
let essential_via_features _ = Features.ViaFeatureSet.bottom in
565567
let essential_tito_positions _ = Features.TitoPositionSet.bottom in
566568
let essential_leaf_names _ = Features.LeafNameSet.bottom in
567569
transform Taint.trace_info Map ~f:essential_trace_info tree
568570
|> transform Features.ReturnAccessPathSet.Self Map ~f:essential_return_access_paths
569-
|> transform Features.SimpleSet.Self Map ~f:essential_simple_features
571+
|> transform Features.BreadcrumbSet.Self Map ~f:essential_breadcrumbs
572+
|> transform Features.ViaFeatureSet.Self Map ~f:essential_via_features
570573
|> transform Features.TitoPositionSet.Self Map ~f:essential_tito_positions
571574
|> transform Features.LeafNameSet.Self Map ~f:essential_leaf_names
572575

@@ -605,29 +608,46 @@ module MakeTaintTree (Taint : TAINT_DOMAIN) () = struct
605608

606609

607610
let breadcrumbs taint_tree =
611+
let gather_breadcrumbs to_add breadcrumbs =
612+
Features.BreadcrumbSet.add_set breadcrumbs ~to_add
613+
in
608614
fold
609-
Features.SimpleSet.Self
610-
~f:Features.gather_breadcrumbs
611-
~init:Features.SimpleSet.bottom
615+
Features.BreadcrumbSet.Self
616+
~f:gather_breadcrumbs
617+
~init:Features.BreadcrumbSet.bottom
612618
taint_tree
613619
end
614620

615621
module MakeTaintEnvironment (Taint : TAINT_DOMAIN) () = struct
616622
module Tree = struct
617623
include MakeTaintTree (Taint) ()
618624

619-
let add_breadcrumb breadcrumb = transform Features.SimpleSet.Element Add ~f:breadcrumb
625+
let add_breadcrumb breadcrumb = transform Features.BreadcrumbSet.Element Add ~f:breadcrumb
620626

621627
let add_breadcrumbs breadcrumbs taint_tree =
622-
if Features.SimpleSet.is_bottom breadcrumbs || Features.SimpleSet.is_empty breadcrumbs then
628+
if Features.BreadcrumbSet.is_bottom breadcrumbs || Features.BreadcrumbSet.is_empty breadcrumbs
629+
then
623630
taint_tree
624631
else
625-
transform Features.SimpleSet.Self Add ~f:breadcrumbs taint_tree
632+
transform Features.BreadcrumbSet.Self Add ~f:breadcrumbs taint_tree
633+
626634

635+
let breadcrumbs taint_tree =
636+
let gather_breadcrumbs to_add breadcrumbs =
637+
Features.BreadcrumbSet.add_set breadcrumbs ~to_add
638+
in
639+
fold
640+
Features.BreadcrumbSet.Self
641+
~f:gather_breadcrumbs
642+
~init:Features.BreadcrumbSet.bottom
643+
taint_tree
627644

628-
let features taint_tree =
629-
let gather_features to_add features = Features.SimpleSet.add_set features ~to_add in
630-
fold Features.SimpleSet.Self ~f:gather_features ~init:Features.SimpleSet.bottom taint_tree
645+
646+
let add_via_features via_features taint_tree =
647+
if Features.ViaFeatureSet.is_bottom via_features then
648+
taint_tree
649+
else
650+
transform Features.ViaFeatureSet.Self Add ~f:via_features taint_tree
631651
end
632652

633653
include
@@ -697,20 +717,30 @@ module MakeTaintEnvironment (Taint : TAINT_DOMAIN) () = struct
697717
698718
let roots environment = fold Key ~f:List.cons ~init:[] environment
699719
700-
let add_breadcrumb breadcrumb = transform Features.SimpleSet.Element Add ~f:breadcrumb
720+
let add_breadcrumb breadcrumb = transform Features.BreadcrumbSet.Element Add ~f:breadcrumb
701721
702722
let add_breadcrumbs breadcrumbs taint_tree =
703-
if Features.SimpleSet.is_bottom breadcrumbs || Features.SimpleSet.is_empty breadcrumbs then
723+
if Features.BreadcrumbSet.is_bottom breadcrumbs || Features.BreadcrumbSet.is_empty breadcrumbs
724+
then
725+
taint_tree
726+
else
727+
transform Features.BreadcrumbSet.Self Add ~f:breadcrumbs taint_tree
728+
729+
730+
let add_via_features via_features taint_tree =
731+
if Features.ViaFeatureSet.is_bottom via_features then
704732
taint_tree
705733
else
706-
transform Features.SimpleSet.Self Add ~f:breadcrumbs taint_tree
734+
transform Features.ViaFeatureSet.Self Add ~f:via_features taint_tree
707735
708736
709737
let extract_features_to_attach ~root ~attach_to_kind taint =
710-
read ~root ~path:[] taint
711-
|> Tree.transform Taint.kind Filter ~f:(Taint.equal_kind attach_to_kind)
712-
|> Tree.collapse ~transform:Fn.id
713-
|> Taint.features
738+
let taint =
739+
read ~root ~path:[] taint
740+
|> Tree.transform Taint.kind Filter ~f:(Taint.equal_kind attach_to_kind)
741+
|> Tree.collapse ~transform:Fn.id
742+
in
743+
Taint.breadcrumbs taint, Taint.via_features taint
714744
end
715745
716746
module ForwardState = MakeTaintEnvironment (ForwardTaint) ()
@@ -728,7 +758,7 @@ let local_return_taint =
728758
Part (BackwardTaint.kind, Sinks.LocalReturn);
729759
Part (TraceLength.Self, 0);
730760
Part (Features.ReturnAccessPathSet.Element, []);
731-
Part (Features.SimpleSet.Self, Features.SimpleSet.empty);
761+
Part (Features.BreadcrumbSet.Self, Features.BreadcrumbSet.empty);
732762
]
733763
734764

0 commit comments

Comments
 (0)