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 bdaca417 Utility.JavaReader escape decoding: OOB table index, bad-hex
crash, silent non-canonical aliasing (f026).
bdaca417 is described below
commit bdaca417da39c49ae98c85c6a0a46831fd24aa24
Author: Gary Gregory <[email protected]>
AuthorDate: Fri Sep 4 19:02:02 2026 -0400
Utility.JavaReader escape decoding: OOB table index, bad-hex crash,
silent non-canonical aliasing (f026).
---
src/changes/changes.xml | 1 +
.../java/org/apache/bcel/classfile/Utility.java | 53 ++++++++++++++++------
.../bcel/classfile/UtilityEncodeDecodeTest.java | 27 +++++++++++
3 files changed, 66 insertions(+), 15 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 6261e348..4eab68ca 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -111,6 +111,7 @@ The <action> type attribute can be add,update,fix,remove.
<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>
+ <action type="fix" dev="ggregory" due-to="Gary
Gregory">Utility.JavaReader escape decoding: OOB table index, bad-hex crash,
silent non-canonical aliasing (f026).</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 a4cabd32..f9e65231 100644
--- a/src/main/java/org/apache/bcel/classfile/Utility.java
+++ b/src/main/java/org/apache/bcel/classfile/Utility.java
@@ -31,6 +31,7 @@ import java.io.PrintWriter;
import java.io.Reader;
import java.io.Writer;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.List;
import java.util.zip.GZIPInputStream;
import java.util.zip.GZIPOutputStream;
@@ -49,18 +50,6 @@ import org.apache.commons.lang3.StringUtils;
// @since 6.0 methods are no longer final
public abstract class Utility {
- /*
- * Maximum nesting depth accepted by typeSignatureToString(). Signatures
are attacker-controlled bytes from untrusted class files; without a limit, a
deeply
- * nested generic signature such as "LA<LA<LA<...>;>;>;" drives one stack
frame per nesting level and kills the calling thread with a StackOverflowError.
- */
- private static final int MAX_SIGNATURE_NESTING = 512;
-
- /**
- * The maximum number of bytes that {@link #decode(String, boolean)} will
decompress. Guards against decompression bombs: the compressed input is
- * attacker-controlled and a small input can decompress to an enormous
size.
- */
- private static final int MAX_DECODED_LENGTH = 64 * 1024 * 1024;
-
/**
* Decode characters into bytes. Used by <a
href="Utility.html#decode(java.lang.String, boolean)">decode()</a>
*/
@@ -80,21 +69,35 @@ public abstract class Utility {
if (i < 0) {
return -1;
}
- if (i >= '0' && i <= '9' || i >= 'a' && i <= 'f') { // Normal
escape
+ if (isHex(i)) { // Normal escape
final int j = in.read();
if (j < 0) {
return -1;
}
- final char[] tmp = {(char) i, (char) j};
+ if (!isHex(j)) {
+ // Would otherwise reach Integer.parseInt and throw an
undeclared NumberFormatException.
+ throw new IOException("Invalid escape sequence: expected a
second hexadecimal digit after '" + ESCAPE_CHAR + (char) i + "'");
+ }
+ final char[] tmp = { (char) i, (char) j };
return Integer.parseInt(new String(tmp), 16);
}
+ if (i >= MAP_CHAR.length || MAP_CHAR[i] == UNMAPPED) {
+ // Reject instead of throwing an undeclared
ArrayIndexOutOfBoundsException (i >= 256) or silently
+ // aliasing every unmapped character to MAP_CHAR's default
slot value (the '$A' encoding).
+ throw new IOException("Invalid escape character after '" +
ESCAPE_CHAR + "': 0x" + Integer.toHexString(i));
+ }
return MAP_CHAR[i];
}
@Override
public int read(final char[] cbuf, final int off, final int len)
throws IOException {
for (int i = 0; i < len; i++) {
- cbuf[off + i] = (char) read();
+ final int ch = read();
+ if (ch < 0) {
+ // Propagate end-of-stream instead of writing (char) -1
and over-reporting the read length.
+ return i > 0 ? i : -1;
+ }
+ cbuf[off + i] = (char) ch;
}
return len;
}
@@ -145,6 +148,21 @@ public abstract class Utility {
}
}
+ /*
+ * Maximum nesting depth accepted by typeSignatureToString(). Signatures
are attacker-controlled bytes from untrusted class files; without a limit, a
deeply
+ * nested generic signature such as "LA<LA<LA<...>;>;>;" drives one stack
frame per nesting level and kills the calling thread with a StackOverflowError.
+ */
+ private static final int MAX_SIGNATURE_NESTING = 512;
+
+ /**
+ * The maximum number of bytes that {@link #decode(String, boolean)} will
decompress. Guards against decompression bombs: the compressed input is
+ * attacker-controlled and a small input can decompress to an enormous
size.
+ */
+ private static final int MAX_DECODED_LENGTH = 64 * 1024 * 1024;
+
+ /** Marker for {@link #MAP_CHAR} slots that do not correspond to a valid
special escape character. */
+ private static final int UNMAPPED = -1;
+
/*
* How many chars have been consumed during parsing in
typeSignatureToString(). Read by methodSignatureToString(). Set
* by side effect, but only internally.
@@ -168,6 +186,7 @@ public abstract class Utility {
private static final char ESCAPE_CHAR = '$';
static {
+ Arrays.fill(MAP_CHAR, UNMAPPED);
int j = 0;
for (int i = 'A'; i <= 'Z'; i++) {
CHAR_MAP[j] = i;
@@ -852,6 +871,10 @@ public abstract class Utility {
return buf.toString();
}
+ private static boolean isHex(final int i) {
+ return i >= '0' && i <= '9' || i >= 'a' && i <= 'f';
+ }
+
/**
* WARNING:
*
diff --git
a/src/test/java/org/apache/bcel/classfile/UtilityEncodeDecodeTest.java
b/src/test/java/org/apache/bcel/classfile/UtilityEncodeDecodeTest.java
index 87ceb06c..7d14f46b 100644
--- a/src/test/java/org/apache/bcel/classfile/UtilityEncodeDecodeTest.java
+++ b/src/test/java/org/apache/bcel/classfile/UtilityEncodeDecodeTest.java
@@ -20,6 +20,10 @@
package org.apache.bcel.classfile;
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.io.IOException;
import org.junit.jupiter.api.Test;
@@ -28,6 +32,29 @@ import org.junit.jupiter.api.Test;
*/
class UtilityEncodeDecodeTest {
+ @Test
+ void testDecodeRejectsMalformedEscapes() throws Exception {
+ // Second character of a hex escape was never validated: used to throw
an undeclared NumberFormatException.
+ assertThrows(IOException.class, () -> Utility.decode("$0z", false));
+ // Escape characters above 255 used to throw an undeclared
ArrayIndexOutOfBoundsException.
+ assertThrows(IOException.class, () -> Utility.decode("$\u0141",
false));
+ // Unmapped escape characters used to silently alias to the "$A"
encoding (byte 0).
+ assertThrows(IOException.class, () -> Utility.decode("$!", false));
+ // Sanity check: valid escapes still decode ('$A' is the special
escape for byte 0, '$3a' is hex 0x3a).
+ assertEquals(0, Utility.decode("$A", false)[0]);
+ assertEquals(0x3a, Utility.decode("$3a", false)[0]);
+ }
+
+ @Test
+ void testEncodeDecodeRoundTrip() throws Exception {
+ final byte[] bytes = new byte[256];
+ for (int i = 0; i < bytes.length; i++) {
+ bytes[i] = (byte) i;
+ }
+ assertArrayEquals(bytes, Utility.decode(Utility.encode(bytes, false),
false));
+ assertArrayEquals(bytes, Utility.decode(Utility.encode(bytes, true),
true));
+ }
+
/**
* Zero-filled data compresses far better than the fixed 3:1 ratio the
decode buffer used to assume; the encode/decode round trip must still hold.
*/
