Correct handling of references to invalid constant pool index 0.

markt-asf · markt-asf · commit 77c79974eb22 · 2022-12-09T11:53:55.000Z
Ensure that references to a constant pool entry with index zero trigger
a ClassFormatException, not a NullPointerException.
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
@@ -66,6 +66,7 @@ The <action> type attribute can be add,update,fix,remove.
       <!-- ADD -->
       <!-- FIX -->
       <action                  type="fix" dev="markt" due-to="OSS-Fuzz">When parsing an class with an invalid constant reference, ensure ClassParser.parse() throws ClassFormatException, not NullPointerException.</action>
+      <action                  type="fix" dev="markt" due-to="OSS-Fuzz">Ensure that references to a constant pool entry with index zero trigger a ClassFormatException, not a NullPointerException.</action>
       <!-- UPDATE -->
     </release>
     <release version="6.7.0" date="2022-11-28" description="Maintenance and bug fix release.">
diff --git a/src/main/java/org/apache/bcel/classfile/ConstantPool.java b/src/main/java/org/apache/bcel/classfile/ConstantPool.java
@@ -83,6 +83,7 @@ public ConstantPool(final DataInput input) throws IOException {
         constantPool = new Constant[constantPoolCount];
         /*
          * constantPool[0] is unused by the compiler and may be used freely by the implementation.
+         * constantPool[0] is currently unused by the implementation.
          */
         for (int i = 1; i < constantPoolCount; i++) {
             constantPool[i] = Constant.readConstant(input);
@@ -300,7 +301,7 @@ public <T extends Constant> T getConstant(final int index, final byte tag, final
      * @since 6.6.0
      */
     public <T extends Constant> T getConstant(final int index, final Class<T> castTo) throws ClassFormatException {
-        if (index >= constantPool.length || index < 0) {
+        if (index >= constantPool.length || index < 1) {
             throw new ClassFormatException("Invalid constant pool reference using index: " + index + ". Constant pool size is: " + constantPool.length);
         }
         if (constantPool[index] != null && !castTo.isAssignableFrom(constantPool[index].getClass())) {
@@ -309,9 +310,7 @@ public <T extends Constant> T getConstant(final int index, final Class<T> castTo
         }
         // Previous check ensures this won't throw a ClassCastException
         final T c = castTo.cast(constantPool[index]);
-        if (c == null
-            // the 0th element is always null
-            && index != 0) {
+        if (c == null) {
             final Constant prev = constantPool[index - 1];
             if (prev == null || prev.getTag() != Const.CONSTANT_Double && prev.getTag() != Const.CONSTANT_Long) {
                 throw new ClassFormatException("Constant pool at index " + index + " is null.");
diff --git a/src/test/java/org/apache/bcel/OssFuzzTestCase.java b/src/test/java/org/apache/bcel/OssFuzzTestCase.java
@@ -66,6 +66,11 @@ public void testIssue53676() throws Exception {
         testOssFuzzReproducer("53676");
     }
 
+    @Test
+    public void testIssue54119() throws Exception {
+        testOssFuzzReproducer("54119");
+    }
+
     private void testOssFuzzReproducer(final String issue) throws Exception {
         final File reproducerFile = new File("target/test-classes/ossfuzz/issue" + issue + "/Test.class");
         try (final FileInputStream reproducerInputStream = new FileInputStream(reproducerFile)) {
diff --git a/src/test/java/org/apache/bcel/classfile/ConstantPoolTestCase.java b/src/test/java/org/apache/bcel/classfile/ConstantPoolTestCase.java
@@ -59,7 +59,7 @@ public void testClassWithDoubleConstantPoolItem() throws ClassNotFoundException,
             assertEquals(1, fields.length);
             assertEquals(ClassWithDoubleConstantPoolItem.class.getDeclaredFields()[0].getName(), fields[0].getName());
             final ConstantPool pool = c.getConstantPool();
-            IntStream.range(0, pool.getLength()).forEach(i -> assertDoesNotThrow(() -> {
+            IntStream.range(1, pool.getLength()).forEach(i -> assertDoesNotThrow(() -> {
                 final Constant constant = pool.getConstant(i);
                 if (constant instanceof ConstantDouble) {
                     assertEquals(classWithDoubleConstantPoolItem.d, ((ConstantDouble) constant).getBytes());
@@ -79,7 +79,7 @@ public void testClassWithLongConstantPoolItem() throws ClassNotFoundException, I
             assertEquals(1, fields.length);
             assertEquals(ClassWithLongConstantPoolItem.class.getDeclaredFields()[0].getName(), fields[0].getName());
             final ConstantPool pool = c.getConstantPool();
-            IntStream.range(0, pool.getLength()).forEach(i -> assertDoesNotThrow(() -> {
+            IntStream.range(1, pool.getLength()).forEach(i -> assertDoesNotThrow(() -> {
                 final Constant constant = pool.getConstant(i);
                 if (constant instanceof ConstantLong) {
                     assertEquals(classWithLongConstantPoolItem.l, ((ConstantLong) constant).getBytes());
