From 2427e3cc0e76d9c4cef237e24fbcfd80d81c2675 Mon Sep 17 00:00:00 2001 From: Naveed Khan Date: Wed, 15 Jul 2026 21:41:40 +0530 Subject: [PATCH 1/2] validate u1 count bound in invokeinterface and multianewarray --- .../apache/bcel/generic/INVOKEINTERFACE.java | 4 +- .../apache/bcel/generic/MULTIANEWARRAY.java | 2 +- .../apache/bcel/generic/CountBoundsTest.java | 69 +++++++++++++++++++ 3 files changed, 72 insertions(+), 3 deletions(-) create mode 100644 src/test/java/org/apache/bcel/generic/CountBoundsTest.java diff --git a/src/main/java/org/apache/bcel/generic/INVOKEINTERFACE.java b/src/main/java/org/apache/bcel/generic/INVOKEINTERFACE.java index 6657b3a897..1945b1fb53 100644 --- a/src/main/java/org/apache/bcel/generic/INVOKEINTERFACE.java +++ b/src/main/java/org/apache/bcel/generic/INVOKEINTERFACE.java @@ -55,8 +55,8 @@ public final class INVOKEINTERFACE extends InvokeInstruction { public INVOKEINTERFACE(final int index, final int nargs) { super(Const.INVOKEINTERFACE, index); super.setLength(5); - if (nargs < 1) { - throw new ClassGenException("Number of arguments must be > 0 " + nargs); + if (nargs < 1 || nargs > Const.MAX_BYTE) { + throw new ClassGenException("Number of arguments out of range (1 - " + Const.MAX_BYTE + "): " + nargs); } this.nargs = nargs; } diff --git a/src/main/java/org/apache/bcel/generic/MULTIANEWARRAY.java b/src/main/java/org/apache/bcel/generic/MULTIANEWARRAY.java index bdc2f0272c..2c720c7e7f 100644 --- a/src/main/java/org/apache/bcel/generic/MULTIANEWARRAY.java +++ b/src/main/java/org/apache/bcel/generic/MULTIANEWARRAY.java @@ -50,7 +50,7 @@ public class MULTIANEWARRAY extends CPInstruction implements LoadClass, Allocati */ public MULTIANEWARRAY(final int index, final short dimensions) { super(org.apache.bcel.Const.MULTIANEWARRAY, index); - if (dimensions < 1) { + if (dimensions < 1 || dimensions > org.apache.bcel.Const.MAX_BYTE) { throw new ClassGenException("Invalid dimensions value: " + dimensions); } this.dimensions = dimensions; diff --git a/src/test/java/org/apache/bcel/generic/CountBoundsTest.java b/src/test/java/org/apache/bcel/generic/CountBoundsTest.java new file mode 100644 index 0000000000..0d69e0cd88 --- /dev/null +++ b/src/test/java/org/apache/bcel/generic/CountBoundsTest.java @@ -0,0 +1,69 @@ +/* + * 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.generic; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.io.ByteArrayOutputStream; +import java.io.DataOutputStream; +import java.io.IOException; + +import org.apache.bcel.Const; +import org.junit.jupiter.api.Test; + +/** + * The {@code count} operand of {@code invokeinterface} and the {@code dimensions} operand of {@code multianewarray} are + * both encoded as a u1, so the constructors must reject a value above {@link Const#MAX_BYTE}; otherwise {@code dump} + * truncates it with {@code writeByte} and the emitted instruction carries a different count than requested. The lower + * bound was already enforced; these cover the missing upper bound. + */ +class CountBoundsTest { + + private static int dumpedByte(final Instruction i, final int offset) throws IOException { + final ByteArrayOutputStream bos = new ByteArrayOutputStream(); + try (DataOutputStream dos = new DataOutputStream(bos)) { + i.dump(dos); + } + return bos.toByteArray()[offset] & 0xFF; + } + + @Test + void testInvokeInterfaceCountRoundTrips() throws IOException { + assertEquals(Const.MAX_BYTE, dumpedByte(new INVOKEINTERFACE(1, Const.MAX_BYTE), 3)); + } + + @Test + void testInvokeInterfaceRejectsCountAboveU1() { + assertEquals(Const.MAX_BYTE, new INVOKEINTERFACE(1, Const.MAX_BYTE).getCount()); + assertThrows(ClassGenException.class, () -> new INVOKEINTERFACE(1, Const.MAX_BYTE + 1)); + } + + @Test + void testMultiANewArrayDimensionsRoundTrips() throws IOException { + assertEquals(Const.MAX_BYTE, dumpedByte(new MULTIANEWARRAY(1, (short) Const.MAX_BYTE), 3)); + } + + @Test + void testMultiANewArrayRejectsDimensionsAboveU1() { + assertEquals(Const.MAX_BYTE, new MULTIANEWARRAY(1, (short) Const.MAX_BYTE).getDimensions()); + assertThrows(ClassGenException.class, () -> new MULTIANEWARRAY(1, (short) (Const.MAX_BYTE + 1))); + } +} From b9769ea0abb5ad0af3492745c4776d4acb60041f Mon Sep 17 00:00:00 2001 From: Naveed Khan Date: Thu, 16 Jul 2026 01:05:31 +0530 Subject: [PATCH 2/2] split count bounds test into one class per instruction --- ...undsTest.java => INVOKEINTERFACETest.java} | 37 ++++++++----------- .../bcel/generic/MULTIANEWARRAYTest.java | 29 +++++++++++++++ 2 files changed, 44 insertions(+), 22 deletions(-) rename src/test/java/org/apache/bcel/generic/{CountBoundsTest.java => INVOKEINTERFACETest.java} (53%) diff --git a/src/test/java/org/apache/bcel/generic/CountBoundsTest.java b/src/test/java/org/apache/bcel/generic/INVOKEINTERFACETest.java similarity index 53% rename from src/test/java/org/apache/bcel/generic/CountBoundsTest.java rename to src/test/java/org/apache/bcel/generic/INVOKEINTERFACETest.java index 0d69e0cd88..5d5ac4cdff 100644 --- a/src/test/java/org/apache/bcel/generic/CountBoundsTest.java +++ b/src/test/java/org/apache/bcel/generic/INVOKEINTERFACETest.java @@ -30,40 +30,33 @@ import org.junit.jupiter.api.Test; /** - * The {@code count} operand of {@code invokeinterface} and the {@code dimensions} operand of {@code multianewarray} are - * both encoded as a u1, so the constructors must reject a value above {@link Const#MAX_BYTE}; otherwise {@code dump} - * truncates it with {@code writeByte} and the emitted instruction carries a different count than requested. The lower - * bound was already enforced; these cover the missing upper bound. + * Tests {@link INVOKEINTERFACE}. + * + * @see JVM INVOKEINTERFACE specification */ -class CountBoundsTest { +class INVOKEINTERFACETest { - private static int dumpedByte(final Instruction i, final int offset) throws IOException { + private static int dumpedCountByte(final INVOKEINTERFACE instruction) throws IOException { final ByteArrayOutputStream bos = new ByteArrayOutputStream(); try (DataOutputStream dos = new DataOutputStream(bos)) { - i.dump(dos); + instruction.dump(dos); } - return bos.toByteArray()[offset] & 0xFF; + // opcode, u2 index, u1 count, u1 zero + return bos.toByteArray()[3] & 0xFF; } @Test - void testInvokeInterfaceCountRoundTrips() throws IOException { - assertEquals(Const.MAX_BYTE, dumpedByte(new INVOKEINTERFACE(1, Const.MAX_BYTE), 3)); + void testCountRoundTrips() throws IOException { + assertEquals(Const.MAX_BYTE, dumpedCountByte(new INVOKEINTERFACE(1, Const.MAX_BYTE))); } + /** + * The {@code count} operand of {@code invokeinterface} is an unsigned byte (1-255) and {@code dump} writes it with {@code writeByte}, so the constructor + * must reject values above {@link Const#MAX_BYTE} instead of truncating them. + */ @Test - void testInvokeInterfaceRejectsCountAboveU1() { + void testRejectsCountAboveU1() { assertEquals(Const.MAX_BYTE, new INVOKEINTERFACE(1, Const.MAX_BYTE).getCount()); assertThrows(ClassGenException.class, () -> new INVOKEINTERFACE(1, Const.MAX_BYTE + 1)); } - - @Test - void testMultiANewArrayDimensionsRoundTrips() throws IOException { - assertEquals(Const.MAX_BYTE, dumpedByte(new MULTIANEWARRAY(1, (short) Const.MAX_BYTE), 3)); - } - - @Test - void testMultiANewArrayRejectsDimensionsAboveU1() { - assertEquals(Const.MAX_BYTE, new MULTIANEWARRAY(1, (short) Const.MAX_BYTE).getDimensions()); - assertThrows(ClassGenException.class, () -> new MULTIANEWARRAY(1, (short) (Const.MAX_BYTE + 1))); - } } diff --git a/src/test/java/org/apache/bcel/generic/MULTIANEWARRAYTest.java b/src/test/java/org/apache/bcel/generic/MULTIANEWARRAYTest.java index 986f241e14..dd9ad02d5e 100644 --- a/src/test/java/org/apache/bcel/generic/MULTIANEWARRAYTest.java +++ b/src/test/java/org/apache/bcel/generic/MULTIANEWARRAYTest.java @@ -20,6 +20,11 @@ package org.apache.bcel.generic; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.io.ByteArrayOutputStream; +import java.io.DataOutputStream; +import java.io.IOException; import org.apache.bcel.Const; import org.apache.bcel.util.ByteSequence; @@ -32,6 +37,20 @@ */ class MULTIANEWARRAYTest { + private static int dumpedDimensionsByte(final MULTIANEWARRAY instruction) throws IOException { + final ByteArrayOutputStream bos = new ByteArrayOutputStream(); + try (DataOutputStream dos = new DataOutputStream(bos)) { + instruction.dump(dos); + } + // opcode, u2 index, u1 dimensions + return bos.toByteArray()[3] & 0xFF; + } + + @Test + void testDimensionsRoundTrips() throws IOException { + assertEquals(Const.MAX_BYTE, dumpedDimensionsByte(new MULTIANEWARRAY(1, (short) Const.MAX_BYTE))); + } + /** * The {@code dimensions} operand of {@code multianewarray} is an unsigned byte (1-255), so a value above 127 must not be read as a negative number. */ @@ -44,4 +63,14 @@ void testInitFromFileReadsDimensionsUnsigned() throws Exception { assertEquals(200, instruction.getDimensions()); } } + + /** + * The {@code dimensions} operand of {@code multianewarray} is an unsigned byte (1-255) and {@code dump} writes it with {@code writeByte}, so the + * constructor must reject values above {@link Const#MAX_BYTE} instead of truncating them. + */ + @Test + void testRejectsDimensionsAboveU1() { + assertEquals(Const.MAX_BYTE, new MULTIANEWARRAY(1, (short) Const.MAX_BYTE).getDimensions()); + assertThrows(ClassGenException.class, () -> new MULTIANEWARRAY(1, (short) (Const.MAX_BYTE + 1))); + } }