From 8c98f5a4a6f986fdc2c42b0d83c49dfd9144ffde Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Tue, 5 Nov 2019 20:07:36 -0500 Subject: [PATCH] Simplify and harden Netty handlers --- .../protocol/netty/MinecraftCipherDecoder.java | 6 +++--- .../netty/MinecraftCompressDecoder.java | 17 ++++++++++------- .../proxy/protocol/netty/MinecraftDecoder.java | 15 ++++++++------- .../netty/MinecraftVarintFrameDecoder.java | 9 +++------ 4 files changed, 24 insertions(+), 23 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCipherDecoder.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCipherDecoder.java index 2b4d0b502..ad02191e7 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCipherDecoder.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCipherDecoder.java @@ -5,10 +5,10 @@ import com.velocitypowered.natives.encryption.VelocityCipher; import com.velocitypowered.natives.util.MoreByteBufUtils; import io.netty.buffer.ByteBuf; import io.netty.channel.ChannelHandlerContext; -import io.netty.handler.codec.ByteToMessageDecoder; +import io.netty.handler.codec.MessageToMessageDecoder; import java.util.List; -public class MinecraftCipherDecoder extends ByteToMessageDecoder { +public class MinecraftCipherDecoder extends MessageToMessageDecoder { private final VelocityCipher cipher; @@ -30,7 +30,7 @@ public class MinecraftCipherDecoder extends ByteToMessageDecoder { } @Override - protected void handlerRemoved0(ChannelHandlerContext ctx) throws Exception { + public void handlerRemoved(ChannelHandlerContext ctx) throws Exception { cipher.dispose(); } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressDecoder.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressDecoder.java index 9a040dec4..ebf7f927e 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressDecoder.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressDecoder.java @@ -13,7 +13,8 @@ import java.util.List; public class MinecraftCompressDecoder extends MessageToMessageDecoder { - private static final int MAXIMUM_UNCOMPRESSED_SIZE = 2 * 1024 * 1024; // 2MiB + private static final int SOFT_MAXIMUM_UNCOMPRESSED_SIZE = 2 * 1024 * 1024; // 2MiB + private static final int HARD_MAXIMUM_UNCOMPRESSED_SIZE = 16 * 1024 * 1024; // 16MiB private final int threshold; private final VelocityCompressor compressor; @@ -25,21 +26,23 @@ public class MinecraftCompressDecoder extends MessageToMessageDecoder { @Override protected void decode(ChannelHandlerContext ctx, ByteBuf in, List out) throws Exception { - int expectedSize = ProtocolUtils.readVarInt(in); - if (expectedSize == 0) { + int claimedUncompressedSize = ProtocolUtils.readVarInt(in); + if (claimedUncompressedSize == 0) { // Strip the now-useless uncompressed size, this message is already uncompressed. out.add(in.retainedSlice()); in.skipBytes(in.readableBytes()); return; } - checkFrame(expectedSize >= threshold, "Uncompressed size %s is less than threshold %s", - expectedSize, threshold); - int initialCapacity = Math.min(expectedSize, MAXIMUM_UNCOMPRESSED_SIZE); + checkFrame(claimedUncompressedSize >= threshold, "Uncompressed size %s is less than" + + " threshold %s", claimedUncompressedSize, threshold); + int allowedMax = Math.min(claimedUncompressedSize, HARD_MAXIMUM_UNCOMPRESSED_SIZE); + int initialCapacity = Math.min(claimedUncompressedSize, SOFT_MAXIMUM_UNCOMPRESSED_SIZE); + ByteBuf compatibleIn = ensureCompatible(ctx.alloc(), compressor, in); ByteBuf uncompressed = preferredBuffer(ctx.alloc(), compressor, initialCapacity); try { - compressor.inflate(compatibleIn, uncompressed, expectedSize); + compressor.inflate(compatibleIn, uncompressed, allowedMax); out.add(uncompressed); } catch (Exception e) { uncompressed.release(); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftDecoder.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftDecoder.java index a6f616941..2cf96e84b 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftDecoder.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftDecoder.java @@ -47,20 +47,21 @@ public class MinecraftDecoder extends MessageToMessageDecoder { packet.decode(msg, direction, registry.version); } catch (Exception e) { throw new CorruptedFrameException( - "Error decoding " + packet.getClass() + " Direction " + direction - + " Protocol " + registry.version + " State " + state + " ID " + Integer - .toHexString(packetId), e); + "Error decoding " + packet.getClass() + " " + getExtraConnectionDetail(packetId), e); } if (msg.isReadable()) { - throw new CorruptedFrameException( - "Did not read full packet for " + packet.getClass() + " Direction " + direction - + " Protocol " + registry.version + " State " + state + " ID " + Integer - .toHexString(packetId)); + throw new CorruptedFrameException("Did not read full packet for " + packet.getClass() + " " + + getExtraConnectionDetail(packetId)); } out.add(packet); } } + private String getExtraConnectionDetail(int packetId) { + return "Direction " + direction + " Protocol " + registry.version + " State " + state + + " ID " + Integer.toHexString(packetId); + } + public void setProtocolVersion(ProtocolVersion protocolVersion) { this.registry = direction.getProtocolRegistry(state, protocolVersion); } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftVarintFrameDecoder.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftVarintFrameDecoder.java index 6a114075c..dd1fa5aa1 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftVarintFrameDecoder.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftVarintFrameDecoder.java @@ -27,16 +27,13 @@ public class MinecraftVarintFrameDecoder extends ByteToMessageDecoder { // Make sure reader index of length buffer is returned to the beginning in.readerIndex(origReaderIndex); int packetLength = ProtocolUtils.readVarInt(in); - if (packetLength == 0) { - return; - } - if (in.readableBytes() < packetLength) { + if (in.readableBytes() >= packetLength) { + out.add(in.readRetainedSlice(packetLength)); + } else { in.readerIndex(origReaderIndex); - return; } - out.add(in.readRetainedSlice(packetLength)); return; } }