Gotcha, will fix. Gary
On Mon, Nov 21, 2022, 08:23 Mark Thomas <ma...@apache.org> wrote: > On 21/11/2022 13:03, ggreg...@apache.org wrote: > > This is an automated email from the ASF dual-hosted git repository. > > > > ggregory 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 428b65e0 Validate the u4 length of all attributes > > 428b65e0 is described below > > > > commit 428b65e0d6cc0f094b428f8ab92810ad7221afe1 > > Author: Gary David Gregory (Code signing key) <ggreg...@apache.org> > > AuthorDate: Mon Nov 21 08:03:04 2022 -0500 > > > > Validate the u4 length of all attributes > > This breaks the format of the error message for four byte unsigned int > values bigger than Integer.MAX_VALUE > > The "length & 0xFFFFFFFFL" snippet was there to ensure that these were > correctly displayed as a positive (long) integer. > > Mark > > > > --- > > src/changes/changes.xml | 2 +- > > .../java/org/apache/bcel/classfile/Attribute.java | 37 > ++++++++++++++++++---- > > .../java/org/apache/bcel/classfile/Unknown.java | 10 +----- > > src/main/java/org/apache/bcel/util/Args.java | 28 > ++++++++++++++++ > > 4 files changed, 61 insertions(+), 16 deletions(-) > > > > diff --git a/src/changes/changes.xml b/src/changes/changes.xml > > index 502b818f..30a26c71 100644 > > --- a/src/changes/changes.xml > > +++ b/src/changes/changes.xml > > @@ -98,7 +98,7 @@ The <action> type attribute can be > add,update,fix,remove. > > <action type="fix" dev="ggregory" due-to="Gary > Gregory">org.apache.bcel.util.ClassPath hashCode() and equals() don't > match.</action> > > <action type="fix" dev="markt" > due-to="OSS-Fuzz">org.apache.bcel.classfile.StackMapType constructors now > throw ClassFormatException on invalid input.</action> > > <action type="add" dev="ggregory" > due-to="nbauma109, Gary Gregory">Code coverage and bug fixes for bcelifier > #171.</action> > > - <action type="fix" dev="markt" due-to="Mark > Thomas">org.apache.bcel.classfile.Unknown constructors now throw > ClassFormatException on invalid length input.</action> > > + <action type="fix" dev="markt" due-to="Mark > Thomas, Gary Gregory">org.apache.bcel.classfile.Attribute constructors now > throw ClassFormatException on invalid length input.</action> > > <!-- UPDATE --> > > <action type="update" dev="ggregory" > due-to="Gary Gregory">Bump spotbugs-maven-plugin from 4.7.2.2 to 4.7.3.0 > #167.</action> > > <action type="update" dev="ggregory" > due-to="Dependabot">Bump jmh.version from 1.35 to 1.36 #170.</action> > > diff --git a/src/main/java/org/apache/bcel/classfile/Attribute.java > b/src/main/java/org/apache/bcel/classfile/Attribute.java > > index d1d38f40..14a76cc2 100644 > > --- a/src/main/java/org/apache/bcel/classfile/Attribute.java > > +++ b/src/main/java/org/apache/bcel/classfile/Attribute.java > > @@ -27,9 +27,17 @@ import org.apache.bcel.Const; > > import org.apache.bcel.util.Args; > > > > /** > > - * Abstract super class for <em>Attribute</em> objects. Currently the > <em>ConstantValue</em>, <em>SourceFile</em>, > > - * <em>Code</em>, <em>Exceptiontable</em>, <em>LineNumberTable</em>, > <em>LocalVariableTable</em>, <em>InnerClasses</em> > > - * and <em>Synthetic</em> attributes are supported. The > <em>Unknown</em> attribute stands for non-standard-attributes. > > + * Abstract super class for <em>Attribute</em> objects. Currently the > <em>ConstantValue</em>, <em>SourceFile</em>, <em>Code</em>, > <em>Exceptiontable</em>, > > + * <em>LineNumberTable</em>, <em>LocalVariableTable</em>, > <em>InnerClasses</em> and <em>Synthetic</em> attributes are supported. The > <em>Unknown</em> attribute > > + * stands for non-standard-attributes. > > + * > > + * <pre> > > + * attribute_info { > > + * u2 attribute_name_index; > > + * u4 attribute_length; > > + * u1 info[attribute_length]; > > + * } > > + * </pre> > > * > > * @see ConstantValue > > * @see SourceFile > > @@ -238,10 +246,26 @@ public abstract class Attribute implements > Cloneable, Node { > > @java.lang.Deprecated > > protected ConstantPool constant_pool; // TODO make private (has > getter & setter) > > > > + /** > > + * Constructs an instance. > > + * > > + * <pre> > > + * attribute_info { > > + * u2 attribute_name_index; > > + * u4 attribute_length; > > + * u1 info[attribute_length]; > > + * } > > + * </pre> > > + * > > + * @param tag tag. > > + * @param nameIndex u2 name index. > > + * @param length u4 length. > > + * @param constantPool constant pool. > > + */ > > protected Attribute(final byte tag, final int nameIndex, final int > length, final ConstantPool constantPool) { > > this.tag = tag; > > this.name_index = Args.requireU2(nameIndex, 0, > constantPool.getLength(), getClass().getSimpleName() + " name index"); > > - this.length = length; > > + this.length = Args.requireU4(length, getClass().getSimpleName() > + " attribute length"); > > this.constant_pool = constantPool; > > } > > > > @@ -271,12 +295,13 @@ public abstract class Attribute implements > Cloneable, Node { > > } > > > > /** > > - * @return deep copy of this attribute > > + * @param constantPool constant pool to save. > > + * @return deep copy of this attribute. > > */ > > public abstract Attribute copy(ConstantPool constantPool); > > > > /** > > - * Dump attribute to file stream in binary format. > > + * Dumps attribute to file stream in binary format. > > * > > * @param file Output file stream > > * @throws IOException if an I/O error occurs. > > diff --git a/src/main/java/org/apache/bcel/classfile/Unknown.java > b/src/main/java/org/apache/bcel/classfile/Unknown.java > > index df088430..6f72dc24 100644 > > --- a/src/main/java/org/apache/bcel/classfile/Unknown.java > > +++ b/src/main/java/org/apache/bcel/classfile/Unknown.java > > @@ -49,7 +49,7 @@ public final class Unknown extends Attribute { > > public Unknown(final int nameIndex, final int length, final byte[] > bytes, final ConstantPool constantPool) { > > super(Const.ATTR_UNKNOWN, nameIndex, length, constantPool); > > this.bytes = bytes; > > - name = constantPool.getConstantUtf8(nameIndex).getBytes(); > > + this.name = constantPool.getConstantUtf8(nameIndex).getBytes(); > > } > > > > /** > > @@ -66,14 +66,6 @@ public final class Unknown extends Attribute { > > if (length > 0) { > > bytes = new byte[length]; > > input.readFully(bytes); > > - } else if (length < 0) { > > - // Length is defined in the JVM specification as an > unsigned 4 byte > > - // integer but is read as a signed integer. This is not an > issue as > > - // the JRE API consistently uses byte[] or ByteBuffer for > classes. > > - // Therefore treat any negative number here as a format > error. > > - throw new ClassFormatException(String.format( > > - "Invalid length %,d for Unknown attribute. Must be > greater than or equal to zero and less than %,d", > > - length & 0xFFFFFFFFL, Integer.MAX_VALUE)); > > } > > } > > > > diff --git a/src/main/java/org/apache/bcel/util/Args.java > b/src/main/java/org/apache/bcel/util/Args.java > > index defc071f..642f6508 100644 > > --- a/src/main/java/org/apache/bcel/util/Args.java > > +++ b/src/main/java/org/apache/bcel/util/Args.java > > @@ -53,6 +53,20 @@ public class Args { > > return require(value, 0, message); > > } > > > > + /** > > + * Requires a u1 value. > > + * > > + * @param value The value to test. > > + * @param message The message prefix > > + * @return The value to test. > > + */ > > + public static int requireU1(final int value, final String message) { > > + if (value < 0 || value > Const.MAX_BYTE) { > > + throw new ClassFormatException(String.format("%s [Value out > of range (0 - %,d) for type u1: %,d]", message, Const.MAX_BYTE, value)); > > + } > > + return value; > > + } > > + > > /** > > * Requires a u2 value of at least {@code min} and not above > {@code max}. > > * > > @@ -97,4 +111,18 @@ public class Args { > > public static int requireU2(final int value, final String message) > { > > return requireU2(value, 0, message); > > } > > + > > + /** > > + * Requires a u4 value. > > + * > > + * @param value The value to test. > > + * @param message The message prefix > > + * @return The value to test. > > + */ > > + public static int requireU4(final int value, final String message) { > > + if (value < 0) { > > + throw new ClassFormatException(String.format("%s [Value out > of range (0 - %,d) for type u4: %,d]", message, Integer.MAX_VALUE, value)); > > + } > > + return value; > > + } > > } > > >
