From 74d05211d685ae8aa2a221b0bb086bae51b43eb4 Mon Sep 17 00:00:00 2001 From: Shane Freeder Date: Sat, 12 Apr 2025 16:52:31 +0100 Subject: [PATCH] Also validate length before caring to invest time into processing --- .../connection/client/ClientPlaySessionHandler.java | 10 +++++++--- .../proxy/connection/client/ConnectedPlayer.java | 2 +- .../proxy/protocol/util/PluginMessageUtil.java | 8 +++++++- 3 files changed, 15 insertions(+), 5 deletions(-) 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 1fd72063..adaeb337 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 @@ -308,14 +308,17 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { logger.warn("A plugin message was received while the backend server was not " + "ready. Channel: {}. Packet discarded.", packet.getChannel()); } else if (PluginMessageUtil.isRegister(packet)) { - List channels = PluginMessageUtil.getChannels(packet, this.player.getProtocolVersion()); + List channels = + PluginMessageUtil.getChannels(this.player.getClientsideChannels().size(), packet, + this.player.getProtocolVersion()); player.getClientsideChannels().addAll(channels); server.getEventManager() .fireAndForget( new PlayerChannelRegisterEvent(player, ImmutableList.copyOf(channels))); backendConn.write(packet.retain()); } else if (PluginMessageUtil.isUnregister(packet)) { - player.getClientsideChannels().removeAll(PluginMessageUtil.getChannels(packet, this.player.getProtocolVersion())); + player.getClientsideChannels() + .removeAll(PluginMessageUtil.getChannels(0, packet, this.player.getProtocolVersion())); backendConn.write(packet.retain()); } else if (PluginMessageUtil.isMcBrand(packet)) { String brand = PluginMessageUtil.readBrandMessage(packet.content()); @@ -392,7 +395,8 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { // Complete client switch player.getConnection().setActiveSessionHandler(StateRegistry.CONFIG); VelocityServerConnection serverConnection = player.getConnectedServer(); - server.getEventManager().fireAndForget(new PlayerEnteredConfigurationEvent(player, serverConnection)); + server.getEventManager() + .fireAndForget(new PlayerEnteredConfigurationEvent(player, serverConnection)); if (serverConnection != null) { MinecraftConnection smc = serverConnection.ensureConnected(); CompletableFuture.runAsync(() -> { diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java index 8f89add5..982b9652 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java @@ -145,7 +145,7 @@ import org.jetbrains.annotations.NotNull; public class ConnectedPlayer implements MinecraftConnectionAssociation, Player, KeyIdentifiable, VelocityInboundConnection { - private static final int MAX_CLIENTSIDE_PLUGIN_CHANNELS = 1024; + public static final int MAX_CLIENTSIDE_PLUGIN_CHANNELS = 1024; private static final PlainTextComponentSerializer PASS_THRU_TRANSLATE = PlainTextComponentSerializer.builder().flattener(TranslatableMapper.FLATTENER).build(); static final PermissionProvider DEFAULT_PERMISSIONS = s -> PermissionFunction.ALWAYS_UNDEFINED; diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/util/PluginMessageUtil.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/util/PluginMessageUtil.java index fefc64c6..1936eb3f 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/util/PluginMessageUtil.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/util/PluginMessageUtil.java @@ -26,6 +26,7 @@ import com.velocitypowered.api.proxy.messages.ChannelIdentifier; import com.velocitypowered.api.proxy.messages.LegacyChannelIdentifier; import com.velocitypowered.api.proxy.messages.MinecraftChannelIdentifier; import com.velocitypowered.api.util.ProxyVersion; +import com.velocitypowered.proxy.connection.client.ConnectedPlayer; import com.velocitypowered.proxy.protocol.ProtocolUtils; import com.velocitypowered.proxy.protocol.netty.MinecraftDecoder; import com.velocitypowered.proxy.protocol.packet.PluginMessagePacket; @@ -96,10 +97,12 @@ public final class PluginMessageUtil { /** * Fetches all the channels in a register or unregister plugin message. * + * @param existingChannels the number of channels already registered * @param message the message to get the channels from * @return the channels, as an immutable list */ - public static List getChannels(PluginMessagePacket message, + public static List getChannels(int existingChannels, + PluginMessagePacket message, ProtocolVersion protocolVersion) { checkNotNull(message, "message"); checkArgument(isRegister(message) || isUnregister(message), "Unknown channel type %s", @@ -110,7 +113,10 @@ public final class PluginMessageUtil { return ImmutableList.of(); } String payload = message.content().toString(StandardCharsets.UTF_8); + checkArgument(payload.length() <= Short.MAX_VALUE, "payload too long: %s", payload.length()); String[] channels = payload.split("\0"); + checkArgument(existingChannels + channels.length <= ConnectedPlayer.MAX_CLIENTSIDE_PLUGIN_CHANNELS, + "too many channels: %s + %s > %s", existingChannels, channels.length, ConnectedPlayer.MAX_CLIENTSIDE_PLUGIN_CHANNELS); ImmutableList.Builder channelIdentifiers = ImmutableList.builderWithExpectedSize(channels.length); try { for (String channel : channels) {