From fd7d5c269f91a1a1bcd7a46a16636358b90a129c Mon Sep 17 00:00:00 2001 From: Worive <13164341+Worive@users.noreply.github.com> Date: Fri, 13 Nov 2026 18:20:48 +0100 Subject: [PATCH] Bound compressed packet decoding (#58) --- .../codechicken/lib/packet/PacketCustom.java | 76 +++--- .../lib/packet/PacketCustomTest.java | 218 ++++++++++++++++++ 2 files changed, 266 insertions(+), 28 deletions(-) create mode 100644 src/test/java/codechicken/lib/packet/PacketCustomTest.java diff --git a/src/main/java/codechicken/lib/packet/PacketCustom.java b/src/main/java/codechicken/lib/packet/PacketCustom.java index 52a1455..af6c5ff 100644 --- a/src/main/java/codechicken/lib/packet/PacketCustom.java +++ b/src/main/java/codechicken/lib/packet/PacketCustom.java @@ -2,6 +2,7 @@ package codechicken.lib.packet; import java.util.EnumMap; import java.util.List; +import java.util.zip.DataFormatException; import java.util.zip.Deflater; import java.util.zip.Inflater; @@ -50,6 +51,8 @@ import io.netty.util.AttributeKey; public final class PacketCustom implements MCDataInput, MCDataOutput { + private static final int MAX_DECOMPRESSED_SIZE = 2 * 1024 * 1024; + public static interface ICustomPacketHandler { } @@ -190,7 +193,7 @@ public final class PacketCustom implements MCDataInput, MCDataOutput { byteBuf = payload; type = byteBuf.readUnsignedByte(); - if (type > 0x80) decompress(); + if (type > 0x80) decompressPayload(); type &= 0x7F; } @@ -204,44 +207,61 @@ public final class PacketCustom implements MCDataInput, MCDataOutput { byteBuf.writeByte(type); } - /** - * Decompresses the remaining ByteBuf (after type has been read) using Snappy - */ - private void decompress() { + private void decompressPayload() { + if (byteBuf.readableBytes() < 4) throw new IllegalArgumentException("Missing compressed packet length"); + int declaredLength = byteBuf.readInt(); + if (declaredLength < 0 || declaredLength > MAX_DECOMPRESSED_SIZE) + throw new IllegalArgumentException("Invalid decompressed packet length: " + declaredLength); + Inflater inflater = new Inflater(); + ByteBuf decodedPayload = null; try { - int len = byteBuf.readInt(); - ByteBuf out = Unpooled.buffer(len); - inflater.setInput(byteBuf.array(), byteBuf.readerIndex(), byteBuf.readableBytes()); - inflater.inflate(out.array()); - out.writerIndex(len); - byteBuf = out; - } catch (Exception e) { - throw new RuntimeException(e); + byte[] compressedInput = new byte[byteBuf.readableBytes()]; + byteBuf.readBytes(compressedInput); + inflater.setInput(compressedInput); + decodedPayload = Unpooled.buffer(Math.min(declaredLength, 8192), declaredLength); + byte[] inflatedChunk = new byte[Math.min(declaredLength + 1, 8192)]; + while (!inflater.finished()) { + int inflatedBytes = inflater.inflate(inflatedChunk); + if (inflatedBytes > declaredLength - decodedPayload.writerIndex()) + throw new IllegalArgumentException("Decompressed packet exceeds declared length"); + if (inflatedBytes == 0 && !inflater.finished()) + throw new IllegalArgumentException("Incomplete or dictionary-dependent compressed packet"); + decodedPayload.writeBytes(inflatedChunk, 0, inflatedBytes); + } + if (decodedPayload.writerIndex() != declaredLength || inflater.getRemaining() != 0) + throw new IllegalArgumentException( + "Compressed packet does not match declared length or has trailing data"); + byteBuf = decodedPayload; + decodedPayload = null; + } catch (DataFormatException e) { + throw new IllegalArgumentException("Invalid compressed packet", e); } finally { + if (decodedPayload != null) decodedPayload.release(); inflater.end(); } } - /** - * Compresses the payload ByteBuf after the type byte - */ - private void do_compress() { + private void compressPayload() { + if ((byteBuf.getUnsignedByte(0) & 0x80) != 0) return; + + int payloadLength = byteBuf.writerIndex() - 1; + if (payloadLength > MAX_DECOMPRESSED_SIZE) + throw new IllegalArgumentException("Packet payload exceeds 2 MiB decompressed limit: " + payloadLength); + Deflater deflater = new Deflater(); try { byteBuf.readerIndex(1); - int len = byteBuf.readableBytes(); - deflater.setInput(byteBuf.array(), byteBuf.readerIndex(), len); + deflater.setInput(byteBuf.array(), byteBuf.readerIndex(), payloadLength); deflater.finish(); - ByteBuf out = Unpooled.buffer(len + 5); - int clen = deflater.deflate(out.array(), 5, len); - if (clen >= len - 5 || !deflater.finished()) // not worth compressing, gets larger - return; + ByteBuf compressedPayload = Unpooled.buffer(payloadLength + 5); + int compressedLength = deflater.deflate(compressedPayload.array(), 5, payloadLength); + if (compressedLength >= payloadLength - 5 || !deflater.finished()) return; - out.setByte(0, type | 0x80); - out.setInt(1, len); - out.writerIndex(clen + 5); - byteBuf = out; + compressedPayload.setByte(0, type | 0x80); + compressedPayload.setInt(1, payloadLength); + compressedPayload.writerIndex(compressedLength + 5); + byteBuf = compressedPayload; } catch (Exception e) { throw new RuntimeException(e); } finally { @@ -474,7 +494,7 @@ public final class PacketCustom implements MCDataInput, MCDataOutput { public FMLProxyPacket toPacket() { if (incoming()) throw new IllegalStateException("Tried to write an incoming packet"); - if (byteBuf.readableBytes() > 32000 || (type & 0x80) != 0) do_compress(); + if (byteBuf.readableBytes() > 32000 || (type & 0x80) != 0) compressPayload(); // FML packet impl returns the whole of the backing array, copy used portion of array to another ByteBuf return new FMLProxyPacket(byteBuf.copy(), channel); diff --git a/src/test/java/codechicken/lib/packet/PacketCustomTest.java b/src/test/java/codechicken/lib/packet/PacketCustomTest.java new file mode 100644 index 0000000..5ff9149 --- /dev/null +++ b/src/test/java/codechicken/lib/packet/PacketCustomTest.java @@ -0,0 +1,218 @@ +package codechicken.lib.packet; + +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.ByteArrayOutputStream; +import java.util.Arrays; +import java.util.zip.Deflater; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import cpw.mods.fml.common.network.internal.FMLProxyPacket; +import io.netty.buffer.ByteBuf; +import io.netty.buffer.Unpooled; + +public class PacketCustomTest { + + private static final int MAX_DECOMPRESSED_SIZE = 2 * 1024 * 1024; + + @Test + public void rejectsLargeDeclarationWithoutCompressedData() { + assertRejected(Unpooled.buffer(5).writeByte(0x81).writeInt(8 * 1024 * 1024)); + } + + @ParameterizedTest + @ValueSource(ints = { -1, Integer.MIN_VALUE, MAX_DECOMPRESSED_SIZE + 1, Integer.MAX_VALUE }) + public void rejectsInvalidDeclaredLengths(int length) { + assertRejected(Unpooled.buffer(5).writeByte(0x81).writeInt(length)); + } + + @Test + public void rejectsMissingCompressedDataWithinLimit() { + assertRejected(Unpooled.buffer(5).writeByte(0x81).writeInt(MAX_DECOMPRESSED_SIZE)); + } + + @Test + public void rejectsCompleteStreamAboveDecodeLimit() { + assertRejected(compressed(new byte[MAX_DECOMPRESSED_SIZE + 1])); + } + + @Test + public void rejectsTruncatedLengthHeader() { + assertRejected(Unpooled.buffer(4).writeByte(0x81).writeMedium(10)); + } + + @Test + public void rejectsMismatchedInflatedLengths() { + byte[] data = new byte[16384]; + assertRejected(compressed(data).setInt(1, data.length - 1)); + assertRejected(compressed(data).setInt(1, data.length + 1)); + assertRejected(compressed(data).setInt(1, 0)); + } + + @Test + public void rejectsTruncatedStreamEvenWhenAllOutputWasProduced() { + ByteBuf payload = compressed(new byte[16384]); + payload.writerIndex(payload.writerIndex() - 1); + assertRejected(payload); + } + + @Test + public void rejectsCorruptStream() { + ByteBuf payload = compressed(new byte[16384]); + payload.setByte(payload.writerIndex() - 1, payload.getByte(payload.writerIndex() - 1) ^ 1); + assertRejected(payload); + } + + @Test + public void rejectsDictionaryDependentStream() { + assertRejected(compressed(new byte[16384], new byte[] { 1, 2, 3 })); + } + + @Test + public void rejectsTrailingCompressedData() { + assertRejected(compressed(new byte[16384]).writeByte(0)); + } + + @ParameterizedTest + @ValueSource(ints = { 0, 1, 8192, 8193, MAX_DECOMPRESSED_SIZE }) + public void decodesCompleteStreams(int length) { + byte[] data = new byte[length]; + Arrays.fill(data, (byte) 42); + assertDecoded(compressed(data), data); + } + + @Test + public void decodesSlicedAndDirectBuffers() { + byte[] data = new byte[16384]; + Arrays.fill(data, (byte) 42); + ByteBuf payload = compressed(data); + ByteBuf prefixed = Unpooled.buffer().writeLong(-1).writeBytes(payload, 0, payload.readableBytes()); + ByteBuf direct = Unpooled.directBuffer().writeBytes(payload); + payload.release(); + try { + assertDecoded(prefixed.slice(8, prefixed.readableBytes() - 8).retain(), data); + assertDecoded(direct, data); + } finally { + prefixed.release(); + } + } + + @Test + public void preservesUncompressedPackets() { + ByteBuf payload = Unpooled.buffer().writeByte(7).writeInt(42); + try { + PacketCustom packet = new PacketCustom(payload); + assertEquals(7, packet.getType()); + assertEquals(42, packet.readInt()); + assertEquals(0, packet.getByteBuf().readableBytes()); + } finally { + payload.release(); + } + } + + @Test + public void roundTripsOutgoingCompression() { + for (int length : new int[] { 16, 64, 40000, MAX_DECOMPRESSED_SIZE }) { + byte[] data = new byte[length]; + Arrays.fill(data, (byte) 42); + PacketCustom outgoing = new PacketCustom("PacketTest", 7); + FMLProxyPacket wire = null; + try { + outgoing.writeByteArray(data); + if (length <= 64) outgoing.compress(); + wire = outgoing.toPacket(); + assertEquals(length == 16 ? 7 : 0x87, wire.payload().getUnsignedByte(0)); + assertDecoded(wire.payload().retain(), data); + } finally { + if (wire != null) wire.payload().release(); + outgoing.getByteBuf().release(); + } + } + } + + @ParameterizedTest + @ValueSource(booleans = { false, true }) + public void repeatedSerializationPreservesCompressedPayload(boolean forceCompression) { + byte[] data = new byte[40000]; + Arrays.fill(data, (byte) 42); + PacketCustom outgoing = new PacketCustom("PacketTest", 7).writeByteArray(data); + if (forceCompression) outgoing.compress(); + try { + for (int serialization = 0; serialization < 3; serialization++) { + FMLProxyPacket wire = outgoing.toPacket(); + try { + assertEquals(0x87, wire.payload().getUnsignedByte(0)); + assertDecoded(wire.payload().retain(), data); + } finally { + wire.payload().release(); + } + } + } finally { + outgoing.getByteBuf().release(); + } + } + + @Test + public void rejectsOutgoingPayloadAboveDecodeLimit() { + PacketCustom outgoing = new PacketCustom("PacketTest", 7); + try { + outgoing.getByteBuf().writeZero(MAX_DECOMPRESSED_SIZE + 1); + assertThrows(IllegalArgumentException.class, outgoing::toPacket); + } finally { + outgoing.getByteBuf().release(); + } + } + + private static ByteBuf compressed(byte[] data) { + return compressed(data, null); + } + + private static ByteBuf compressed(byte[] data, byte[] dictionary) { + Deflater deflater = new Deflater(); + try { + if (dictionary != null) deflater.setDictionary(dictionary); + deflater.setInput(data); + deflater.finish(); + ByteArrayOutputStream compressed = new ByteArrayOutputStream(); + byte[] scratch = new byte[8192]; + while (!deflater.finished()) { + int count = deflater.deflate(scratch); + compressed.write(scratch, 0, count); + } + return Unpooled.buffer().writeByte(0x87).writeInt(data.length).writeBytes(compressed.toByteArray()); + } finally { + deflater.end(); + } + } + + private static void assertDecoded(ByteBuf payload, byte[] expected) { + PacketCustom packet = null; + try { + packet = new PacketCustom(payload); + assertEquals(7, packet.getType()); + assertEquals(expected.length, packet.getByteBuf().readableBytes()); + byte[] actual = new byte[expected.length]; + packet.getByteBuf().readBytes(actual); + assertArrayEquals(expected, actual); + } finally { + if (packet != null && packet.getByteBuf() != payload) packet.getByteBuf().release(); + payload.release(); + } + } + + private static void assertRejected(ByteBuf payload) { + try { + assertThrows(IllegalArgumentException.class, () -> { + PacketCustom packet = new PacketCustom(payload); + if (packet.getByteBuf() != payload) packet.getByteBuf().release(); + }); + } finally { + payload.release(); + } + } +}