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;
+    }
  }

Reply via email to