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
The following commit(s) were added to refs/heads/master by this push:
new 66f5a115 Nested Code/Record attributes drive unbounded parse-time
recursion in ClassParser (f001).
66f5a115 is described below
commit 66f5a115212451c9953a1fd7450c39e153c1f74f
Author: Gary Gregory <[email protected]>
AuthorDate: Fri Sep 4 10:26:17 2026 -0400
Nested Code/Record attributes drive unbounded parse-time recursion in
ClassParser (f001).
Class files with attributes nested deeper than 64 levels are now
rejected with ClassFormatException; raise
org.apache.bcel.classfile.Attribute.maxNestingDepth if legitimate deeper
nesting is required.
---
src/changes/changes.xml | 1 +
.../java/org/apache/bcel/classfile/Attribute.java | 44 +++++++++++-
.../bcel/classfile/AttributeNestingTest.java | 81 ++++++++++++++++++++++
3 files changed, 125 insertions(+), 1 deletion(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 58f74291..87b7ea02 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -88,6 +88,7 @@ The <action> type attribute can be add,update,fix,remove.
<action type="fix" dev="ggregory" due-to="Naveed Khan,
Gary Gregory">Match wide local variable instruction length to dumped bytes
(#525).</action>
<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>
<!-- 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/Attribute.java
b/src/main/java/org/apache/bcel/classfile/Attribute.java
index deb478f4..61144e07 100644
--- a/src/main/java/org/apache/bcel/classfile/Attribute.java
+++ b/src/main/java/org/apache/bcel/classfile/Attribute.java
@@ -40,7 +40,13 @@ import org.apache.bcel.util.Args;
* u1 info[attribute_length];
* }
* </pre>
- *
+ * <p>
+ * The default maximum number of attribute nesting levels in {@link
#readAttribute(DataInput, ConstantPool)} is {@code 64} before throwing a
+ * {@link ClassFormatException}. This is configurable through the system
property {@code org.apache.bcel.classfile.Attribute.maxNestingDepth}.
Attributes may
+ * legitimately nest (for example, a <em>Code</em> attribute carries its own
attribute table, and <em>Record</em> components carry theirs), but a malicious
+ * class file can nest such attributes deeply enough to overflow the parser's
stack.
+ * </p>
+ *
* @see ConstantValue
* @see SourceFile
* @see Code
@@ -57,6 +63,19 @@ public abstract class Attribute implements Cloneable, Node {
private static final boolean debug =
Boolean.getBoolean(Attribute.class.getCanonicalName() + ".debug"); // Debugging
on/off
+ /**
+ * Maximum number of attribute nesting levels {@link
#readAttribute(DataInput, ConstantPool)} accepts before throwing a {@link
ClassFormatException},
+ * configurable through the system property {@code
org.apache.bcel.classfile.Attribute.maxNestingDepth}. Attributes may
legitimately nest (for example, a
+ * <em>Code</em> attribute carries its own attribute table, and
<em>Record</em> components carry theirs), but a malicious class file can nest
such
+ * attributes deeply enough to overflow the parser's stack.
+ */
+ private static final int MAX_NESTING_DEPTH =
Integer.getInteger(Attribute.class.getCanonicalName() + ".maxNestingDepth",
64).intValue();
+
+ /**
+ * The per-thread attribute nesting depth of {@link
#readAttribute(DataInput, ConstantPool)}.
+ */
+ private static final ThreadLocal<Integer> NESTING_DEPTH =
ThreadLocal.withInitial(() -> Integer.valueOf(0));
+
private static final Map<String, Object> READERS = new HashMap<>();
/**
@@ -114,6 +133,29 @@ public abstract class Attribute implements Cloneable, Node
{
* @since 6.0
*/
public static Attribute readAttribute(final DataInput dataInput, final
ConstantPool constantPool) throws IOException {
+ // Track the nesting depth to guard against malicious class files that
nest attributes (for example, a Code attribute inside a Code attribute, or
+ // mutually recursive Record component attributes) deeply enough to
overflow the parser's stack (CWE-674).
+ final int depth = NESTING_DEPTH.get().intValue() + 1;
+ if (depth > MAX_NESTING_DEPTH) {
+ throw new ClassFormatException("Attributes are nested more than "
+ MAX_NESTING_DEPTH + " levels deep; if this is a valid class file, raise the"
+ + " limit with the system property " +
Attribute.class.getCanonicalName() + ".maxNestingDepth.");
+ }
+ NESTING_DEPTH.set(Integer.valueOf(depth));
+ try {
+ return readAttribute0(dataInput, constantPool);
+ } finally {
+ if (depth == 1) {
+ NESTING_DEPTH.remove();
+ } else {
+ NESTING_DEPTH.set(Integer.valueOf(depth - 1));
+ }
+ }
+ }
+
+ /**
+ * Reads one attribute without tracking the nesting depth; only to be
called by {@link #readAttribute(DataInput, ConstantPool)}.
+ */
+ private static Attribute readAttribute0(final DataInput dataInput, final
ConstantPool constantPool) throws IOException {
byte tag = Const.ATTR_UNKNOWN; // Unknown attribute
// Get class name from constant pool via 'name_index' indirection
final int nameIndex = dataInput.readUnsignedShort();
diff --git a/src/test/java/org/apache/bcel/classfile/AttributeNestingTest.java
b/src/test/java/org/apache/bcel/classfile/AttributeNestingTest.java
new file mode 100644
index 00000000..5595e368
--- /dev/null
+++ b/src/test/java/org/apache/bcel/classfile/AttributeNestingTest.java
@@ -0,0 +1,81 @@
+/*
+ * 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.DataInput;
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
+
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests that {@link Attribute#readAttribute(DataInput, ConstantPool)} bounds
the attribute nesting depth instead of recursing until a
+ * {@link StackOverflowError}.
+ */
+class AttributeNestingTest {
+
+ /**
+ * Builds a Code attribute nested {@code depth} times inside itself: each
level declares one sub-attribute, which is again a Code attribute.
+ */
+ private static byte[] nestedCodeAttribute(final int depth) throws
IOException {
+ byte[] inner = {};
+ int attributeCount = 0;
+ for (int i = 0; i < depth; i++) {
+ final ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ try (DataOutputStream dos = new DataOutputStream(baos)) {
+ dos.writeShort(1); // attribute_name_index -> "Code"
+ dos.writeInt(12 + inner.length); // attribute_length
+ dos.writeShort(0); // max_stack
+ dos.writeShort(0); // max_locals
+ dos.writeInt(0); // code_length
+ dos.writeShort(0); // exception_table_length
+ dos.writeShort(attributeCount); // attributes_count
+ dos.write(inner);
+ }
+ inner = baos.toByteArray();
+ attributeCount = 1;
+ }
+ return inner;
+ }
+
+ @Test
+ void testDeeplyNestedCodeAttributeRejected() throws IOException {
+ final ConstantPool cp = new ConstantPool(new ConstantUtf8("unused
index 0"), new ConstantUtf8("Code"));
+ final byte[] bytes = nestedCodeAttribute(1_000);
+ try (DataInputStream in = new DataInputStream(new
ByteArrayInputStream(bytes))) {
+ assertThrows(ClassFormatException.class, () ->
Attribute.readAttribute((DataInput) in, cp));
+ }
+ }
+
+ @Test
+ void testModeratelyNestedCodeAttributeAccepted() throws IOException {
+ final ConstantPool cp = new ConstantPool(new ConstantUtf8("unused
index 0"), new ConstantUtf8("Code"));
+ final byte[] bytes = nestedCodeAttribute(3);
+ try (DataInputStream in = new DataInputStream(new
ByteArrayInputStream(bytes))) {
+ assertTrue(Attribute.readAttribute((DataInput) in, cp) instanceof
Code);
+ }
+ }
+}
