Also validate length before caring to invest time into processing

This commit is contained in:
Shane Freeder
2025-04-12 16:52:31 +01:00
parent 7ad06614fe
commit 74d05211d6
3 changed files with 15 additions and 5 deletions

View File

@@ -308,14 +308,17 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler {
logger.warn("A plugin message was received while the backend server was not " logger.warn("A plugin message was received while the backend server was not "
+ "ready. Channel: {}. Packet discarded.", packet.getChannel()); + "ready. Channel: {}. Packet discarded.", packet.getChannel());
} else if (PluginMessageUtil.isRegister(packet)) { } else if (PluginMessageUtil.isRegister(packet)) {
List<ChannelIdentifier> channels = PluginMessageUtil.getChannels(packet, this.player.getProtocolVersion()); List<ChannelIdentifier> channels =
PluginMessageUtil.getChannels(this.player.getClientsideChannels().size(), packet,
this.player.getProtocolVersion());
player.getClientsideChannels().addAll(channels); player.getClientsideChannels().addAll(channels);
server.getEventManager() server.getEventManager()
.fireAndForget( .fireAndForget(
new PlayerChannelRegisterEvent(player, ImmutableList.copyOf(channels))); new PlayerChannelRegisterEvent(player, ImmutableList.copyOf(channels)));
backendConn.write(packet.retain()); backendConn.write(packet.retain());
} else if (PluginMessageUtil.isUnregister(packet)) { } 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()); backendConn.write(packet.retain());
} else if (PluginMessageUtil.isMcBrand(packet)) { } else if (PluginMessageUtil.isMcBrand(packet)) {
String brand = PluginMessageUtil.readBrandMessage(packet.content()); String brand = PluginMessageUtil.readBrandMessage(packet.content());
@@ -392,7 +395,8 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler {
// Complete client switch // Complete client switch
player.getConnection().setActiveSessionHandler(StateRegistry.CONFIG); player.getConnection().setActiveSessionHandler(StateRegistry.CONFIG);
VelocityServerConnection serverConnection = player.getConnectedServer(); VelocityServerConnection serverConnection = player.getConnectedServer();
server.getEventManager().fireAndForget(new PlayerEnteredConfigurationEvent(player, serverConnection)); server.getEventManager()
.fireAndForget(new PlayerEnteredConfigurationEvent(player, serverConnection));
if (serverConnection != null) { if (serverConnection != null) {
MinecraftConnection smc = serverConnection.ensureConnected(); MinecraftConnection smc = serverConnection.ensureConnected();
CompletableFuture.runAsync(() -> { CompletableFuture.runAsync(() -> {

View File

@@ -145,7 +145,7 @@ import org.jetbrains.annotations.NotNull;
public class ConnectedPlayer implements MinecraftConnectionAssociation, Player, KeyIdentifiable, public class ConnectedPlayer implements MinecraftConnectionAssociation, Player, KeyIdentifiable,
VelocityInboundConnection { 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 = private static final PlainTextComponentSerializer PASS_THRU_TRANSLATE =
PlainTextComponentSerializer.builder().flattener(TranslatableMapper.FLATTENER).build(); PlainTextComponentSerializer.builder().flattener(TranslatableMapper.FLATTENER).build();
static final PermissionProvider DEFAULT_PERMISSIONS = s -> PermissionFunction.ALWAYS_UNDEFINED; static final PermissionProvider DEFAULT_PERMISSIONS = s -> PermissionFunction.ALWAYS_UNDEFINED;

View File

@@ -26,6 +26,7 @@ import com.velocitypowered.api.proxy.messages.ChannelIdentifier;
import com.velocitypowered.api.proxy.messages.LegacyChannelIdentifier; import com.velocitypowered.api.proxy.messages.LegacyChannelIdentifier;
import com.velocitypowered.api.proxy.messages.MinecraftChannelIdentifier; import com.velocitypowered.api.proxy.messages.MinecraftChannelIdentifier;
import com.velocitypowered.api.util.ProxyVersion; import com.velocitypowered.api.util.ProxyVersion;
import com.velocitypowered.proxy.connection.client.ConnectedPlayer;
import com.velocitypowered.proxy.protocol.ProtocolUtils; import com.velocitypowered.proxy.protocol.ProtocolUtils;
import com.velocitypowered.proxy.protocol.netty.MinecraftDecoder; import com.velocitypowered.proxy.protocol.netty.MinecraftDecoder;
import com.velocitypowered.proxy.protocol.packet.PluginMessagePacket; 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. * 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 * @param message the message to get the channels from
* @return the channels, as an immutable list * @return the channels, as an immutable list
*/ */
public static List<ChannelIdentifier> getChannels(PluginMessagePacket message, public static List<ChannelIdentifier> getChannels(int existingChannels,
PluginMessagePacket message,
ProtocolVersion protocolVersion) { ProtocolVersion protocolVersion) {
checkNotNull(message, "message"); checkNotNull(message, "message");
checkArgument(isRegister(message) || isUnregister(message), "Unknown channel type %s", checkArgument(isRegister(message) || isUnregister(message), "Unknown channel type %s",
@@ -110,7 +113,10 @@ public final class PluginMessageUtil {
return ImmutableList.of(); return ImmutableList.of();
} }
String payload = message.content().toString(StandardCharsets.UTF_8); 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"); 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<ChannelIdentifier> channelIdentifiers = ImmutableList.builderWithExpectedSize(channels.length); ImmutableList.Builder<ChannelIdentifier> channelIdentifiers = ImmutableList.builderWithExpectedSize(channels.length);
try { try {
for (String channel : channels) { for (String channel : channels) {