Merge "Reject classes inheriting from themselves directly or transitively."

2 files changed, 109 insertions, 0 deletions

diff --git a/runtime/dex_file_verifier.cc b/runtime/dex_file_verifier.cc
index bbffbbb7b7..c76044dd5a 100644
--- a/runtime/dex_file_verifier.cc
+++ b/runtime/dex_file_verifier.cc
@@ -1956,6 +1956,29 @@ bool DexFileVerifier::CheckInterClassDefItem() {
   }
 
   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 3741c1e76c..fa6cfeee70 100644
--- a/runtime/dex_file_verifier_test.cc
+++ b/runtime/dex_file_verifier_test.cc
@@ -184,6 +184,12 @@ static std::unique_ptr<const DexFile> OpenDexFileBase64(const char* base64,
   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 @@ TEST_F(DexFileVerifierTest, ProtoOrdering) {
   }
 }
 
+// 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