This is an automated email from the ASF dual-hosted git repository. garydgregory pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-bcel.git
commit ad48367df4b73b140f90c936ccd0d7a46544e7af Author: Gary Gregory <[email protected]> AuthorDate: Fri Sep 4 15:26:52 2026 -0400 Nested annotation element values recurse unboundedly; MAX_ARRAY_DIMENSIONS cap bypassed (f002). --- src/changes/changes.xml | 1 + .../org/apache/bcel/classfile/AnnotationEntry.java | 20 +++- .../org/apache/bcel/classfile/ElementValue.java | 10 +- .../bcel/classfile/ElementValueNestingTest.java | 103 +++++++++++++++++++++ 4 files changed, 131 insertions(+), 3 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 87b7ea02..81430405 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -89,6 +89,7 @@ The <action> type attribute can be add,update,fix,remove. <action type="fix" dev="ggregory" due-to="Naveed Khan, Gary Gregory">Bound IINC increment to signed short (#526).</action> <action type="fix" dev="ggregory" due-to="Gary Gregory">Fix SpotBugs USO_UNSAFE_METHOD_SYNCHRONIZATION in ConstantUtf8.</action> <action type="fix" dev="ggregory" due-to="Gary Gregory">Nested Code/Record attributes drive unbounded parse-time recursion in ClassParser (f001).</action> + <action type="fix" dev="ggregory" due-to="Gary Gregory">Nested annotation element values recurse unboundedly; MAX_ARRAY_DIMENSIONS cap bypassed (f002).</action> <!-- ADD --> <action type="add" dev="ggregory" due-to="nbauma109, Gary Gregory">Add support for permitted subclasses #493.</action> <action type="add" dev="ggregory" due-to="nbauma109, Gary Gregory">Add RecordComponentInfo.getAttribute(byte tag)#494.</action> diff --git a/src/main/java/org/apache/bcel/classfile/AnnotationEntry.java b/src/main/java/org/apache/bcel/classfile/AnnotationEntry.java index 9a58b0cc..0c3ae5e1 100644 --- a/src/main/java/org/apache/bcel/classfile/AnnotationEntry.java +++ b/src/main/java/org/apache/bcel/classfile/AnnotationEntry.java @@ -61,11 +61,27 @@ public class AnnotationEntry implements Node { * @throws IOException Thrown if an I/O error occurs. */ public static AnnotationEntry read(final DataInput input, final ConstantPool constantPool, final boolean isRuntimeVisible) throws IOException { + return read(input, constantPool, isRuntimeVisible, 0); + } + + /** + * Factory method to create an AnnotionEntry from a DataInput, carrying the nesting depth of the enclosing element values so that + * {@link ElementValue#readElementValue(DataInput, ConstantPool, boolean, int)} can bound the combined annotation/array nesting depth. + * + * @param input The input stream. + * @param constantPool The constant pool. + * @param isRuntimeVisible whether the annotation is runtime visible. + * @param nesting the current element value nesting level. + * @return The entry. + * @throws IOException Thrown if an I/O error occurs. + */ + static AnnotationEntry read(final DataInput input, final ConstantPool constantPool, final boolean isRuntimeVisible, final int nesting) + throws IOException { final AnnotationEntry annotationEntry = new AnnotationEntry(input.readUnsignedShort(), constantPool, isRuntimeVisible); final int numElementValuePairs = input.readUnsignedShort(); for (int i = 0; i < numElementValuePairs; i++) { - annotationEntry.elementValuePairs - .add(new ElementValuePair(input.readUnsignedShort(), ElementValue.readElementValue(input, constantPool, isRuntimeVisible, 0), constantPool)); + annotationEntry.elementValuePairs.add( + new ElementValuePair(input.readUnsignedShort(), ElementValue.readElementValue(input, constantPool, isRuntimeVisible, nesting), constantPool)); } return annotationEntry; } diff --git a/src/main/java/org/apache/bcel/classfile/ElementValue.java b/src/main/java/org/apache/bcel/classfile/ElementValue.java index e1af96df..1a607323 100644 --- a/src/main/java/org/apache/bcel/classfile/ElementValue.java +++ b/src/main/java/org/apache/bcel/classfile/ElementValue.java @@ -129,7 +129,15 @@ public abstract class ElementValue { return new ClassElementValue(CLASS, input.readUnsignedShort(), cpool); case ANNOTATION: - return new AnnotationElementValue(ANNOTATION, AnnotationEntry.read(input, cpool, isRuntimeVisible), cpool); + arrayNesting++; + if (arrayNesting > Const.MAX_ARRAY_DIMENSIONS) { + // Annotation element values may legitimately nest (annotations whose members are annotations or arrays thereof), but a malicious class file + // can alternate annotation and array nesting to recurse without limit. Count both kinds of nesting against the same JVM spec 4.4.1 bound so + // the depth cannot be reset by wrapping an array in an annotation (CWE-674). + throw new ClassFormatException( + String.format("Annotation element values are only valid if they nest %,d or fewer levels.", Const.MAX_ARRAY_DIMENSIONS)); + } + return new AnnotationElementValue(ANNOTATION, AnnotationEntry.read(input, cpool, isRuntimeVisible, arrayNesting), cpool); case ARRAY: arrayNesting++; diff --git a/src/test/java/org/apache/bcel/classfile/ElementValueNestingTest.java b/src/test/java/org/apache/bcel/classfile/ElementValueNestingTest.java new file mode 100644 index 00000000..0ab96abc --- /dev/null +++ b/src/test/java/org/apache/bcel/classfile/ElementValueNestingTest.java @@ -0,0 +1,103 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.bcel.classfile; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.DataInputStream; +import java.io.DataOutputStream; +import java.io.IOException; + +import org.junit.jupiter.api.Test; + +/** + * Tests that {@link ElementValue#readElementValue(java.io.DataInput, ConstantPool)} bounds the combined annotation/array nesting depth instead of recursing + * until a {@link StackOverflowError}. + */ +class ElementValueNestingTest { + + /** + * Wraps the given element value bytes into an annotation element value ('@'): one annotation with a single element value pair. + */ + private static byte[] annotationOf(final byte[] inner) throws IOException { + final ByteArrayOutputStream baos = new ByteArrayOutputStream(); + try (DataOutputStream dos = new DataOutputStream(baos)) { + dos.writeByte(ElementValue.ANNOTATION); + dos.writeShort(0); // type_index + dos.writeShort(1); // num_element_value_pairs + dos.writeShort(0); // element_name_index + dos.write(inner); + } + return baos.toByteArray(); + } + + /** + * Wraps the given element value bytes into an array element value ('[') with a single element. + */ + private static byte[] arrayOf(final byte[] inner) throws IOException { + final ByteArrayOutputStream baos = new ByteArrayOutputStream(); + try (DataOutputStream dos = new DataOutputStream(baos)) { + dos.writeByte(ElementValue.ARRAY); + dos.writeShort(1); // num_values + dos.write(inner); + } + return baos.toByteArray(); + } + + private static byte[] stringValue() { + return new byte[] {ElementValue.STRING, 0, 0}; + } + + @Test + void testAlternatingArrayAnnotationNestingRejected() throws IOException { + byte[] bytes = stringValue(); + for (int i = 0; i < 300; i++) { + bytes = arrayOf(annotationOf(bytes)); + } + try (DataInputStream in = new DataInputStream(new ByteArrayInputStream(bytes))) { + assertThrows(ClassFormatException.class, () -> ElementValue.readElementValue(in, new ConstantPool(new ConstantUtf8("Test")))); + } + } + + @Test + void testDeeplyNestedAnnotationsRejected() throws IOException { + byte[] bytes = stringValue(); + for (int i = 0; i < 300; i++) { + bytes = annotationOf(bytes); + } + try (DataInputStream in = new DataInputStream(new ByteArrayInputStream(bytes))) { + assertThrows(ClassFormatException.class, () -> ElementValue.readElementValue(in, new ConstantPool(new ConstantUtf8("Test")))); + } + } + + @Test + void testModeratelyNestedAnnotationsAccepted() throws IOException { + byte[] bytes = stringValue(); + for (int i = 0; i < 3; i++) { + bytes = annotationOf(arrayOf(bytes)); + } + try (DataInputStream in = new DataInputStream(new ByteArrayInputStream(bytes))) { + assertTrue(ElementValue.readElementValue(in, new ConstantPool(new ConstantUtf8("Test"))) instanceof AnnotationElementValue); + } + } +}
