From 4f6d238b39609933a71c622c592f778c4341a754 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Fri, 29 Jan 2021 17:56:50 -0500 Subject: [PATCH] Strictly limit the hostname size and limit it to ASCII characters only --- .../proxy/protocol/ProtocolUtils.java | 20 ++++++++++++++++++- .../proxy/protocol/packet/Handshake.java | 8 +++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java index 2c4f50a99..e0259b033 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java @@ -102,7 +102,7 @@ public enum ProtocolUtils { } /** - * Reads a VarInt length-prefixed string from the {@code buf}, making sure to not go over + * Reads a VarInt length-prefixed UTF-8 string from the {@code buf}, making sure to not go over * {@code cap} size. * @param buf the buffer to read from * @param cap the maximum size of the string, in UTF-8 character length @@ -113,6 +113,24 @@ public enum ProtocolUtils { return readString(buf, cap, length); } + /** + * Reads a VarInt length-prefixed ASCII string from the {@code buf}, making sure to not go over + * {@code cap} size. This method is specialized for select parts of the Minecraft protocol where + * ASCII characters are guaranteed to be used. + * + * @param buf the buffer to read from + * @param cap the maximum size of the string, in UTF-8 character length + * @return the decoded string + */ + public static String readAsciiString(ByteBuf buf, int cap) { + int length = readVarInt(buf); + checkFrame(length >= 0, "Got a negative-length string (%s)", length); + checkFrame(length <= cap, "Bad string size (got %s, maximum is %s)", length, cap); + String str = buf.toString(buf.readerIndex(), length, StandardCharsets.US_ASCII); + buf.skipBytes(length); + return str; + } + private static String readString(ByteBuf buf, int cap, int length) { checkFrame(length >= 0, "Got a negative-length string (%s)", length); // `cap` is interpreted as a UTF-8 character length. To cover the full Unicode plane, we must diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/Handshake.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/Handshake.java index cf64b7b93..a1c902057 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/Handshake.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/Handshake.java @@ -1,13 +1,19 @@ package com.velocitypowered.proxy.protocol.packet; +import static com.velocitypowered.proxy.connection.forge.legacy.LegacyForgeConstants.HANDSHAKE_HOSTNAME_TOKEN; + import com.velocitypowered.api.network.ProtocolVersion; import com.velocitypowered.proxy.connection.MinecraftSessionHandler; +import com.velocitypowered.proxy.connection.forge.legacy.LegacyForgeConstants; import com.velocitypowered.proxy.protocol.MinecraftPacket; import com.velocitypowered.proxy.protocol.ProtocolUtils; import io.netty.buffer.ByteBuf; public class Handshake implements MinecraftPacket { + // This size was chosen to ensure Forge clients can still connect even with very long hostnames. + // While DNS technically allows any character to be used, in practice ASCII is used. + private static final int MAXIMUM_HOSTNAME_LENGTH = 255 + HANDSHAKE_HOSTNAME_TOKEN.length() + 1; private ProtocolVersion protocolVersion; private String serverAddress = ""; private int port; @@ -59,7 +65,7 @@ public class Handshake implements MinecraftPacket { public void decode(ByteBuf buf, ProtocolUtils.Direction direction, ProtocolVersion ignored) { int realProtocolVersion = ProtocolUtils.readVarInt(buf); this.protocolVersion = ProtocolVersion.getProtocolVersion(realProtocolVersion); - this.serverAddress = ProtocolUtils.readString(buf); + this.serverAddress = ProtocolUtils.readAsciiString(buf, MAXIMUM_HOSTNAME_LENGTH); this.port = buf.readUnsignedShort(); this.nextStatus = ProtocolUtils.readVarInt(buf); }