Skip to content

Commit

Permalink
Fail on bad field writes
Browse files Browse the repository at this point in the history 
Summary: Fail on `IPUT/IPUT_OBJECT/SPUT/SPUT_OBJECT` if the value getting written to the field is incorrect. This caused some issues with `CHECK_CAST`, which we need to support anyways. If usedefs resolves to `CHECK_CAST`, keep searching for the actual source.

Reviewed By: thezhangwei

Differential Revision: D59834956

fbshipit-source-id: 28e80987d33e13984901044140d368ea06b4cf43
  • Loading branch information
@itang00 @facebook-github-bot
itang00 authored and facebook-github-bot committed Oct 23, 2024
1 parent 5820a20 commit 0de7597
Show file tree
Hide file tree
Showing 2 changed files with 138 additions and 24 deletions.
62 changes: 56 additions & 6 deletions opt/typedef-anno-checker/TypedefAnnoCheckerPass.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include "AnnoUtils.h"
#include "ClassUtil.h"
#include "KotlinNullCheckMethods.h"
#include "PassManager.h"
#include "Resolver.h"
#include "Show.h"
Expand Down Expand Up @@ -56,6 +57,34 @@ bool is_int_or_obj_ref(const type_inference::TypeEnvironment& env, reg_t reg) {
: true;
}

bool is_null_check(const DexMethod* m) {
for (DexMethodRef* kotlin_null_method :
kotlin_nullcheck_wrapper::get_kotlin_null_assertions()) {
if (kotlin_null_method->get_name() == m->get_name()) {
return true;
}
}
return m->get_deobfuscated_name_or_empty_copy() ==
"Lcom/google/common/base/Preconditions;.checkNotNull:(Ljava/lang/"
"Object;)Ljava/lang/Object;";
}

bool has_typedef_annos(ParamAnnotations* param_annos,
const std::unordered_set<DexType*>& typedef_annos) {
if (!param_annos) {
return false;
}
for (auto& anno : *param_annos) {
auto& anno_set = anno.second;
auto typedef_anno = type_inference::get_typedef_annotation(
anno_set->get_annotations(), typedef_annos);
if (typedef_anno != boost::none) {
return true;
}
}
return false;
}

DexMethod* resolve_method(DexMethod* caller, IRInstruction* insn) {
auto def_method =
resolve_method(insn->get_method(), opcode_to_search(insn), caller);
Expand Down Expand Up @@ -345,13 +374,14 @@ void patch_return_anno_from_get(type_inference::TypeInference& inference,
IRInstruction* insn) {
always_assert(opcode::is_an_iget(insn->opcode()) ||
opcode::is_an_sget(insn->opcode()));
auto name = caller->get_deobfuscated_name_or_empty();
auto pos = name.rfind('$');
if (pos == std::string::npos) {
auto name = caller->get_simple_deobfuscated_name();
auto last_dollar = name.find('$');
if (last_dollar == std::string::npos) {
return;
}
pos++;
if (!(pos < name.size() && name[pos] >= '0' && name[pos] <= '9')) {
last_dollar++;
if (!(last_dollar < name.size() && name[last_dollar] >= '0' &&
name[last_dollar] <= '9')) {
return;
}
auto field_ref = insn->get_field();
Expand Down Expand Up @@ -534,7 +564,7 @@ void SynthAccessorPatcher::run(const Scope& scope) {
}
patch_synth_methods_overriding_annotated_methods(m);
if (is_constructor(m)) {
if (m->get_param_anno()) {
if (has_typedef_annos(m->get_param_anno(), m_typedef_annos)) {
patch_synth_cls_fields_from_ctor_param(m);
} else {
if (has_kotlin_default_ctor_marker(m)) {
Expand Down Expand Up @@ -1367,6 +1397,16 @@ void TypedefAnnoChecker::check_instruction(
<< ".\n failed instruction: " << show(insn) << "\n\n";
m_error += out.str();
m_good = false;
} else if (env_anno == boost::none && field_anno != boost::none) {
bool good = check_typedef_value(m, field_anno, ud_chains, insn, 0,
inference, envs);
if (!good) {
std::ostringstream out;
out << " Error writing to field " << show(insn->get_field())
<< "in method" << SHOW(m) << "\n\n";
m_error += out.str();
TRACE(TAC, 1, "writing to field: %s", SHOW(insn->get_field()));
}
}
break;
}
Expand Down Expand Up @@ -1566,6 +1606,12 @@ bool TypedefAnnoChecker::check_typedef_value(
if (is_parcel_or_json_read(def_method) && is_model_gen(m)) {
break;
}
// the result of usedef chains on a check cast could resolve to this
// look further up for the real source
if (is_null_check(def_method)) {
check_typedef_value(m, annotation, ud_chains, def, 0, inference, envs);
break;
}
std::vector<const DexMethod*> callees;
if (mog::is_true_virtual(m_method_override_graph, def_method) &&
!def_method->get_code()) {
Expand Down Expand Up @@ -1652,6 +1698,10 @@ bool TypedefAnnoChecker::check_typedef_value(
}
break;
}
case OPCODE_CHECK_CAST: {
check_typedef_value(m, annotation, ud_chains, def, 0, inference, envs);
break;
}
default: {
std::ostringstream out;
out << "TypedefAnnoCheckerPass: the method " << show(m)
Expand Down
100 changes: 82 additions & 18 deletions test/integ/TypedefAnnoCheckerTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1115,8 +1115,15 @@ TEST_F(TypedefAnnoCheckerTest, TestInvalidVarField) {
run_patcher(scope, *method_override_graph);

auto checker = run_checker(scope, method, *method_override_graph);
// See T193638685: Temporarily disabling to unblock SDK 35
// EXPECT_FALSE(checker.complete());
EXPECT_FALSE(checker.complete());
EXPECT_EQ(
checker.error(),
"TypedefAnnoCheckerPass: in method Lcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testInvalidVarField:()Ljava/lang/String;\n\
the string value 5 does not have the typedef annotation \n\
Linteg/TestStringDef; attached to it. \n\
Check that the value is annotated and exists in the typedef annotation class.\n\
failed instruction: CONST_STRING \"5\"\n\
Error writing to field Lcom/facebook/redextest/TypedefAnnoCheckerKtTest;.var_field:Ljava/lang/String;in methodLcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testInvalidVarField:()Ljava/lang/String;\n\n");
}

TEST_F(TypedefAnnoCheckerTest, TestReturnIntField) {
Expand Down Expand Up @@ -1182,18 +1189,15 @@ TEST_F(TypedefAnnoCheckerTest, TestInvalidCompanionVarSetter) {
run_patcher(scope, *method_override_graph);

auto checker = run_checker(scope, method, *method_override_graph);
// See T193638685: Temporarily disabling to unblock SDK 35
// EXPECT_FALSE(checker.complete());
// EXPECT_EQ(
// checker.error(),
// "TypedefAnnoCheckerPass: in method
// Lcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testInvalidCompanionVarSetter:()Ljava/lang/String;\n\
// the string value 5 does not have the typedef annotation \n\
// Linteg/TestStringDef; attached to it. \n\
// Check that the value is annotated and exists in the typedef annotation
// class.\n\
// failed instruction: CONST_STRING \"5\"\n\
// Error caught when returning the faulty value\n\n");
EXPECT_FALSE(checker.complete());
EXPECT_EQ(
checker.error(),
"TypedefAnnoCheckerPass: in method Lcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testInvalidCompanionVarSetter:()Ljava/lang/String;\n\
the string value 5 does not have the typedef annotation \n\
Linteg/TestStringDef; attached to it. \n\
Check that the value is annotated and exists in the typedef annotation class.\n\
failed instruction: CONST_STRING \"5\"\n\
Error writing to field Lcom/facebook/redextest/TypedefAnnoCheckerKtTest;.companion_var:Ljava/lang/String;in methodLcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testInvalidCompanionVarSetter:()Ljava/lang/String;\n\n");
}

TEST_F(TypedefAnnoCheckerTest, TestCompanionIntVarSetter) {
Expand Down Expand Up @@ -1675,7 +1679,22 @@ TEST_F(TypedefAnnoCheckerTest, TestLambdaCallLocalVarIntInvalid) {
run_patcher(scope, *method_override_graph);

auto checker = run_checker(scope, method, *method_override_graph);
// EXPECT_FALSE(checker.complete());
EXPECT_FALSE(checker.complete());
EXPECT_EQ(
checker.error(),
"TypedefAnnoCheckerPass: in method Lcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarIntInvalid:()I\n\
the int value 7 does not have the typedef annotation \n\
Linteg/TestIntDef; attached to it. \n\
Check that the value is annotated and exists in its typedef annotation class.\n\
failed instruction: CONST v1, 7\n\
Error writing to field Lkotlin/jvm/internal/Ref$IntRef;.element:Iin methodLcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarIntInvalid:()I\n\
\n\
TypedefAnnoCheckerPass: in method Lcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarIntInvalid:()I\n\
the int value 9 does not have the typedef annotation \n\
Linteg/TestIntDef; attached to it. \n\
Check that the value is annotated and exists in its typedef annotation class.\n\
failed instruction: CONST v1, 9\n\
Error writing to field Lkotlin/jvm/internal/Ref$IntRef;.element:Iin methodLcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarIntInvalid:()I\n\n");
}

TEST_F(TypedefAnnoCheckerTest, TestLambdaCallLocalVarIntDefault) {
Expand Down Expand Up @@ -1723,7 +1742,22 @@ TEST_F(TypedefAnnoCheckerTest, TestLambdaCallLocalVarIntDefaultInvalid) {
run_patcher(scope, *method_override_graph);

auto checker = run_checker(scope, method, *method_override_graph);
// EXPECT_FALSE(checker.complete());
EXPECT_FALSE(checker.complete());
EXPECT_EQ(
checker.error(),
"TypedefAnnoCheckerPass: in method Lcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarIntDefaultInvalid:()I\n\
the int value 7 does not have the typedef annotation \n\
Linteg/TestIntDef; attached to it. \n\
Check that the value is annotated and exists in its typedef annotation class.\n\
failed instruction: CONST v1, 7\n\
Error writing to field Lkotlin/jvm/internal/Ref$IntRef;.element:Iin methodLcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarIntDefaultInvalid:()I\n\
\n\
TypedefAnnoCheckerPass: in method Lcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarIntDefaultInvalid:()I\n\
the int value 9 does not have the typedef annotation \n\
Linteg/TestIntDef; attached to it. \n\
Check that the value is annotated and exists in its typedef annotation class.\n\
failed instruction: CONST v1, 9\n\
Error writing to field Lkotlin/jvm/internal/Ref$IntRef;.element:Iin methodLcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarIntDefaultInvalid:()I\n\n");
}

TEST_F(TypedefAnnoCheckerTest, TestLambdaCallLocalVarString) {
Expand Down Expand Up @@ -1771,7 +1805,22 @@ TEST_F(TypedefAnnoCheckerTest, TestLambdaCallLocalVarStringInvalid) {
run_patcher(scope, *method_override_graph);

auto checker = run_checker(scope, method, *method_override_graph);
// EXPECT_FALSE(checker.complete());
EXPECT_FALSE(checker.complete());
EXPECT_EQ(
checker.error(),
"TypedefAnnoCheckerPass: in method Lcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarStringInvalid:()Ljava/lang/String;\n\
the string value seven does not have the typedef annotation \n\
Linteg/TestStringDef; attached to it. \n\
Check that the value is annotated and exists in the typedef annotation class.\n\
failed instruction: CONST_STRING \"seven\"\n\
Error writing to field Lkotlin/jvm/internal/Ref$ObjectRef;.element:Ljava/lang/Object;in methodLcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarStringInvalid:()Ljava/lang/String;\n\
\n\
TypedefAnnoCheckerPass: in method Lcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarStringInvalid:()Ljava/lang/String;\n\
the string value eight does not have the typedef annotation \n\
Linteg/TestStringDef; attached to it. \n\
Check that the value is annotated and exists in the typedef annotation class.\n\
failed instruction: CONST_STRING \"eight\"\n\
Error writing to field Lkotlin/jvm/internal/Ref$ObjectRef;.element:Ljava/lang/Object;in methodLcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarStringInvalid:()Ljava/lang/String;\n\n");
}

TEST_F(TypedefAnnoCheckerTest, TestLambdaCallLocalVarStringDefault) {
Expand Down Expand Up @@ -1822,5 +1871,20 @@ TEST_F(TypedefAnnoCheckerTest, TestLambdaCallLocalVarStringDefaultInvalid) {
run_patcher(scope, *method_override_graph);

auto checker = run_checker(scope, method, *method_override_graph);
// EXPECT_FALSE(checker.complete());
EXPECT_FALSE(checker.complete());
EXPECT_EQ(
checker.error(),
"TypedefAnnoCheckerPass: in method Lcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarStringDefaultInvalid:()Ljava/lang/String;\n\
the string value seven does not have the typedef annotation \n\
Linteg/TestStringDef; attached to it. \n\
Check that the value is annotated and exists in the typedef annotation class.\n\
failed instruction: CONST_STRING \"seven\"\n\
Error writing to field Lkotlin/jvm/internal/Ref$ObjectRef;.element:Ljava/lang/Object;in methodLcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarStringDefaultInvalid:()Ljava/lang/String;\n\
\n\
TypedefAnnoCheckerPass: in method Lcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarStringDefaultInvalid:()Ljava/lang/String;\n\
the string value eight does not have the typedef annotation \n\
Linteg/TestStringDef; attached to it. \n\
Check that the value is annotated and exists in the typedef annotation class.\n\
failed instruction: CONST_STRING \"eight\"\n\
Error writing to field Lkotlin/jvm/internal/Ref$ObjectRef;.element:Ljava/lang/Object;in methodLcom/facebook/redextest/TypedefAnnoCheckerKtTest;.testLambdaCallLocalVarStringDefaultInvalid:()Ljava/lang/String;\n\n");
}

0 comments on commit 0de7597

Please sign in to comment.