Hm, after reading https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.7 I will pull up the validation.
TY for the commit :-) Gary On 2022/11/21 12:00:20 Gary Gregory wrote: > Hi Mark, > > Any reason not to do this check in the Attribute superclass? > > Gaty > > On Mon, Nov 21, 2022, 05:50 <ma...@apache.org> wrote: > > > This is an automated email from the ASF dual-hosted git repository. > > > > markt 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 89015357 Unknown attributes with invalid length now trigger > > ClassFormatException > > 89015357 is described below > > > > commit 89015357effff5559d0ae3e73b25801db08d7812 > > Author: Mark Thomas <ma...@apache.org> > > AuthorDate: Mon Nov 21 10:50:23 2022 +0000 > > > > Unknown attributes with invalid length now trigger ClassFormatException > > --- > > src/changes/changes.xml | 1 + > > src/main/java/org/apache/bcel/classfile/Unknown.java | 8 ++++++++ > > src/test/java/org/apache/bcel/OssFuzzTestCase.java | 9 +++++++++ > > src/test/resources/ossfuzz/issue53544a/Test.class | Bin 0 -> 49 bytes > > 4 files changed, 18 insertions(+) > > > > diff --git a/src/changes/changes.xml b/src/changes/changes.xml > > index ab28355f..502b818f 100644 > > --- a/src/changes/changes.xml > > +++ b/src/changes/changes.xml > > @@ -98,6 +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> > > <!-- 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/Unknown.java > > b/src/main/java/org/apache/bcel/classfile/Unknown.java > > index 23cd38eb..df088430 100644 > > --- a/src/main/java/org/apache/bcel/classfile/Unknown.java > > +++ b/src/main/java/org/apache/bcel/classfile/Unknown.java > > @@ -66,6 +66,14 @@ 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/test/java/org/apache/bcel/OssFuzzTestCase.java > > b/src/test/java/org/apache/bcel/OssFuzzTestCase.java > > index 9f4e63bb..8944ca4b 100644 > > --- a/src/test/java/org/apache/bcel/OssFuzzTestCase.java > > +++ b/src/test/java/org/apache/bcel/OssFuzzTestCase.java > > @@ -47,6 +47,15 @@ public class OssFuzzTestCase { > > testOssFuzzReproducer("53543"); > > } > > > > + /* > > + * The original issue 53544 was a false positive but reviewing that > > issue > > + * did find a valid issue nearby. > > + */ > > + @Test > > + public void testIssue53544a() throws Exception { > > + testOssFuzzReproducer("53544a"); > > + } > > + > > 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/resources/ossfuzz/issue53544a/Test.class > > b/src/test/resources/ossfuzz/issue53544a/Test.class > > new file mode 100644 > > index 00000000..5fbdd67f > > Binary files /dev/null and > > b/src/test/resources/ossfuzz/issue53544a/Test.class differ > > > > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
