From fb63cbadb419f1de91acae9fc66be258e1d3d214 Mon Sep 17 00:00:00 2001 From: Adam Sotona Date: Mon, 29 Apr 2024 07:12:46 +0000 Subject: [PATCH] 8330684: ClassFile API runs into StackOverflowError while parsing certain class' bytes Reviewed-by: psandoz --- .../classfile/impl/ClassReaderImpl.java | 32 ++++++++++++++----- test/jdk/jdk/classfile/LimitsTest.java | 9 +++++- 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/classfile/impl/ClassReaderImpl.java b/src/java.base/share/classes/jdk/internal/classfile/impl/ClassReaderImpl.java index 9b6c7d7ad4c..2022f8f0154 100644 --- a/src/java.base/share/classes/jdk/internal/classfile/impl/ClassReaderImpl.java +++ b/src/java.base/share/classes/jdk/internal/classfile/impl/ClassReaderImpl.java @@ -150,7 +150,7 @@ public final class ClassReaderImpl @Override public ClassEntry thisClassEntry() { if (thisClass == null) { - thisClass = readEntry(thisClassPos, ClassEntry.class); + thisClass = readClassEntry(thisClassPos); } return thisClass; } @@ -339,6 +339,10 @@ public final class ClassReaderImpl // 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 final class ClassReaderImpl 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 final class ClassReaderImpl 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 final class ClassReaderImpl @Override public T readEntry(int pos, Class cls) { - var e = readEntry(pos); + return readEntry(pos, cls, 0, 0xff); + } + + private T readEntry(int pos, Class cls, int expectedTag) { + return readEntry(pos, cls, expectedTag, expectedTag); + } + + private T readEntry(int pos, Class 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 final class ClassReaderImpl @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 diff --git a/test/jdk/jdk/classfile/LimitsTest.java b/test/jdk/jdk/classfile/LimitsTest.java index d23a948fe9f..00aaf32385d 100644 --- a/test/jdk/jdk/classfile/LimitsTest.java +++ b/test/jdk/jdk/classfile/LimitsTest.java @@ -23,7 +23,7 @@ /* * @test - * @bug 8320360 + * @bug 8320360 8330684 * @summary Testing ClassFile limits. * @run junit LimitsTest */ @@ -31,6 +31,7 @@ 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 @@ class LimitsTest { 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()); + } }