Skip to content

Commit

Permalink
Propagate config overrides in backward analysis to inferred sinks
Browse files Browse the repository at this point in the history 
Summary: As titled, we now propagate the config overrides from the callee's sink models to the caller and apply them while adding the new inferred sinks.

Reviewed By: yuhshin-oss

Differential Revision: D61407905

fbshipit-source-id: 577b8aa5324cd569bd1e3b9ddb899be6f0b0bccf
  • Loading branch information
@facebook-github-bot
Anwesh Tuladhar authored and facebook-github-bot committed Aug 19, 2024
1 parent 879e05b commit 2a3fc15
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 102 deletions.
13 changes: 9 additions & 4 deletions source/BackwardTaintTransfer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ void infer_input_taint(
context->new_model.add_inferred_sinks(
std::move(port),
std::move(sinks),
taint_tree.config_overrides(),
widening_features,
context->heuristics);
}
Expand Down Expand Up @@ -545,7 +546,7 @@ void check_call_flows(
BackwardTaintEnvironment* environment,
const std::function<std::optional<Register>(Root)>& get_register,
const DexMethodRef* callee_method_reference,
const TaintAccessPathTree& sinks,
const TaintAccessPathTree& sinks_tree,
const std::vector<std::optional<std::string>>& source_constant_arguments,
const FeatureMayAlwaysSet& extra_features,
const FulfilledPartialKindState& fulfilled_partial_sinks) {
Expand All @@ -555,7 +556,7 @@ void check_call_flows(
"Processing sinks for call to `{}`",
show(callee_method_reference));

for (const auto& [port, sinks] : sinks.elements()) {
for (const auto& [port, sinks] : sinks_tree.elements()) {
auto register_id = get_register(port.root());
if (!register_id) {
continue;
Expand Down Expand Up @@ -592,6 +593,10 @@ void check_call_flows(
new_sinks.add_locally_inferred_features(
locally_inferred_fulfilled_sink_features);

// Apply config overrides
auto new_sink_tree =
TaintTree(new_sinks, sinks_tree.config_overrides(port.root()));

auto memory_locations = aliasing.register_memory_locations(*register_id);
LOG_OR_DUMP(
context,
Expand All @@ -600,11 +605,11 @@ void check_call_flows(
*register_id,
memory_locations,
path_resolved,
new_sinks);
new_sink_tree);
environment->write(
memory_locations,
path_resolved,
std::move(new_sinks),
std::move(new_sink_tree),
UpdateKind::Weak);
}
}
Expand Down
2 changes: 2 additions & 0 deletions source/ForwardTaintTransfer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,8 @@ void apply_generations(
"Processing generations for call to `{}`",
show(callee.method_reference));

// We operate on TaintTrees at the roots() directly, so config overrides are
// implicitly propagated.
for (const auto& [root, generations] : callee.model.generations().roots()) {
switch (root.kind()) {
case Root::Kind::Return: {
Expand Down
4 changes: 4 additions & 0 deletions source/Model.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -893,11 +893,15 @@ void Model::add_sink(
void Model::add_inferred_sinks(
AccessPath port,
Taint sinks,
const TaintTreeConfigurationOverrides& config_overrides,
const FeatureMayAlwaysSet& widening_features,
const Heuristics& heuristics) {
auto sanitized_sinks = apply_source_sink_sanitizers(
SanitizerKind::Sinks, std::move(sinks), port.root());
if (!sanitized_sinks.is_bottom()) {
// Apply config overrides
sinks_.apply_config_overrides(port.root(), config_overrides);

update_taint_tree(
sinks_,
std::move(port),
Expand Down
1 change: 1 addition & 0 deletions source/Model.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ class Model final {
void add_inferred_sinks(
AccessPath port,
Taint sinks,
const TaintTreeConfigurationOverrides& config_overrides,
const FeatureMayAlwaysSet& widening_features,
const Heuristics& heuristics);
const TaintAccessPathTree& sinks() const {
Expand Down
2 changes: 1 addition & 1 deletion source/tests/integration/end-to-end/code/builder_pattern_collapse/Test.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ public void testToSinkFields() {
// .a: Source -> Sink issue
// .b: DifferentSource -> DifferentSink issue
// .c: Source -> Sink issue
// .d: No issue => Currently FP
// .d: No issue
toSinkFields(output);
}
}
162 changes: 65 additions & 97 deletions source/tests/integration/end-to-end/code/builder_pattern_collapse/expected_output.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6242,9 +6242,7 @@
{
"always_features" :
[
"via-cast:Lcom/facebook/marianatrench/integrationtests/Test;",
"via-issue-broadening",
"via-widen-broadening"
"via-cast:Lcom/facebook/marianatrench/integrationtests/Test;"
],
"callee" : "Lcom/facebook/marianatrench/integrationtests/Test;.toSinkFields:(Lcom/facebook/marianatrench/integrationtests/Test;)V",
"position" :
Expand All @@ -6260,7 +6258,7 @@
"call_info" :
{
"call_kind" : "CallSite",
"port" : "Argument(0)",
"port" : "Argument(0).a",
"position" :
{
"end" : 22,
Expand All @@ -6273,10 +6271,35 @@
"kinds" :
[
{
"always_features" :
"distance" : 2,
"kind" : "Sink",
"origins" :
[
"via-widen-broadening"
],
{
"method" : "Lcom/facebook/marianatrench/integrationtests/Origin;.sink:(Ljava/lang/Object;)V",
"port" : "Argument(0)"
}
]
}
]
},
{
"call_info" :
{
"call_kind" : "CallSite",
"port" : "Argument(0).c",
"position" :
{
"end" : 22,
"line" : 309,
"path" : "Test.java",
"start" : 17
},
"resolves_to" : "Lcom/facebook/marianatrench/integrationtests/Test;.toSinkFields:(Lcom/facebook/marianatrench/integrationtests/Test;)V"
},
"kinds" :
[
{
"distance" : 2,
"kind" : "Sink",
"origins" :
Expand Down Expand Up @@ -6328,8 +6351,7 @@
{
"always_features" :
[
"via-cast:Lcom/facebook/marianatrench/integrationtests/Test;",
"via-issue-broadening"
"via-cast:Lcom/facebook/marianatrench/integrationtests/Test;"
]
},
"local_positions" :
Expand Down Expand Up @@ -6377,8 +6399,7 @@
{
"always_features" :
[
"via-cast:Lcom/facebook/marianatrench/integrationtests/Test;",
"via-issue-broadening"
"via-cast:Lcom/facebook/marianatrench/integrationtests/Test;"
]
},
"local_positions" :
Expand All @@ -6395,9 +6416,7 @@
{
"always_features" :
[
"via-cast:Lcom/facebook/marianatrench/integrationtests/Test;",
"via-issue-broadening",
"via-widen-broadening"
"via-cast:Lcom/facebook/marianatrench/integrationtests/Test;"
],
"callee" : "Lcom/facebook/marianatrench/integrationtests/Test;.toSinkFields:(Lcom/facebook/marianatrench/integrationtests/Test;)V",
"position" :
Expand All @@ -6413,7 +6432,7 @@
"call_info" :
{
"call_kind" : "CallSite",
"port" : "Argument(0)",
"port" : "Argument(0).b",
"position" :
{
"end" : 22,
Expand All @@ -6426,10 +6445,6 @@
"kinds" :
[
{
"always_features" :
[
"via-widen-broadening"
],
"distance" : 2,
"kind" : "DifferentSink",
"origins" :
Expand Down Expand Up @@ -6481,8 +6496,7 @@
{
"always_features" :
[
"via-cast:Lcom/facebook/marianatrench/integrationtests/Test;",
"via-issue-broadening"
"via-cast:Lcom/facebook/marianatrench/integrationtests/Test;"
]
},
"local_positions" :
Expand All @@ -6493,55 +6507,6 @@
"start" : 15
}
]
},
{
"call_info" :
{
"call_kind" : "Origin",
"port" : "Return",
"position" :
{
"end" : 41,
"line" : 302,
"path" : "Test.java",
"start" : 27
}
},
"kinds" :
[
{
"callee_interval" :
[
38,
39
],
"kind" : "DifferentSource",
"origins" :
[
{
"method" : "Lcom/facebook/marianatrench/integrationtests/Test;.differentSource:()Ljava/lang/Object;",
"port" : "Return"
}
],
"preserves_type_context" : true
}
],
"local_features" :
{
"always_features" :
[
"via-cast:Lcom/facebook/marianatrench/integrationtests/Test;",
"via-issue-broadening"
]
},
"local_positions" :
[
{
"end" : 44,
"line" : 302,
"start" : 15
}
]
}
]
}
Expand Down Expand Up @@ -6746,7 +6711,11 @@
"sinks" :
[
{
"port" : "Argument(0)",
"config_overrides" :
{
"max_model_width" : 3
},
"port" : "Argument(0).a",
"taint" :
[
{
Expand Down Expand Up @@ -6782,15 +6751,18 @@
],
"preserves_type_context" : false
}
],
"local_features" :
{
"always_features" :
[
"via-widen-broadening"
]
}
},
]
}
]
},
{
"config_overrides" :
{
"max_model_width" : 3
},
"port" : "Argument(0).b",
"taint" :
[
{
"call_info" :
{
Expand Down Expand Up @@ -6824,15 +6796,18 @@
],
"preserves_type_context" : false
}
],
"local_features" :
{
"always_features" :
[
"via-widen-broadening"
]
}
},
]
}
]
},
{
"config_overrides" :
{
"max_model_width" : 3
},
"port" : "Argument(0).c",
"taint" :
[
{
"call_info" :
{
Expand Down Expand Up @@ -6866,14 +6841,7 @@
],
"preserves_type_context" : false
}
],
"local_features" :
{
"always_features" :
[
"via-widen-broadening"
]
}
]
}
]
}
Expand Down

0 comments on commit 2a3fc15

Please sign in to comment.