This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 23ccbf326006c352efe6f0179295b28c4db1cdf4 Author: opencode <[email protected]> AuthorDate: Sun Oct 11 10:01:18 2026 +0200 Validate package size field in XByteBuffer package framing The package size read from the wire was used without validation. A negative size for the second or a later package could make the footer check land on the footer of the preceding package, resetting the loop start to its previous value, so countPackages() never terminated and permanently spun one of the replication receiver workers or the multicast receiver thread. A negative or overflowingly large size for the first package instead threw an ArrayIndexOutOfBoundsException when scanning for the package footer. Sizes are now rejected, consistent with a footer mismatch, when they are negative or when the package they describe cannot fit within the buffered data. The length check is done with long arithmetic so that a large size cannot overflow and wrap around to a small or negative value, and the offset guard of firstIndexOf() now also rejects negative offsets. Add tests for the livelock and exception inputs and for valid package counting. --- .../org/apache/catalina/tribes/io/XByteBuffer.java | 14 ++--- .../apache/catalina/tribes/io/TestXByteBuffer.java | 63 ++++++++++++++++++++++ 2 files changed, 71 insertions(+), 6 deletions(-) diff --git a/java/org/apache/catalina/tribes/io/XByteBuffer.java b/java/org/apache/catalina/tribes/io/XByteBuffer.java index d5f5c8dcba..baf56f4a4d 100644 --- a/java/org/apache/catalina/tribes/io/XByteBuffer.java +++ b/java/org/apache/catalina/tribes/io/XByteBuffer.java @@ -369,12 +369,14 @@ public class XByteBuffer implements Serializable { } // next 4 bytes are the size of the package int size = toInt(buf, pos); - // now the total buffer has to be long enough to hold - // START_DATA.length+4+size+END_DATA.length - pos = start + START_DATA.length + 4 + size; - if ((pos + END_DATA.length) > bufSize) { + // the size must not be negative and the total buffer has to be + // long enough to hold START_DATA.length+4+size+END_DATA.length; + // use long arithmetic so that a large size cannot overflow and + // wrap around to a negative or small value + if (size < 0 || (long) start + START_DATA.length + 4 + size + END_DATA.length > bufSize) { break; } + pos = start + START_DATA.length + 4 + size; // and finally check the footer of the package END_DATA int newpos = firstIndexOf(buf, pos, END_DATA); // mismatch, there is no package @@ -633,8 +635,8 @@ public class XByteBuffer implements Serializable { if (find.length == 0) { return result; } - if (srcOff >= src.length) { - throw new ArrayIndexOutOfBoundsException(); + if (srcOff < 0 || srcOff >= src.length) { + throw new ArrayIndexOutOfBoundsException(srcOff); } boolean found = false; int srclen = src.length; diff --git a/test/org/apache/catalina/tribes/io/TestXByteBuffer.java b/test/org/apache/catalina/tribes/io/TestXByteBuffer.java index cf7ff86cc3..0487c75950 100644 --- a/test/org/apache/catalina/tribes/io/TestXByteBuffer.java +++ b/test/org/apache/catalina/tribes/io/TestXByteBuffer.java @@ -38,4 +38,67 @@ public class TestXByteBuffer { assertThat(obj, instanceOf(String.class)); Assert.assertEquals(test, obj); } + + private static byte[] packageOf(int... message) { + XByteBuffer msg = new XByteBuffer(new byte[0], false); + for (int b : message) { + msg.append(new byte[] { (byte) b }, 0, 1); + } + return XByteBuffer.createDataPackage(new ChannelData(new byte[] { 1, 2 }, msg, System.currentTimeMillis())); + } + + private static XByteBuffer validPackageBuffer() { + return new XByteBuffer(packageOf(3), false); + } + + private static byte[] packageStub(int size) { + // A header, a size field and three filler bytes, the minimum that + // countPackages() inspects before the footer check + byte[] stub = new byte[14]; + System.arraycopy(packageOf(2), 0, stub, 0, 7); + XByteBuffer.toBytes(size, stub, 7); + stub[11] = 'x'; + stub[12] = 'y'; + stub[13] = 'z'; + return stub; + } + + @Test + public void testCountPackagesValid() throws Exception { + XByteBuffer buf = validPackageBuffer(); + Assert.assertEquals(1, buf.countPackages()); + Assert.assertEquals(1, buf.countPackages(true)); + byte[] pkg = packageOf(2, 3); + buf.append(pkg, 0, pkg.length); + Assert.assertEquals(2, buf.countPackages()); + Assert.assertEquals(1, buf.countPackages(true)); + } + + // A size field of -18 makes the footer check land on the footer of the + // preceding package. This used to reset the loop start to its previous + // value, so countPackages() never terminated. + @Test(timeout = 30000) + public void testCountPackagesNegativeSizeAfterValidPackage() throws Exception { + XByteBuffer buf = validPackageBuffer(); + byte[] stub = packageStub(-18); + buf.append(stub, 0, stub.length); + Assert.assertEquals(1, buf.countPackages()); + Assert.assertEquals(1, buf.countPackages(true)); + } + + @Test + public void testCountPackagesNegativeSize() throws Exception { + XByteBuffer buf = new XByteBuffer(4096, false); + byte[] stub = packageStub(-18); + buf.append(stub, 0, stub.length); + Assert.assertEquals(0, buf.countPackages()); + } + + @Test + public void testCountPackagesHugeSize() throws Exception { + XByteBuffer buf = new XByteBuffer(4096, false); + byte[] stub = packageStub(Integer.MAX_VALUE); + buf.append(stub, 0, stub.length); + Assert.assertEquals(0, buf.countPackages()); + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
