Skip to content

Commit

Permalink
Propagate class intervals
Browse files Browse the repository at this point in the history
Summary:
Handle class intervals during in `propagate()`. Gist of it is:

* At call-site:
  * Look-up the receiver's interval based on its register type.
  * Determine if the receiver is a `this.*` call (preserves type context = true)
* When calling `propagate`:
  * Pass on receiver interval
  * Pass on caller's class interval (i.e. if caller is `C::m()`, use `interval(C)`). Use max interval for static methods. If callee is a `this.*` call, this would be the same as the receiver's class interval.

When propagating, if the next hop (the propagated frame) is a `this.*` call, intersect the next hop's interval with the receiver's interval and use the intersection as the propagated interval.

The frame is dropped if the intersection is empty as it means that the receiver's type is unrelated to the next hop's `this.*` type, i.e. the next hop is not reachable from here.

If preserves_type_context = false, it is not a `this.*` call. We do not have any more information about how reachable the next hop, so taint should be propagated as before.

NOTE: Because we do not yet call `propagate()` on propagations, class interval intersection only happens on source/sink frames at the moment.

This change would address invalid traces of the following form:

```
void Base::toSink(Object argument) {
  // This resolves to Derived*.derivedToSink(), e.g.
  //   interval(DerivedA) -> DerivedA.derivedToSink()
  //   interval(DerivedB) -> DerivedB.derivedToSink()
  //   ...
  this.derivedToSink(argument);
}

void toSink(DerivedA derivedA, Object argument) {
  // If DerivedA does not override toSink(), this resolves to Base.toSink()
  // Without class intervals, the analysis this leads to all Derived*.derivedToSink() sinks.
  // With class intervals, only classes in the hierarchy of DerivedA's
  // will have an interval that intersects with the receiver's type (DerivedA).
  // All other sinks are invalid and will be dropped.
  derivedA.toSink(argument);
}
```

___

Reviewed By: arthaud

Differential Revision: D47763382

fbshipit-source-id: f0827384580dadd831f756121fbf61cc83ac3208
  • Loading branch information
Yuh Shin Ong authored and facebook-github-bot committed Aug 14, 2023
1 parent 651e489 commit b554e07
Show file tree
Hide file tree
Showing 27 changed files with 674 additions and 183 deletions.
3 changes: 2 additions & 1 deletion source/BackwardTaintTransfer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,8 @@ bool BackwardTaintTransfer::analyze_invoke(
instruction,
aliasing.position(),
get_source_register_types(context, instruction),
source_constant_arguments);
source_constant_arguments,
get_is_this_call(aliasing.register_memory_locations_map(), instruction));

TaintTree result_taint = TaintTree::bottom();
if (callee.resolved_base_method &&
Expand Down
9 changes: 6 additions & 3 deletions source/CallPositionFrames.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,9 @@ CallPositionFrames CallPositionFrames::propagate(
int maximum_source_sink_distance,
Context& context,
const std::vector<const DexType * MT_NULLABLE>& source_register_types,
const std::vector<std::optional<std::string>>& source_constant_arguments)
const {
const std::vector<std::optional<std::string>>& source_constant_arguments,
const CalleeInterval& callee_interval,
const ClassIntervals::Interval& caller_class_interval) const {
if (is_bottom()) {
return CallPositionFrames::bottom();
}
Expand All @@ -73,7 +74,9 @@ CallPositionFrames CallPositionFrames::propagate(
maximum_source_sink_distance,
context,
source_register_types,
source_constant_arguments);
source_constant_arguments,
callee_interval,
caller_class_interval);
result.update(
propagated.callee_port(), [&propagated](CalleePortFrames* frames) {
frames->join_with(propagated);
Expand Down
5 changes: 3 additions & 2 deletions source/CallPositionFrames.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,9 @@ class CallPositionFrames final : public FramesMap<
int maximum_source_sink_distance,
Context& context,
const std::vector<const DexType * MT_NULLABLE>& source_register_types,
const std::vector<std::optional<std::string>>& source_constant_arguments)
const;
const std::vector<std::optional<std::string>>& source_constant_arguments,
const CalleeInterval& callee_interval,
const ClassIntervals::Interval& caller_class_interval) const;

/* Return the set of leaf frames with the given position. */
CallPositionFrames attach_position(const Position* position) const;
Expand Down
9 changes: 6 additions & 3 deletions source/CalleeFrames.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,9 @@ CalleeFrames CalleeFrames::propagate(
int maximum_source_sink_distance,
Context& context,
const std::vector<const DexType * MT_NULLABLE>& source_register_types,
const std::vector<std::optional<std::string>>& source_constant_arguments)
const {
const std::vector<std::optional<std::string>>& source_constant_arguments,
const CalleeInterval& callee_interval,
const ClassIntervals::Interval& caller_class_interval) const {
if (is_bottom()) {
return CalleeFrames::bottom();
}
Expand All @@ -84,7 +85,9 @@ CalleeFrames CalleeFrames::propagate(
maximum_source_sink_distance,
context,
source_register_types,
source_constant_arguments));
source_constant_arguments,
callee_interval,
caller_class_interval));
}

if (result.is_bottom()) {
Expand Down
5 changes: 3 additions & 2 deletions source/CalleeFrames.h
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,9 @@ class CalleeFrames final : public FramesMap<
int maximum_source_sink_distance,
Context& context,
const std::vector<const DexType * MT_NULLABLE>& source_register_types,
const std::vector<std::optional<std::string>>& source_constant_arguments)
const;
const std::vector<std::optional<std::string>>& source_constant_arguments,
const CalleeInterval& callee_interval,
const ClassIntervals::Interval& caller_class_interval) const;

/**
* Propagate the taint from the callee to the caller to track the next hops
Expand Down
13 changes: 9 additions & 4 deletions source/CalleePortFrames.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,9 @@ CalleePortFrames CalleePortFrames::propagate(
int maximum_source_sink_distance,
Context& context,
const std::vector<const DexType * MT_NULLABLE>& source_register_types,
const std::vector<std::optional<std::string>>& source_constant_arguments)
const {
const std::vector<std::optional<std::string>>& source_constant_arguments,
const CalleeInterval& callee_interval,
const ClassIntervals::Interval& caller_class_interval) const {
if (is_bottom()) {
return CalleePortFrames::bottom();
}
Expand All @@ -231,7 +232,9 @@ CalleePortFrames CalleePortFrames::propagate(
locally_inferred_features_,
maximum_source_sink_distance,
context,
source_register_types);
source_register_types,
callee_interval,
caller_class_interval);
} else {
std::vector<const Feature*> via_type_of_features_added;
propagated = frames.propagate(
Expand All @@ -243,7 +246,9 @@ CalleePortFrames CalleePortFrames::propagate(
context,
source_register_types,
source_constant_arguments,
via_type_of_features_added);
via_type_of_features_added,
callee_interval,
caller_class_interval);
}

if (!propagated.is_bottom()) {
Expand Down
5 changes: 3 additions & 2 deletions source/CalleePortFrames.h
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,9 @@ class CalleePortFrames final : public sparta::AbstractDomain<CalleePortFrames> {
int maximum_source_sink_distance,
Context& context,
const std::vector<const DexType * MT_NULLABLE>& source_register_types,
const std::vector<std::optional<std::string>>& source_constant_arguments)
const;
const std::vector<std::optional<std::string>>& source_constant_arguments,
const CalleeInterval& callee_interval,
const ClassIntervals::Interval& caller_class_interval) const;

/**
* Propagate the taint from the callee to the caller to track the next hops
Expand Down
4 changes: 2 additions & 2 deletions source/ForwardAliasTransfer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,8 @@ MemoryLocationsDomain invoke_result_memory_location(
instruction,
environment->last_position(),
get_source_register_types(context, instruction),
get_source_constant_arguments(
register_memory_locations_map, instruction));
get_source_constant_arguments(register_memory_locations_map, instruction),
get_is_this_call(register_memory_locations_map, instruction));

if (callee.resolved_base_method &&
callee.resolved_base_method->returns_void()) {
Expand Down
3 changes: 2 additions & 1 deletion source/ForwardTaintTransfer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -817,7 +817,8 @@ bool ForwardTaintTransfer::analyze_invoke(
instruction,
aliasing.position(),
get_source_register_types(context, instruction),
source_constant_arguments);
source_constant_arguments,
get_is_this_call(aliasing.register_memory_locations_map(), instruction));

const ForwardTaintEnvironment previous_environment = *environment;

Expand Down
Loading

0 comments on commit b554e07

Please sign in to comment.