diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java index 821c09876..8880233bb 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java @@ -86,6 +86,8 @@ import java.util.Queue; import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; import net.kyori.adventure.key.Key; import net.kyori.adventure.text.Component; import net.kyori.adventure.text.ComponentLike; @@ -102,12 +104,23 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { private static final boolean BACKPRESSURE_LOG = Boolean.getBoolean("velocity.log-server-backpressure"); + // Caps the per-connection queue used while the FML/login phases are not yet "complete". Without + // these caps, a client that never completes its handshake phase can spam plugin messages (each up + // to ~32 KiB serverbound) and grow the queue without bound. + private static final long MAX_QUEUED_LOGIN_PLUGIN_MESSAGE_BYTES = + Long.getLong("velocity.max-queued-login-plugin-message-bytes", 4L * 1024 * 1024); + private static final int MAX_QUEUED_LOGIN_PLUGIN_MESSAGES = + Integer.getInteger("velocity.max-queued-login-plugin-messages", 1024); + private static final Logger logger = LogManager.getLogger(ClientPlaySessionHandler.class); private final ConnectedPlayer player; private boolean spawned = false; private final List serverBossBars = new ArrayList<>(); private final Queue loginPluginMessages = new ConcurrentLinkedQueue<>(); + private final AtomicLong loginPluginMessagesBytes = new AtomicLong(); + private final AtomicInteger loginPluginMessagesCount = new AtomicInteger(); + private volatile boolean loginPluginMessagesOverflowed; private final VelocityServer server; private @Nullable TabCompleteRequestPacket outstandingTabComplete; private final ChatHandler chatHandler; @@ -176,9 +189,38 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { @Override public void deactivated() { player.discardChatQueue(); - for (PluginMessagePacket message : loginPluginMessages) { + PluginMessagePacket message; + while ((message = loginPluginMessages.poll()) != null) { ReferenceCountUtil.release(message); } + loginPluginMessagesBytes.set(0); + loginPluginMessagesCount.set(0); + } + + /** + * Adds a retained plugin message to the queue used while the FML/login phases are still in + * progress, enforcing the per-connection byte and count caps. Returns {@code true} if queued, + * {@code false} if the packet was released (and the player disconnected on overflow). + */ + private boolean enqueueLoginPluginMessage(PluginMessagePacket packet) { + if (loginPluginMessagesOverflowed) { + ReferenceCountUtil.release(packet); + return false; + } + int packetSize = packet.content().readableBytes(); + long newBytes = loginPluginMessagesBytes.addAndGet(packetSize); + int newCount = loginPluginMessagesCount.incrementAndGet(); + if (newBytes > MAX_QUEUED_LOGIN_PLUGIN_MESSAGE_BYTES + || newCount > MAX_QUEUED_LOGIN_PLUGIN_MESSAGES) { + loginPluginMessagesOverflowed = true; + ReferenceCountUtil.release(packet); + logger.warn("Disconnecting {}: pre-join plugin-message queue exceeded its limits " + + "({} messages, {} bytes).", player, newCount, newBytes); + player.disconnect(Component.translatable("velocity.error.plugin-message-overflow")); + return false; + } + loginPluginMessages.add(packet); + return true; } @Override @@ -359,7 +401,7 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { // // We also need to make sure to retain these packets, so they can be flushed // appropriately. - loginPluginMessages.add(packet.retain()); + enqueueLoginPluginMessage(packet.retain()); } else { // The connection is ready, send the packet now. backendConn.write(packet.retain()); @@ -374,7 +416,7 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { if (!player.getPhase().consideredComplete() || !serverConn.getPhase() .consideredComplete()) { // We're still processing the connection (see above), enqueue the packet for now. - loginPluginMessages.add(message.retain()); + enqueueLoginPluginMessage(message.retain()); } else { backendConn.write(message); } @@ -637,6 +679,8 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { while ((pm = loginPluginMessages.poll()) != null) { serverMc.delayedWrite(pm); } + loginPluginMessagesBytes.set(0); + loginPluginMessagesCount.set(0); // Clear any title from the previous server. if (player.getProtocolVersion().noLessThan(ProtocolVersion.MINECRAFT_1_8)) { @@ -869,6 +913,8 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { while ((pm = loginPluginMessages.poll()) != null) { connection.write(pm); } + loginPluginMessagesBytes.set(0); + loginPluginMessagesCount.set(0); } } } diff --git a/proxy/src/main/resources/com/velocitypowered/proxy/l10n/messages.properties b/proxy/src/main/resources/com/velocitypowered/proxy/l10n/messages.properties index 744cb68b4..31f8d1fc1 100644 --- a/proxy/src/main/resources/com/velocitypowered/proxy/l10n/messages.properties +++ b/proxy/src/main/resources/com/velocitypowered/proxy/l10n/messages.properties @@ -25,6 +25,7 @@ velocity.error.internal-server-connection-error=An internal server connection er velocity.error.logging-in-too-fast=You are logging in too fast, try again later. velocity.error.online-mode-only=You are not logged into your Minecraft account. If you are logged into your Minecraft account, try restarting your Minecraft client. velocity.error.player-connection-error=An internal error occurred in your connection. +velocity.error.plugin-message-overflow=You sent too many plugin messages before completing the connection. velocity.error.modern-forwarding-needs-new-client=This server is only compatible with Minecraft 1.13 and above. velocity.error.modern-forwarding-failed=Your server did not send a forwarding request to the proxy. Make sure the server is configured for Velocity forwarding. velocity.error.moved-to-new-server=You were kicked from :