Validate handshake packet length early

proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java

@@ -46,6 +46,7 @@ import com.velocitypowered.proxy.protocol.netty.MinecraftCompressDecoder;
 import com.velocitypowered.proxy.protocol.netty.MinecraftCompressorAndLengthEncoder;
 import com.velocitypowered.proxy.protocol.netty.MinecraftDecoder;
 import com.velocitypowered.proxy.protocol.netty.MinecraftEncoder;
+import com.velocitypowered.proxy.protocol.netty.MinecraftVarintFrameDecoder;
 import com.velocitypowered.proxy.protocol.netty.MinecraftVarintLengthEncoder;
 import com.velocitypowered.proxy.protocol.netty.PlayPacketQueueInboundHandler;
 import com.velocitypowered.proxy.protocol.netty.PlayPacketQueueOutboundHandler;
@@ -368,6 +369,11 @@ public class MinecraftConnection extends ChannelInboundHandlerAdapter {
     ensureInEventLoop();
 
     this.state = state;
+    final MinecraftVarintFrameDecoder frameDecoder = this.channel.pipeline()
+        .get(MinecraftVarintFrameDecoder.class);
+    if (frameDecoder != null) {
+      frameDecoder.setState(state);
+    }
     // If the connection is LEGACY (<1.6), the decoder and encoder are not set.
     final MinecraftEncoder minecraftEncoder = this.channel.pipeline()
         .get(MinecraftEncoder.class);

proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java

@@ -47,6 +47,7 @@ import com.velocitypowered.proxy.connection.util.ConnectionMessages;
 import com.velocitypowered.proxy.protocol.MinecraftPacket;
 import com.velocitypowered.proxy.protocol.StateRegistry;
 import com.velocitypowered.proxy.protocol.netty.MinecraftDecoder;
+import com.velocitypowered.proxy.protocol.netty.MinecraftVarintFrameDecoder;
 import com.velocitypowered.proxy.protocol.packet.AvailableCommandsPacket;
 import com.velocitypowered.proxy.protocol.packet.BossBarPacket;
 import com.velocitypowered.proxy.protocol.packet.BundleDelimiterPacket;
@@ -149,6 +150,7 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler {
     MinecraftConnection smc = serverConn.ensureConnected();
     smc.setAutoReading(false);
     // Even when not auto reading messages are still decoded. Decode them with the correct state
+    smc.getChannel().pipeline().get(MinecraftVarintFrameDecoder.class).setState(StateRegistry.CONFIG);
     smc.getChannel().pipeline().get(MinecraftDecoder.class).setState(StateRegistry.CONFIG);
     serverConn.getPlayer().switchToConfigState();
     return true;

proxy/src/main/java/com/velocitypowered/proxy/connection/backend/ConfigSessionHandler.java

@@ -40,6 +40,7 @@ import com.velocitypowered.proxy.connection.util.ConnectionRequestResults.Impl;
 import com.velocitypowered.proxy.protocol.MinecraftPacket;
 import com.velocitypowered.proxy.protocol.StateRegistry;
 import com.velocitypowered.proxy.protocol.netty.MinecraftDecoder;
+import com.velocitypowered.proxy.protocol.netty.MinecraftVarintFrameDecoder;
 import com.velocitypowered.proxy.protocol.packet.ClientboundCookieRequestPacket;
 import com.velocitypowered.proxy.protocol.packet.ClientboundStoreCookiePacket;
 import com.velocitypowered.proxy.protocol.packet.DisconnectPacket;
@@ -232,6 +233,7 @@ public class ConfigSessionHandler implements MinecraftSessionHandler {
     final ConnectedPlayer player = serverConn.getPlayer();
     final ClientConfigSessionHandler configHandler = (ClientConfigSessionHandler) player.getConnection().getActiveSessionHandler();
 
+    smc.getChannel().pipeline().get(MinecraftVarintFrameDecoder.class).setState(StateRegistry.PLAY);
     smc.getChannel().pipeline().get(MinecraftDecoder.class).setState(StateRegistry.PLAY);
     //noinspection DataFlowIssue
     configHandler.handleBackendFinishUpdate(serverConn).thenRunAsync(() -> {

proxy/src/main/java/com/velocitypowered/proxy/network/BackendChannelInitializer.java

@@ -51,7 +51,7 @@ public class BackendChannelInitializer extends ChannelInitializer<Channel> {
   @Override
   protected void initChannel(Channel ch) {
     ch.pipeline()
-        .addLast(FRAME_DECODER, new MinecraftVarintFrameDecoder())
+        .addLast(FRAME_DECODER, new MinecraftVarintFrameDecoder(ProtocolUtils.Direction.CLIENTBOUND))
         .addLast(READ_TIMEOUT,
             new ReadTimeoutHandler(server.getConfiguration().getReadTimeout(),
                 TimeUnit.MILLISECONDS))

proxy/src/main/java/com/velocitypowered/proxy/network/ServerChannelInitializer.java

@@ -58,7 +58,7 @@ public class ServerChannelInitializer extends ChannelInitializer<Channel> {
   protected void initChannel(final Channel ch) {
     ch.pipeline()
         .addLast(LEGACY_PING_DECODER, new LegacyPingDecoder())
-        .addLast(FRAME_DECODER, new MinecraftVarintFrameDecoder())
+        .addLast(FRAME_DECODER, new MinecraftVarintFrameDecoder(ProtocolUtils.Direction.SERVERBOUND))
         .addLast(READ_TIMEOUT,
             new ReadTimeoutHandler(this.server.getConfiguration().getReadTimeout(),
                 TimeUnit.MILLISECONDS))

proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftVarintFrameDecoder.java

@@ -19,10 +19,16 @@ package com.velocitypowered.proxy.protocol.netty;
 
 import static io.netty.util.ByteProcessor.FIND_NON_NUL;
 
+import com.velocitypowered.api.network.ProtocolVersion;
+import com.velocitypowered.proxy.protocol.MinecraftPacket;
+import com.velocitypowered.proxy.protocol.ProtocolUtils;
+import com.velocitypowered.proxy.protocol.StateRegistry;
 import com.velocitypowered.proxy.util.except.QuietDecoderException;
+import com.velocitypowered.proxy.util.except.QuietRuntimeException;
 import io.netty.buffer.ByteBuf;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.handler.codec.ByteToMessageDecoder;
+import io.netty.handler.codec.CorruptedFrameException;
 import java.util.List;
 
 /**
@@ -30,10 +36,31 @@ import java.util.List;
  */
 public class MinecraftVarintFrameDecoder extends ByteToMessageDecoder {
 
+  private static final QuietRuntimeException FRAME_DECODER_FAILED =
+      new QuietRuntimeException("A packet frame decoder failed. For more information, launch "
+          + "Velocity with -Dvelocity.packet-decode-logging=true to see more.");
   private static final QuietDecoderException BAD_PACKET_LENGTH =
       new QuietDecoderException("Bad packet length");
   private static final QuietDecoderException VARINT_TOO_BIG =
       new QuietDecoderException("VarInt too big");
+  private static final QuietDecoderException UNKNOWN_PACKET =
+      new QuietDecoderException("Unknown packet");
+
+  private final ProtocolUtils.Direction direction;
+  private final StateRegistry.PacketRegistry.ProtocolRegistry registry;
+  private StateRegistry state;
+
+  /**
+   * Creates a new {@code MinecraftVarintFrameDecoder} decoding packets from the specified {@code Direction}.
+   *
+   * @param direction the direction from which we decode from
+   */
+  public MinecraftVarintFrameDecoder(ProtocolUtils.Direction direction) {
+    this.direction = direction;
+    this.registry = StateRegistry.HANDSHAKE.getProtocolRegistry(
+        direction, ProtocolVersion.MINIMUM_VERSION);
+    this.state = StateRegistry.HANDSHAKE;
+  }
 
   @Override
   protected void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out)
@@ -62,6 +89,38 @@ public class MinecraftVarintFrameDecoder extends ByteToMessageDecoder {
       throw BAD_PACKET_LENGTH;
     }
 
+    if (length > 0) {
+      if (state == StateRegistry.HANDSHAKE && direction == ProtocolUtils.Direction.SERVERBOUND) {
+        StateRegistry.PacketRegistry.ProtocolRegistry registry =
+            state.getProtocolRegistry(direction, ProtocolVersion.MINIMUM_VERSION);
+
+        final int index = in.readerIndex();
+        final int packetId = ProtocolUtils.readVarInt(in);
+        final int payloadLength = length - ProtocolUtils.varIntBytes(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 '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.expectedMinLength(in, direction, registry.version);
+        int expectedMaxLen = packet.expectedMaxLength(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);
+      }
+    }
+
     // note that zero-length packets are ignored
     if (length > 0) {
       if (in.readableBytes() < length) {
@@ -141,4 +200,26 @@ public class MinecraftVarintFrameDecoder extends ByteToMessageDecoder {
     }
     return result | (tmp & 0x7F) << 14;
   }
+
+  private Exception handleOverflow(MinecraftPacket packet, int expected, int actual) {
+    if (MinecraftDecoder.DEBUG) {
+      return new CorruptedFrameException("Packet sent for " + packet.getClass() + " was too "
+          + "big (expected " + expected + " bytes, got " + actual + " bytes)");
+    } else {
+      return FRAME_DECODER_FAILED;
+    }
+  }
+
+  private Exception handleUnderflow(MinecraftPacket packet, int expected, int actual) {
+    if (MinecraftDecoder.DEBUG) {
+      return new CorruptedFrameException("Packet sent for " + packet.getClass() + " was too "
+          + "small (expected " + expected + " bytes, got " + actual + " bytes)");
+    } else {
+      return FRAME_DECODER_FAILED;
+    }
+  }
+
+  public void setState(StateRegistry stateRegistry) {
+    this.state = stateRegistry;
+  }
 }

proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/HandshakePacket.java

@@ -106,4 +106,16 @@ public class HandshakePacket implements MinecraftPacket {
   public boolean handle(MinecraftSessionHandler handler) {
     return handler.handle(this);
   }
+
+  @Override
+  public int expectedMinLength(ByteBuf buf, ProtocolUtils.Direction direction,
+                               ProtocolVersion version) {
+    return 7;
+  }
+
+  @Override
+  public int expectedMaxLength(ByteBuf buf, ProtocolUtils.Direction direction,
+                               ProtocolVersion version) {
+    return 270;
+  }
 }

proxy/src/main/java/com/velocitypowered/proxy/server/VelocityRegisteredServer.java

@@ -114,7 +114,7 @@ public class VelocityRegisteredServer implements RegisteredServer, ForwardingAud
     server.createBootstrap(loop).handler(new ChannelInitializer<>() {
       @Override
       protected void initChannel(Channel ch) {
-        ch.pipeline().addLast(FRAME_DECODER, new MinecraftVarintFrameDecoder())
+        ch.pipeline().addLast(FRAME_DECODER, new MinecraftVarintFrameDecoder(ProtocolUtils.Direction.CLIENTBOUND))
             .addLast(READ_TIMEOUT, new ReadTimeoutHandler(
                 pingOptions.getTimeout() == 0
                     ? server.getConfiguration().getReadTimeout()