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 972dfb6c Static WIDE ThreadLocal survives exceptions, corrupting the
next class's disassembly on the same thread (f025).
972dfb6c is described below
commit 972dfb6c4706ff4f102657a261d77af019c63f96
Author: Gary Gregory <[email protected]>
AuthorDate: Fri Sep 4 18:35:49 2026 -0400
Static WIDE ThreadLocal survives exceptions, corrupting the next class's
disassembly on the same thread (f025).
---
src/changes/changes.xml | 1 +
src/main/java/org/apache/bcel/classfile/Utility.java | 7 +++++++
.../java/org/apache/bcel/classfile/UtilityTest.java | 20 ++++++++++++++++++++
3 files changed, 28 insertions(+)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index d740d622..6261e348 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -110,6 +110,7 @@ The <action> type attribute can be add,update,fix,remove.
<action type="fix" dev="ggregory" due-to="Gary
Gregory">BCELifier interpolates attacker class/package names into generated
Java source unescaped (f022).</action>
<action type="fix" dev="ggregory" due-to="Gary
Gregory">Verifier pass 3a delayed checks are quadratic in attribute and code
size (f023).</action>
<action type="fix" dev="ggregory" due-to="Gary
Gregory">Verifier cache grows unboundedly with attacker-chosen class names
(f024).</action>
+ <action type="fix" dev="ggregory" due-to="Gary
Gregory">Static WIDE ThreadLocal survives exceptions, corrupting the next
class's disassembly on the same thread (f025).</action>
<!-- ADD -->
<action type="add" dev="ggregory" due-to="nbauma109,
Gary Gregory">Add support for permitted subclasses #493.</action>
<action type="add" dev="ggregory" due-to="nbauma109,
Gary Gregory">Add RecordComponentInfo.getAttribute(byte tag)#494.</action>
diff --git a/src/main/java/org/apache/bcel/classfile/Utility.java
b/src/main/java/org/apache/bcel/classfile/Utility.java
index 244954e7..a4cabd32 100644
--- a/src/main/java/org/apache/bcel/classfile/Utility.java
+++ b/src/main/java/org/apache/bcel/classfile/Utility.java
@@ -281,6 +281,8 @@ public abstract class Utility {
*/
public static String codeToString(final byte[] code, final ConstantPool
constantPool, final int index, final int length, final boolean verbose) {
final StringBuilder buf = new StringBuilder(code.length * 20); //
Should be sufficient // CHECKSTYLE IGNORE MagicNumber
+ // Defend against a stale flag left behind by a previous (possibly
truncated) disassembly on this thread.
+ WIDE.set(Boolean.FALSE);
try (ByteSequence stream = new ByteSequence(code)) {
for (int i = 0; i < index; i++) {
codeToString(stream, constantPool, verbose);
@@ -293,6 +295,11 @@ public abstract class Utility {
}
} catch (final IOException e) {
throw new ClassFormatException("Byte code error: " +
buf.toString(), e);
+ } finally {
+ // A crafted code array can end right after a WIDE opcode (normal
loop exit) or throw before the flag is
+ // consumed; never leak the flag to the next disassembly on this
thread, or that (unrelated) input is
+ // mis-decoded from its first load/store/iinc/ret instruction
onwards.
+ WIDE.remove();
}
return buf.toString();
}
diff --git a/src/test/java/org/apache/bcel/classfile/UtilityTest.java
b/src/test/java/org/apache/bcel/classfile/UtilityTest.java
index e3ee1b14..a7c7e22d 100644
--- a/src/test/java/org/apache/bcel/classfile/UtilityTest.java
+++ b/src/test/java/org/apache/bcel/classfile/UtilityTest.java
@@ -117,6 +117,26 @@ class UtilityTest {
assertThrows(ClassFormatException.class, () ->
Utility.codeToString(new ByteSequence(code), new ConstantPool()));
}
+ @Test
+ void testCodeToStringWideDoesNotLeakAcrossCalls() {
+ // A truncated code array ending right after a WIDE opcode must not
leave the thread-local WIDE flag
+ // set, or the next (unrelated) disassembly on the same thread is
mis-decoded.
+ final String first = Utility.codeToString(new byte[] {(byte)
Const.WIDE}, new ConstantPool(), 0, -1, false);
+ assertTrue(first.contains("wide"), first);
+ // iload with a single index byte; without the reset this reads a
16-bit index (%258) and swallows a byte.
+ final String next = Utility.codeToString(new byte[] {(byte)
Const.ILOAD, 1, 2}, new ConstantPool(), 0, -1, false);
+ assertTrue(next.contains("iload\t\t%1"), next);
+ // Same for the exceptional path: wide iload with its operand missing
throws, but must still reset the flag.
+ try {
+ Utility.codeToString(new byte[] {(byte) Const.WIDE, (byte)
Const.ILOAD}, new ConstantPool(), 0, -1, false);
+ fail("Expected ClassFormatException for a truncated wide
instruction");
+ } catch (final ClassFormatException e) {
+ // expected: the operand of the wide iload is missing
+ }
+ final String afterThrow = Utility.codeToString(new byte[] {(byte)
Const.ILOAD, 1, 2}, new ConstantPool(), 0, -1, false);
+ assertTrue(afterThrow.contains("iload\t\t%1"), afterThrow);
+ }
+
@Test
void testCodeToStringWideIsThreadLocal() throws Exception {
// A WIDE opcode disassembled on one thread must not change how the
next
