diff --git a/proxy/src/main/java/com/velocitypowered/proxy/network/ProtocolUtils.java b/proxy/src/main/java/com/velocitypowered/proxy/network/ProtocolUtils.java index 411b1c74f..fd84e1e50 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/network/ProtocolUtils.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/network/ProtocolUtils.java @@ -398,14 +398,15 @@ public enum ProtocolUtils { * @param buf the buffer to read from * @return the read byte array */ - public static byte[] readByteArray17(ByteBuf buf) { + public static byte[] readByteArray17(ByteBuf buf, int cap) { // Read in a 2 or 3 byte number that represents the length of the packet. (3 byte "shorts" for // Forge only) // No vanilla packet should give a 3 byte packet int len = readExtendedForgeShort(buf); - checkArgument(len <= FORGE_MAX_ARRAY_LENGTH, - "Cannot receive array longer than %s (got %s bytes)", FORGE_MAX_ARRAY_LENGTH, len); + int maximumCap = Math.min(FORGE_MAX_ARRAY_LENGTH, cap); + checkArgument(len <= maximumCap, + "Cannot receive array longer than %s (got %s bytes)", maximumCap, len); byte[] ret = new byte[len]; buf.readBytes(ret); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/network/packet/AbstractStatusPingPacket.java b/proxy/src/main/java/com/velocitypowered/proxy/network/packet/AbstractStatusPingPacket.java index 2c31ad1ad..10e26b9f4 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/network/packet/AbstractStatusPingPacket.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/network/packet/AbstractStatusPingPacket.java @@ -18,13 +18,28 @@ package com.velocitypowered.proxy.network.packet; import com.google.common.base.MoreObjects; +import com.velocitypowered.api.network.ProtocolVersion; +import io.netty.buffer.ByteBuf; import java.util.function.LongFunction; public abstract class AbstractStatusPingPacket implements Packet { protected static

PacketReader

decoder(final LongFunction

factory) { - return (buf, version) -> { - final long randomId = buf.readLong(); - return factory.apply(randomId); + return new PacketReader

() { + @Override + public P read(ByteBuf buf, ProtocolVersion version) { + final long randomId = buf.readLong(); + return factory.apply(randomId); + } + + @Override + public int expectedMaxLength(ByteBuf buf, ProtocolVersion version) { + return 8; + } + + @Override + public int expectedMinLength(ByteBuf buf, ProtocolVersion version) { + return 8; + } }; } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/network/packet/Packet.java b/proxy/src/main/java/com/velocitypowered/proxy/network/packet/Packet.java index 6c7cf8dd5..89740e4e4 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/network/packet/Packet.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/network/packet/Packet.java @@ -33,13 +33,4 @@ public interface Packet { } boolean handle(PacketHandler handler); - - // TODO: Move this into decoder - default int expectedMinLength(ByteBuf buf, PacketDirection direction, ProtocolVersion version) { - return 0; - } - - default int expectedMaxLength(ByteBuf buf, PacketDirection direction, ProtocolVersion version) { - return -1; - } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/network/packet/PacketReader.java b/proxy/src/main/java/com/velocitypowered/proxy/network/packet/PacketReader.java index f8bb56852..2c0bf0a9c 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/network/packet/PacketReader.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/network/packet/PacketReader.java @@ -24,11 +24,11 @@ import java.util.function.Supplier; public interface PacketReader

{ P read(final ByteBuf buf, final ProtocolVersion version); - default int expectedMinLength(ByteBuf buf, PacketDirection direction, ProtocolVersion version) { + default int expectedMinLength(ByteBuf buf, ProtocolVersion version) { return 0; } - default int expectedMaxLength(ByteBuf buf, PacketDirection direction, ProtocolVersion version) { + default int expectedMaxLength(ByteBuf buf, ProtocolVersion version) { return -1; } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/network/packet/clientbound/ClientboundEncryptionRequestPacket.java b/proxy/src/main/java/com/velocitypowered/proxy/network/packet/clientbound/ClientboundEncryptionRequestPacket.java index 08fe4f4f1..7f7818f8b 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/network/packet/clientbound/ClientboundEncryptionRequestPacket.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/network/packet/clientbound/ClientboundEncryptionRequestPacket.java @@ -60,8 +60,8 @@ public class ClientboundEncryptionRequestPacket implements Packet { publicKey = ProtocolUtils.readByteArray(buf, 256); verifyToken = ProtocolUtils.readByteArray(buf, 16); } else { - publicKey = ProtocolUtils.readByteArray17(buf); - verifyToken = ProtocolUtils.readByteArray17(buf); + publicKey = ProtocolUtils.readByteArray17(buf, 256); + verifyToken = ProtocolUtils.readByteArray17(buf, 16); } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/network/packet/serverbound/ServerboundEncryptionResponsePacket.java b/proxy/src/main/java/com/velocitypowered/proxy/network/packet/serverbound/ServerboundEncryptionResponsePacket.java index 31990f984..e10835cd5 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/network/packet/serverbound/ServerboundEncryptionResponsePacket.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/network/packet/serverbound/ServerboundEncryptionResponsePacket.java @@ -27,17 +27,34 @@ import com.velocitypowered.proxy.network.packet.PacketWriter; import io.netty.buffer.ByteBuf; public class ServerboundEncryptionResponsePacket implements Packet { - public static final PacketReader DECODER = (buf, version) -> { - final byte[] sharedSecret; - final byte[] verifyToken; - if (version.gte(ProtocolVersion.MINECRAFT_1_8)) { - sharedSecret = ProtocolUtils.readByteArray(buf, 256); - verifyToken = ProtocolUtils.readByteArray(buf, 128); - } else { - sharedSecret = ProtocolUtils.readByteArray17(buf); - verifyToken = ProtocolUtils.readByteArray17(buf); + public static final PacketReader DECODER = new PacketReader<>() { + @Override + public ServerboundEncryptionResponsePacket read(ByteBuf buf, + ProtocolVersion version) { + final byte[] sharedSecret; + final byte[] verifyToken; + if (version.gte(ProtocolVersion.MINECRAFT_1_8)) { + sharedSecret = ProtocolUtils.readByteArray(buf, 128); + verifyToken = ProtocolUtils.readByteArray(buf, 128); + } else { + sharedSecret = ProtocolUtils.readByteArray17(buf, 128); + verifyToken = ProtocolUtils.readByteArray17(buf, 128); + } + return new ServerboundEncryptionResponsePacket(sharedSecret, verifyToken); + } + + @Override + public int expectedMaxLength(ByteBuf buf, ProtocolVersion version) { + // Both arrays are always 128 bytes long (due to padding), and each array has 2 bytes of + // padding (which applies to 1.7 since the length is written as a short and applies to 1.8+ + // as 128 encodes as a 2-byte VarInt). + return 260; + } + + @Override + public int expectedMinLength(ByteBuf buf, ProtocolVersion version) { + return expectedMaxLength(buf, version); } - return new ServerboundEncryptionResponsePacket(sharedSecret, verifyToken); }; public static final PacketWriter ENCODER = PacketWriter.deprecatedEncode(); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/network/packet/serverbound/ServerboundServerLoginPacket.java b/proxy/src/main/java/com/velocitypowered/proxy/network/packet/serverbound/ServerboundServerLoginPacket.java index f4572de72..d70c3a3c0 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/network/packet/serverbound/ServerboundServerLoginPacket.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/network/packet/serverbound/ServerboundServerLoginPacket.java @@ -18,23 +18,35 @@ package com.velocitypowered.proxy.network.packet.serverbound; import com.google.common.base.MoreObjects; +import com.velocitypowered.api.network.ProtocolVersion; import com.velocitypowered.proxy.network.ProtocolUtils; import com.velocitypowered.proxy.network.packet.Packet; import com.velocitypowered.proxy.network.packet.PacketHandler; import com.velocitypowered.proxy.network.packet.PacketReader; import com.velocitypowered.proxy.network.packet.PacketWriter; import com.velocitypowered.proxy.util.except.QuietDecoderException; +import io.netty.buffer.ByteBuf; import java.util.Objects; public class ServerboundServerLoginPacket implements Packet { private static final QuietDecoderException EMPTY_USERNAME = new QuietDecoderException("Empty username!"); - public static final PacketReader DECODER = (buf, version) -> { - final String username = ProtocolUtils.readString(buf, 16); - if (username.isEmpty()) { - throw EMPTY_USERNAME; + public static final PacketReader DECODER = new PacketReader<>() { + @Override + public ServerboundServerLoginPacket read(ByteBuf buf, ProtocolVersion version) { + final String username = ProtocolUtils.readString(buf, 16); + if (username.isEmpty()) { + throw EMPTY_USERNAME; + } + return new ServerboundServerLoginPacket(username); + } + + @Override + public int expectedMaxLength(ByteBuf buf, ProtocolVersion version) { + // Accommodate the rare (but likely malicious) use of UTF-8 usernames, since it is technically + // legal on the protocol level. + return 1 + (16 * 4); } - return new ServerboundServerLoginPacket(username); }; public static final PacketWriter ENCODER = (buf, packet, version) -> diff --git a/proxy/src/main/java/com/velocitypowered/proxy/network/pipeline/MinecraftDecoder.java b/proxy/src/main/java/com/velocitypowered/proxy/network/pipeline/MinecraftDecoder.java index e81391cb5..6f43cb46f 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/network/pipeline/MinecraftDecoder.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/network/pipeline/MinecraftDecoder.java @@ -22,6 +22,7 @@ import com.velocitypowered.api.network.ProtocolVersion; import com.velocitypowered.proxy.network.ProtocolUtils; import com.velocitypowered.proxy.network.packet.Packet; import com.velocitypowered.proxy.network.packet.PacketDirection; +import com.velocitypowered.proxy.network.packet.PacketReader; import com.velocitypowered.proxy.network.registry.packet.PacketRegistryMap; import com.velocitypowered.proxy.network.registry.protocol.ProtocolRegistry; import com.velocitypowered.proxy.network.registry.state.ProtocolStates; @@ -75,17 +76,18 @@ public class MinecraftDecoder extends ChannelInboundHandlerAdapter { int packetId = ProtocolUtils.readVarInt(buf); Packet packet; try { - packet = this.registry.readPacket(packetId, buf, this.version); + packet = this.readPacket(packetId, buf); } catch (Exception e) { throw handleDecodeFailure(e, packetId); } + if (packet == null) { buf.readerIndex(originalReaderIndex); ctx.fireChannelRead(buf); } else { try { if (buf.isReadable()) { - throw handleOverflow(packetId, buf.readerIndex(), buf.writerIndex()); + throw handleOverflow(buf.readerIndex(), buf.writerIndex()); } ctx.fireChannelRead(packet); } finally { @@ -94,33 +96,37 @@ public class MinecraftDecoder extends ChannelInboundHandlerAdapter { } } - // TODO: Reimplement this - private void doLengthSanityChecks(ByteBuf buf, int packetId, Packet packet) throws Exception { - int expectedMinLen = packet.expectedMinLength(buf, direction, version); - int expectedMaxLen = packet.expectedMaxLength(buf, direction, version); + private Packet readPacket(int packetId, ByteBuf buf) throws Exception { + PacketReader reader = this.registry.lookupReader(packetId, this.version); + if (reader == null) { + return null; + } + + int expectedMinLen = reader.expectedMinLength(buf, version); + int expectedMaxLen = reader.expectedMaxLength(buf, version); if (expectedMaxLen != -1 && buf.readableBytes() > expectedMaxLen) { - throw handleOverflow(packetId, expectedMaxLen, buf.readableBytes()); + throw handleOverflow(expectedMaxLen, buf.readableBytes()); } if (buf.readableBytes() < expectedMinLen) { - throw handleUnderflow(packetId, expectedMaxLen, buf.readableBytes()); + throw handleUnderflow(expectedMaxLen, buf.readableBytes()); } + + return reader.read(buf, version); } - private Exception handleOverflow(int packetId, int expected, int actual) { + private Exception handleOverflow(int expected, int actual) { if (DEBUG) { - Class packetClass = this.registry.lookupPacket(packetId); - return new CorruptedFrameException("Packet sent for " + packetClass + " was too " - + "big (expected " + expected + " bytes, got " + actual + " bytes)"); + return new CorruptedFrameException("Packet sent was too big (expected " + + expected + " bytes, got " + actual + " bytes)"); } else { return DECODE_FAILED; } } - private Exception handleUnderflow(int packetId, int expected, int actual) { + private Exception handleUnderflow(int expected, int actual) { if (DEBUG) { - Class packetClass = this.registry.lookupPacket(packetId); - return new CorruptedFrameException("Packet sent for " + packetClass + " was too " - + "small (expected " + expected + " bytes, got " + actual + " bytes)"); + return new CorruptedFrameException("Packet was too small (expected " + expected + + " bytes, got " + actual + " bytes)"); } else { return DECODE_FAILED; } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/network/registry/packet/DensePacketRegistryMap.java b/proxy/src/main/java/com/velocitypowered/proxy/network/registry/packet/DensePacketRegistryMap.java index e0cd1cf6a..bd8fe74e3 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/network/registry/packet/DensePacketRegistryMap.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/network/registry/packet/DensePacketRegistryMap.java @@ -97,12 +97,13 @@ public class DensePacketRegistryMap implements PacketRegistryMap { } @Override - public @Nullable Packet readPacket(int id, ByteBuf buf, ProtocolVersion version) { + public @Nullable PacketReader lookupReader(final int id, + ProtocolVersion version) { if (id < 0 || id >= this.readersById.length) { return null; } - return this.readersById[id].read(buf, version); + return this.readersById[id]; } @Override diff --git a/proxy/src/main/java/com/velocitypowered/proxy/network/registry/packet/EmptyPacketRegistryMap.java b/proxy/src/main/java/com/velocitypowered/proxy/network/registry/packet/EmptyPacketRegistryMap.java index bb73f9607..32e20ef79 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/network/registry/packet/EmptyPacketRegistryMap.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/network/registry/packet/EmptyPacketRegistryMap.java @@ -19,6 +19,7 @@ package com.velocitypowered.proxy.network.registry.packet; import com.velocitypowered.api.network.ProtocolVersion; import com.velocitypowered.proxy.network.packet.Packet; +import com.velocitypowered.proxy.network.packet.PacketReader; import io.netty.buffer.ByteBuf; import org.checkerframework.checker.nullness.qual.Nullable; @@ -31,7 +32,8 @@ public class EmptyPacketRegistryMap implements PacketRegistryMap { } @Override - public @Nullable Packet readPacket(int id, ByteBuf buf, ProtocolVersion version) { + public @Nullable PacketReader lookupReader(final int id, + ProtocolVersion version) { return null; } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/network/registry/packet/PacketRegistryMap.java b/proxy/src/main/java/com/velocitypowered/proxy/network/registry/packet/PacketRegistryMap.java index e7c9fe1d8..96aa17f0c 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/network/registry/packet/PacketRegistryMap.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/network/registry/packet/PacketRegistryMap.java @@ -19,11 +19,12 @@ package com.velocitypowered.proxy.network.registry.packet; import com.velocitypowered.api.network.ProtocolVersion; import com.velocitypowered.proxy.network.packet.Packet; +import com.velocitypowered.proxy.network.packet.PacketReader; import io.netty.buffer.ByteBuf; import org.checkerframework.checker.nullness.qual.Nullable; public interface PacketRegistryMap { - @Nullable Packet readPacket(final int id, ByteBuf buf, ProtocolVersion version); + @Nullable PacketReader lookupReader(final int id, ProtocolVersion version);

void writePacket(P packet, ByteBuf buf, ProtocolVersion version); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/network/registry/packet/RegularPacketRegistryMap.java b/proxy/src/main/java/com/velocitypowered/proxy/network/registry/packet/RegularPacketRegistryMap.java index 65291592b..092ef6e55 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/network/registry/packet/RegularPacketRegistryMap.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/network/registry/packet/RegularPacketRegistryMap.java @@ -58,12 +58,8 @@ public class RegularPacketRegistryMap implements PacketRegistryMap { } @Override - public @Nullable Packet readPacket(int id, ByteBuf buf, ProtocolVersion version) { - PacketReader reader = this.readersById.get(id); - if (reader == null) { - return null; - } - return reader.read(buf, version); + public @Nullable PacketReader lookupReader(int id, ProtocolVersion version) { + return this.readersById.get(id); } @Override diff --git a/proxy/src/main/java/com/velocitypowered/proxy/network/registry/state/PlayPacketRegistry.java b/proxy/src/main/java/com/velocitypowered/proxy/network/registry/state/PlayPacketRegistry.java index 6abece2df..2c09f4cf2 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/network/registry/state/PlayPacketRegistry.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/network/registry/state/PlayPacketRegistry.java @@ -432,16 +432,12 @@ class PlayPacketRegistry implements ProtocolRegistry { * Attempts to create a packet from the specified {@code id}. * * @param id the packet ID - * @param buf the bytebuf * @return the packet instance, or {@code null} if the ID is not registered */ @Override - public @Nullable Packet readPacket(final int id, ByteBuf buf, ProtocolVersion version) { - final PacketReader decoder = this.packetIdToReader.get(id); - if (decoder == null) { - return null; - } - return decoder.read(buf, version); + public @Nullable PacketReader lookupReader(final int id, + ProtocolVersion version) { + return this.packetIdToReader.get(id); } /**