Fix ByteBuf memory leak in MinecraftVarintFrameDecoder (#1715)

- Reset buffer reader index on exception to prevent memory leaks when packet decoding fails.
This commit is contained in:
mason
2026-01-21 10:56:22 -08:00
committed by GitHub
parent 21671daebe
commit 3022793418

View File

@@ -91,29 +91,35 @@ public class MinecraftVarintFrameDecoder extends ByteToMessageDecoder {
// try to read the length of the packet
in.markReaderIndex();
int length = readRawVarInt21(in);
if (packetStart == in.readerIndex()) {
return;
}
if (length < 0) {
throw BAD_PACKET_LENGTH;
}
try {
int length = readRawVarInt21(in);
if (packetStart == in.readerIndex()) {
return;
}
if (length < 0) {
throw BAD_PACKET_LENGTH;
}
if (length > 0) {
if (state == StateRegistry.HANDSHAKE && direction == ProtocolUtils.Direction.SERVERBOUND) {
if (validateServerboundHandshakePacket(in, length)) {
return;
if (length > 0) {
if (state == StateRegistry.HANDSHAKE && direction == ProtocolUtils.Direction.SERVERBOUND) {
if (validateServerboundHandshakePacket(in, length)) {
return;
}
}
}
}
// note that zero-length packets are ignored
if (length > 0) {
if (in.readableBytes() < length) {
in.resetReaderIndex();
} else {
out.add(in.readRetainedSlice(length));
// note that zero-length packets are ignored
if (length > 0) {
if (in.readableBytes() < length) {
in.resetReaderIndex();
} else {
out.add(in.readRetainedSlice(length));
}
}
} catch (Exception e) {
// Reset buffer to consistent state before propagating exception to prevent memory leaks
in.resetReaderIndex();
throw e;
}
}
@@ -122,34 +128,40 @@ public class MinecraftVarintFrameDecoder extends ByteToMessageDecoder {
state.getProtocolRegistry(direction, ProtocolVersion.MINIMUM_VERSION);
final int index = in.readerIndex();
final int packetId = readRawVarInt21(in);
// Index hasn't changed, we've read nothing
if (index == in.readerIndex()) {
in.resetReaderIndex();
return true;
}
final int payloadLength = length - ProtocolUtils.varIntBytes(packetId);
try {
final int packetId = readRawVarInt21(in);
// Index hasn't changed, we've read nothing
if (index == in.readerIndex()) {
in.resetReaderIndex();
return true;
}
final int payloadLength = length - ProtocolUtils.varIntBytes(packetId);
MinecraftPacket packet = registry.createPacket(packetId);
MinecraftPacket packet = registry.createPacket(packetId);
// We handle every packet in this phase, if you said something we don't know, something is really wrong
if (packet == null) {
throw UNKNOWN_PACKET;
}
// We handle every packet in this phase, if you said something we don't know, something is really wrong
if (packet == null) {
throw UNKNOWN_PACKET;
}
// We 'technically' have the incoming bytes of a payload here, and so, these can actually parse
// the packet if needed, so, we'll take advantage of the existing methods
int expectedMinLen = packet.decodeExpectedMinLength(in, direction, registry.version);
int expectedMaxLen = packet.decodeExpectedMaxLength(in, direction, registry.version);
if (expectedMaxLen != -1 && payloadLength > expectedMaxLen) {
throw handleOverflow(packet, expectedMaxLen, in.readableBytes());
}
if (payloadLength < expectedMinLen) {
throw handleUnderflow(packet, expectedMaxLen, in.readableBytes());
}
// We 'technically' have the incoming bytes of a payload here, and so, these can actually parse
// the packet if needed, so, we'll take advantage of the existing methods
int expectedMinLen = packet.decodeExpectedMinLength(in, direction, registry.version);
int expectedMaxLen = packet.decodeExpectedMaxLength(in, direction, registry.version);
if (expectedMaxLen != -1 && payloadLength > expectedMaxLen) {
throw handleOverflow(packet, expectedMaxLen, in.readableBytes());
}
if (payloadLength < expectedMinLen) {
throw handleUnderflow(packet, expectedMaxLen, in.readableBytes());
}
in.readerIndex(index);
return false;
in.readerIndex(index);
return false;
} catch (Exception e) {
// Reset buffer to consistent state before propagating exception to prevent memory leaks
in.readerIndex(index);
throw e;
}
}
@Override