Reject classes inheriting from themselves directly or transitively.
Also enforce class definition ordering with respect to super
classes within a Dex file.
Bug: 28685551
Bug: 27682580
Change-Id: If3eba782538eb4328d4b8a542236632379e7c050
diff --git a/runtime/dex_file_verifier.cc b/runtime/dex_file_verifier.cc
index bbffbbb..c76044d 100644
--- a/runtime/dex_file_verifier.cc
+++ b/runtime/dex_file_verifier.cc
@@ -1956,6 +1956,29 @@
}
if (item->superclass_idx_ != DexFile::kDexNoIndex16) {
+ // Check that a class does not inherit from itself directly (by having
+ // the same type idx as its super class).
+ if (UNLIKELY(item->superclass_idx_ == item->class_idx_)) {
+ ErrorStringPrintf("Class with same type idx as its superclass: '%d'", item->class_idx_);
+ return false;
+ }
+
+ // Check that a class is defined after its super class (if the
+ // latter is defined in the same Dex file).
+ const DexFile::ClassDef* superclass_def = dex_file_->FindClassDef(item->superclass_idx_);
+ if (superclass_def != nullptr) {
+ // The superclass is defined in this Dex file.
+ if (superclass_def > item) {
+ // ClassDef item for super class appearing after the class' ClassDef item.
+ ErrorStringPrintf("Invalid class definition ordering:"
+ " class with type idx: '%d' defined before"
+ " superclass with type idx: '%d'",
+ item->class_idx_,
+ item->superclass_idx_);
+ return false;
+ }
+ }
+
LOAD_STRING_BY_TYPE(superclass_descriptor, item->superclass_idx_,
"inter_class_def_item superclass_idx")
if (UNLIKELY(!IsValidDescriptor(superclass_descriptor) || superclass_descriptor[0] != 'L')) {
diff --git a/runtime/dex_file_verifier_test.cc b/runtime/dex_file_verifier_test.cc
index 3741c1e..fa6cfee 100644
--- a/runtime/dex_file_verifier_test.cc
+++ b/runtime/dex_file_verifier_test.cc
@@ -184,6 +184,12 @@
return dex_file;
}
+// To generate base64 encoded Dex file contents (such as kGoodTestDex,
+// below) from Smali files, use:
+//
+// smali -o classes.dex class1.smali [class2.smali ...]
+// base64 classes.dex >classes.dex.base64
+
// For reference.
static const char kGoodTestDex[] =
"ZGV4CjAzNQDrVbyVkxX1HljTznNf95AglkUAhQuFtmKkAgAAcAAAAHhWNBIAAAAAAAAAAAQCAAAN"
@@ -1521,4 +1527,84 @@
}
}
+// Generated from:
+//
+// .class public LB28685551;
+// .super LB28685551;
+
+static const char kClassExtendsItselfTestDex[] =
+ "ZGV4CjAzNQDtGF0irRLsVfRf3hwTL3zgNupO1a8AkTEEAQAAcAAAAHhWNBIAAAAAAAAAAKwAAAAB"
+ "AAAAcAAAAAEAAAB0AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAQAAAHgAAABsAAAAmAAAAJgA"
+ "AAAAAAAAAAAAAAEAAAAAAAAAAAAAAP////8AAAAAAAAAAAAAAAALTEIyODY4NTU1MTsAAAAAAAAA"
+ "AAcAAAAAAAAAAQAAAAAAAAABAAAAAQAAAHAAAAACAAAAAQAAAHQAAAAGAAAAAQAAAHgAAAACIAAA"
+ "AQAAAJgAAAADEAAAAQAAAKgAAAAAEAAAAQAAAKwAAAA=";
+
+TEST_F(DexFileVerifierTest, ClassExtendsItself) {
+ VerifyModification(
+ kClassExtendsItselfTestDex,
+ "class_extends_itself",
+ [](DexFile* dex_file ATTRIBUTE_UNUSED) { /* empty */ },
+ "Class with same type idx as its superclass: '0'");
+}
+
+// Generated from:
+//
+// .class public LFoo;
+// .super LBar;
+//
+// and:
+//
+// .class public LBar;
+// .super LFoo;
+
+static const char kClassesExtendOneAnotherTestDex[] =
+ "ZGV4CjAzNQDFHx5oxT079TdKMJ3M37hx96CSFQA8MaUsAQAAcAAAAHhWNBIAAAAAAAAAANQAAAAC"
+ "AAAAcAAAAAIAAAB4AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAgAAAIAAAABsAAAAwAAAAMAA"
+ "AADHAAAAAAAAAAEAAAABAAAAAQAAAAAAAAAAAAAA/////wAAAAAAAAAAAAAAAAAAAAABAAAAAQAA"
+ "AAAAAAD/////AAAAAAAAAAAAAAAABUxCYXI7AAVMRm9vOwAAAAAAAAAHAAAAAAAAAAEAAAAAAAAA"
+ "AQAAAAIAAABwAAAAAgAAAAIAAAB4AAAABgAAAAIAAACAAAAAAiAAAAIAAADAAAAAAxAAAAEAAADQ"
+ "AAAAABAAAAEAAADUAAAA";
+
+TEST_F(DexFileVerifierTest, ClassesExtendOneAnother) {
+ VerifyModification(
+ kClassesExtendOneAnotherTestDex,
+ "classes_extend_one_another",
+ [](DexFile* dex_file ATTRIBUTE_UNUSED) { /* empty */ },
+ "Invalid class definition ordering: class with type idx: '1' defined before"
+ " superclass with type idx: '0'");
+}
+
+// Generated from:
+//
+// .class public LAll;
+// .super LYour;
+//
+// and:
+//
+// .class public LYour;
+// .super LBase;
+//
+// and:
+//
+// .class public LBase;
+// .super LAll;
+
+static const char kCircularClassInheritanceTestDex[] =
+ "ZGV4CjAzNQDJJQtdMzp/mPagoltMlDWgd9MqiEtamoVcAQAAcAAAAHhWNBIAAAAAAAAAAAQBAAAD"
+ "AAAAcAAAAAMAAAB8AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAwAAAIgAAAB0AAAA6AAAAOgA"
+ "AADvAAAA9wAAAAAAAAABAAAAAgAAAAEAAAABAAAAAAAAAAAAAAD/////AAAAAAAAAAAAAAAAAgAA"
+ "AAEAAAABAAAAAAAAAP////8AAAAAAAAAAAAAAAAAAAAAAQAAAAIAAAAAAAAA/////wAAAAAAAAAA"
+ "AAAAAAVMQWxsOwAGTEJhc2U7AAZMWW91cjsAAAAAAAAHAAAAAAAAAAEAAAAAAAAAAQAAAAMAAABw"
+ "AAAAAgAAAAMAAAB8AAAABgAAAAMAAACIAAAAAiAAAAMAAADoAAAAAxAAAAEAAAAAAQAAABAAAAEA"
+ "AAAEAQAA";
+
+TEST_F(DexFileVerifierTest, CircularClassInheritance) {
+ VerifyModification(
+ kCircularClassInheritanceTestDex,
+ "circular_class_inheritance",
+ [](DexFile* dex_file ATTRIBUTE_UNUSED) { /* empty */ },
+ "Invalid class definition ordering: class with type idx: '1' defined before"
+ " superclass with type idx: '0'");
+}
+
} // namespace art