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 8f9a414b Validate index upper bound in CPInstruction, IINC and RET
setIndex (#506)
8f9a414b is described below
commit 8f9a414b118fbe39fb3a476d78afd89d41990da1
Author: Dexter.k <[email protected]>
AuthorDate: Sat Jun 20 15:22:09 2026 +0000
Validate index upper bound in CPInstruction, IINC and RET setIndex (#506)
* validate index upper bound in CPInstruction, IINC and RET setIndex
these setters only rejected negative indices, so an index above
Const.MAX_SHORT was silently truncated to a u2 by dump(); match
LocalVariableInstruction.setIndex which already guards both bounds.
* No need for a FQCN, use an import like everywhere else.
* Update CPInstruction.java
* Update CPInstruction.java
---------
Co-authored-by: Gary Gregory <[email protected]>
---
.../org/apache/bcel/generic/CPInstruction.java | 9 ++--
src/main/java/org/apache/bcel/generic/IINC.java | 4 +-
src/main/java/org/apache/bcel/generic/RET.java | 4 +-
.../apache/bcel/generic/SetIndexBoundsTest.java | 53 ++++++++++++++++++++++
4 files changed, 62 insertions(+), 8 deletions(-)
diff --git a/src/main/java/org/apache/bcel/generic/CPInstruction.java
b/src/main/java/org/apache/bcel/generic/CPInstruction.java
index d0a54c8f..8737c27a 100644
--- a/src/main/java/org/apache/bcel/generic/CPInstruction.java
+++ b/src/main/java/org/apache/bcel/generic/CPInstruction.java
@@ -21,6 +21,7 @@ package org.apache.bcel.generic;
import java.io.DataOutputStream;
import java.io.IOException;
+import org.apache.bcel.Const;
import org.apache.bcel.classfile.Constant;
import org.apache.bcel.classfile.ConstantClass;
import org.apache.bcel.classfile.ConstantPool;
@@ -84,7 +85,7 @@ public abstract class CPInstruction extends Instruction
implements TypedInstruct
@Override
public Type getType(final ConstantPoolGen cpg) {
final ConstantPool cp = cpg.getConstantPool();
- String name = cp.getConstantString(index,
org.apache.bcel.Const.CONSTANT_Class);
+ String name = cp.getConstantString(index, Const.CONSTANT_Class);
if (!name.startsWith("[")) {
name = "L" + name + ";";
}
@@ -110,8 +111,8 @@ public abstract class CPInstruction extends Instruction
implements TypedInstruct
*/
@Override
public void setIndex(final int index) { // TODO could be package-protected?
- if (index < 0) {
- throw new ClassGenException("Negative index value: " + index);
+ if (index < 0 || index > Const.MAX_SHORT) {
+ throw new ClassGenException("Illegal value: " + index);
}
this.index = index;
}
@@ -140,6 +141,6 @@ public abstract class CPInstruction extends Instruction
implements TypedInstruct
if (c instanceof ConstantClass) {
str = Utility.packageToPath(str);
}
- return org.apache.bcel.Const.getOpcodeName(super.getOpcode()) + " " +
str;
+ return Const.getOpcodeName(super.getOpcode()) + " " + str;
}
}
diff --git a/src/main/java/org/apache/bcel/generic/IINC.java
b/src/main/java/org/apache/bcel/generic/IINC.java
index 37799fd5..45b479e6 100644
--- a/src/main/java/org/apache/bcel/generic/IINC.java
+++ b/src/main/java/org/apache/bcel/generic/IINC.java
@@ -135,8 +135,8 @@ public class IINC extends LocalVariableInstruction {
*/
@Override
public final void setIndex(final int n) {
- if (n < 0) {
- throw new ClassGenException("Negative index value: " + n);
+ if (n < 0 || n > Const.MAX_SHORT) {
+ throw new ClassGenException("Illegal value: " + n);
}
super.setIndexOnly(n);
setWide();
diff --git a/src/main/java/org/apache/bcel/generic/RET.java
b/src/main/java/org/apache/bcel/generic/RET.java
index 25c7a9f4..0bf22f16 100644
--- a/src/main/java/org/apache/bcel/generic/RET.java
+++ b/src/main/java/org/apache/bcel/generic/RET.java
@@ -116,8 +116,8 @@ public class RET extends Instruction implements
IndexedInstruction, TypedInstruc
*/
@Override
public final void setIndex(final int n) {
- if (n < 0) {
- throw new ClassGenException("Negative index value: " + n);
+ if (n < 0 || n > org.apache.bcel.Const.MAX_SHORT) {
+ throw new ClassGenException("Illegal value: " + n);
}
index = n;
setWide();
diff --git a/src/test/java/org/apache/bcel/generic/SetIndexBoundsTest.java
b/src/test/java/org/apache/bcel/generic/SetIndexBoundsTest.java
new file mode 100644
index 00000000..888710c0
--- /dev/null
+++ b/src/test/java/org/apache/bcel/generic/SetIndexBoundsTest.java
@@ -0,0 +1,53 @@
+/*
+ * 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 org.apache.bcel.Const;
+import org.junit.jupiter.api.Test;
+
+/**
+ * A constant pool or local variable index is encoded as a u2, so {@code
setIndex} must reject a value above
+ * {@link Const#MAX_SHORT}; otherwise {@code dump} truncates it with {@code
writeShort} and the emitted instruction
+ * references a different slot than requested. {@code
LocalVariableInstruction.setIndex} already enforces this; these
+ * cover the siblings that did not.
+ */
+class SetIndexBoundsTest {
+
+ @Test
+ void testCpInstructionRejectsIndexAboveU2() {
+ assertEquals(Const.MAX_SHORT, new LDC_W(Const.MAX_SHORT).getIndex());
+ assertThrows(ClassGenException.class, () -> new LDC_W(Const.MAX_SHORT
+ 1));
+ }
+
+ @Test
+ void testIincRejectsIndexAboveU2() {
+ assertEquals(Const.MAX_SHORT, new IINC(Const.MAX_SHORT, 1).getIndex());
+ assertThrows(ClassGenException.class, () -> new IINC(Const.MAX_SHORT +
1, 1));
+ }
+
+ @Test
+ void testRetRejectsIndexAboveU2() {
+ assertEquals(Const.MAX_SHORT, new RET(Const.MAX_SHORT).getIndex());
+ assertThrows(ClassGenException.class, () -> new RET(Const.MAX_SHORT +
1));
+ }
+}
