8330684: ClassFile API runs into StackOverflowError while parsing certain class' bytes

Reviewed-by: psandoz

Author: asotona

src/java.base/share/classes/jdk/internal/classfile/impl/ClassReaderImpl.java

@@ -150,7 +150,7 @@ public int flags() {
     @Override
     public ClassEntry thisClassEntry() {
         if (thisClass == null) {
-            thisClass = readEntry(thisClassPos, ClassEntry.class);
+            thisClass = readClassEntry(thisClassPos);
         }
         return thisClass;
     }
@@ -339,6 +339,10 @@ void writeConstantPoolEntries(BufWriter buf) {
     // Constantpool
     @Override
     public PoolEntry entryByIndex(int index) {
+        return entryByIndex(index, 0, 0xff);
+    }
+
+    private PoolEntry entryByIndex(int index, int lowerBoundTag, int upperBoundTag) {
         if (index <= 0 || index >= constantPoolCount) {
             throw new ConstantPoolException("Bad CP index: " + index);
         }
@@ -349,6 +353,10 @@ public PoolEntry entryByIndex(int index) {
                 throw new ConstantPoolException("Unusable CP index: " + index);
             }
             int tag = readU1(offset);
+            if (tag < lowerBoundTag || tag > upperBoundTag) {
+                throw new ConstantPoolException(
+                        "Bad tag (" + tag + ") at index (" + index + ") position (" + offset + ")");
+            }
             final int q = offset + 1;
             info = switch (tag) {
                 case TAG_UTF8 -> new AbstractPoolEntry.Utf8EntryImpl(this, index, buffer, q + 2, readU2(q));
@@ -367,7 +375,7 @@ public PoolEntry entryByIndex(int index) {
                 case TAG_NAMEANDTYPE -> new AbstractPoolEntry.NameAndTypeEntryImpl(this, index, (AbstractPoolEntry.Utf8EntryImpl) readUtf8Entry(q),
                                                                                    (AbstractPoolEntry.Utf8EntryImpl) readUtf8Entry(q + 2));
                 case TAG_METHODHANDLE -> new AbstractPoolEntry.MethodHandleEntryImpl(this, index, readU1(q),
-                                                                                     (AbstractPoolEntry.AbstractMemberRefEntry) readEntry(q + 1));
+                                                                                     readEntry(q + 1, AbstractPoolEntry.AbstractMemberRefEntry.class, TAG_FIELDREF, TAG_INTERFACEMETHODREF));
                 case TAG_METHODTYPE -> new AbstractPoolEntry.MethodTypeEntryImpl(this, index, (AbstractPoolEntry.Utf8EntryImpl) readUtf8Entry(q));
                 case TAG_CONSTANTDYNAMIC -> new AbstractPoolEntry.ConstantDynamicEntryImpl(this, index, readU2(q), (AbstractPoolEntry.NameAndTypeEntryImpl) readNameAndTypeEntry(q + 2));
                 case TAG_INVOKEDYNAMIC -> new AbstractPoolEntry.InvokeDynamicEntryImpl(this, index, readU2(q), (AbstractPoolEntry.NameAndTypeEntryImpl) readNameAndTypeEntry(q + 2));
@@ -423,7 +431,15 @@ public PoolEntry readEntry(int pos) {
 
     @Override
     public <T extends PoolEntry> T readEntry(int pos, Class<T> cls) {
-        var e = readEntry(pos);
+        return readEntry(pos, cls, 0, 0xff);
+    }
+
+    private <T extends PoolEntry> T readEntry(int pos, Class<T> cls, int expectedTag) {
+        return readEntry(pos, cls, expectedTag, expectedTag);
+    }
+
+    private <T extends PoolEntry> T readEntry(int pos, Class<T> cls, int lowerBoundTag, int upperBoundTag) {
+        var e = entryByIndex(readU2(pos), lowerBoundTag, upperBoundTag);
         if (cls.isInstance(e)) return cls.cast(e);
         throw new ConstantPoolException("Not a " + cls.getSimpleName() + " at index: " + readU2(pos));
     }
@@ -454,27 +470,27 @@ public Utf8Entry readUtf8EntryOrNull(int pos) {
 
     @Override
     public ModuleEntry readModuleEntry(int pos) {
-        return readEntry(pos, ModuleEntry.class);
+        return readEntry(pos, ModuleEntry.class, TAG_MODULE);
     }
 
     @Override
     public PackageEntry readPackageEntry(int pos) {
-        return readEntry(pos, PackageEntry.class);
+        return readEntry(pos, PackageEntry.class, TAG_PACKAGE);
     }
 
     @Override
     public ClassEntry readClassEntry(int pos) {
-        return readEntry(pos, ClassEntry.class);
+        return readEntry(pos, ClassEntry.class, TAG_CLASS);
     }
 
     @Override
     public NameAndTypeEntry readNameAndTypeEntry(int pos) {
-        return readEntry(pos, NameAndTypeEntry.class);
+        return readEntry(pos, NameAndTypeEntry.class, TAG_NAMEANDTYPE);
     }
 
     @Override
     public MethodHandleEntry readMethodHandleEntry(int pos) {
-        return readEntry(pos, MethodHandleEntry.class);
+        return readEntry(pos, MethodHandleEntry.class, TAG_METHODHANDLE);
     }
 
     @Override

test/jdk/jdk/classfile/LimitsTest.java

@@ -23,14 +23,15 @@
 
 /*
  * @test
- * @bug 8320360
+ * @bug 8320360 8330684
  * @summary Testing ClassFile limits.
  * @run junit LimitsTest
  */
 import java.lang.constant.ClassDesc;
 import java.lang.constant.ConstantDescs;
 import java.lang.constant.MethodTypeDesc;
 import java.lang.classfile.ClassFile;
+import java.lang.classfile.constantpool.ConstantPoolException;
 import jdk.internal.classfile.impl.LabelContext;
 import org.junit.jupiter.api.Test;
 import static org.junit.jupiter.api.Assertions.*;
@@ -92,4 +93,10 @@ void testReadingOutOfBounds() {
         assertThrows(IllegalArgumentException.class, () -> ClassFile.of().parse(new byte[]{(byte)0xCA, (byte)0xFE, (byte)0xBA, (byte)0xBE}), "reading magic only");
         assertThrows(IllegalArgumentException.class, () -> ClassFile.of().parse(new byte[]{(byte)0xCA, (byte)0xFE, (byte)0xBA, (byte)0xBE, 0, 0, 0, 0, 0, 2}), "reading invalid CP size");
     }
+
+    @Test
+    void testInvalidClassEntry() {
+        assertThrows(ConstantPoolException.class, () -> ClassFile.of().parse(new byte[]{(byte)0xCA, (byte)0xFE, (byte)0xBA, (byte)0xBE,
+            0, 0, 0, 0, 0, 2, ClassFile.TAG_METHODREF, 0, 1, 0, 1, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}).thisClass());
+    }
 }