Skip to content

Commit

Permalink
Refactor IntentRoutingAnalyzer to use Intent class setters from Const…
Browse files Browse the repository at this point in the history
…ants

Summary: Refactor thats useful to support a wider range of Intent setters. As we can see in T160998608 the Intent setter case misses callsites include "Landroid/content/Intent;.<init>:(Ljava/lang/String;Landroid/net/Uri;Landroid/content/Context;Ljava/lang/Class;)V" which is fixed in this diff by hardcoding the ParameterPosition as well.

Reviewed By: arthaud

Differential Revision: D48313981

fbshipit-source-id: d78c7b59a84534ee2df0bbc033870888a33d87a7
  • Loading branch information
Gerben Janssen van Doorn authored and facebook-github-bot committed Aug 17, 2023
1 parent db577ea commit f5c7c53
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 15 deletions.
13 changes: 13 additions & 0 deletions source/Constants.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,19 @@ get_activity_routing_methods() {
{"Landroid/app/Activity;.startActivity:(Landroid/content/Intent;)V", 1},
};
}

const std::unordered_map<std::string, ParameterPosition>&
get_intent_class_setters() {
static const std::unordered_map<std::string, ParameterPosition> intent_class_setters =
{{"Landroid/content/Intent;.<init>:(Landroid/content/Context;Ljava/lang/Class;)V",
2},
{"Landroid/content/Intent;.<init>:(Ljava/lang/String;Landroid/net/Uri;Landroid/content/Context;Ljava/lang/Class;)V",
4},
{"Landroid/content/Intent;.setClass:(Landroid/content/Context;Ljava/lang/Class;)Landroid/content/Intent;",
2}};
return intent_class_setters;
}

#endif

} // namespace constants
Expand Down
2 changes: 2 additions & 0 deletions source/Constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ std::string_view get_privacy_decision_type();
// needs to be known.
std::unordered_map<std::string, ParameterPosition>
get_activity_routing_methods();
const std::unordered_map<std::string, ParameterPosition>&
get_intent_class_setters();

} // namespace constants
} // namespace marianatrench
23 changes: 12 additions & 11 deletions source/IntentRoutingAnalyzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,22 +110,23 @@ class Transfer final : public InstructionAnalyzerBase<
if (method == nullptr || method->get_class() == nullptr) {
return false;
}
// Handle new Intent(context, C.class) and intent.setClass(context,
// C.class).
if (method->get_class()->get_name()->str() == ANDROID_INTENT_CLASS &&
(method->get_name()->str() == "<init>" ||
method->get_name()->str() == "setClass")) {

const auto& intent_class_setters = constants::get_intent_class_setters();
auto intent_parameter_position = intent_class_setters.find(show(method));
if (intent_parameter_position != intent_class_setters.end()) {
auto class_index = intent_parameter_position->second;

mt_assert(class_index > 0);
mt_assert(!::is_static(method));

const auto dex_arguments = method->get_proto()->get_args();
if (dex_arguments->size() != 2) {
return false;
}
const std::size_t class_index = 1;
if (dex_arguments->at(class_index) != type::java_lang_Class()) {
if (dex_arguments->at(class_index - 1) != type::java_lang_Class()) {
return false;
}
const auto& environment = context->types().const_class_environment(
context->method(), instruction);
auto found = environment.find(instruction->src(class_index + 1));
mt_assert(class_index < instruction->srcs_size());
auto found = environment.find(instruction->src(class_index));
if (found == environment.end()) {
return false;
}
Expand Down
4 changes: 2 additions & 2 deletions source/tests/IntentRoutingAnalyzerTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ TEST_F(IntentRoutingAnalyzerTest, IntentRoutingSetClass) {
)
))",
R"(
(method (public) "Landroid/content/Intent;.setClass:(Landroid/content/Context;Ljava/lang/Class;)V"
(method (public) "Landroid/content/Intent;.setClass:(Landroid/content/Context;Ljava/lang/Class;)Landroid/content/Intent;"
(
(return-void)
)
Expand All @@ -173,7 +173,7 @@ TEST_F(IntentRoutingAnalyzerTest, IntentRoutingSetClass) {
(move-result-pseudo-object v1)
(const-class "LRouteTo;")
(move-result-pseudo-object v2)
(invoke-direct (v0 v1 v2) "Landroid/content/Intent;.setClass:(Landroid/content/Context;Ljava/lang/Class;)V")
(invoke-direct (v0 v1 v2) "Landroid/content/Intent;.setClass:(Landroid/content/Context;Ljava/lang/Class;)Landroid/content/Intent;")
(return-void)
)
))",
Expand Down
4 changes: 2 additions & 2 deletions source/tests/ShimsTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ TEST_F(ShimsTest, TestBuildCrossComponentAnalysisShims) {
)
))",
R"(
(method (public) "Landroid/content/Intent;.setClass:(Landroid/content/Context;Ljava/lang/Class;)V"
(method (public) "Landroid/content/Intent;.setClass:(Landroid/content/Context;Ljava/lang/Class;)Landroid/content/Intent;"
(
(return-void)
)
Expand Down Expand Up @@ -141,7 +141,7 @@ TEST_F(ShimsTest, TestBuildCrossComponentAnalysisShims) {
(move-result-pseudo-object v1)
(const-class "LRouteTo;")
(move-result-pseudo-object v2)
(invoke-direct (v0 v1 v2) "Landroid/content/Intent;.setClass:(Landroid/content/Context;Ljava/lang/Class;)V")
(invoke-direct (v0 v1 v2) "Landroid/content/Intent;.setClass:(Landroid/content/Context;Ljava/lang/Class;)Landroid/content/Intent;")
(return-void)
)
))",
Expand Down

0 comments on commit f5c7c53

Please sign in to comment.