From 0783b1d4e414660b5cde43d929e015ce2964c599 Mon Sep 17 00:00:00 2001 From: Nassim Jahnke Date: Sun, 15 Mar 2026 08:37:14 +0100 Subject: [PATCH] Add remaining pre-sizing checks --- .../proxy/protocol/ProtocolUtils.java | 23 +++++++++++++++++- .../packet/AvailableCommandsPacket.java | 24 +++++++++---------- .../config/ClientboundServerLinksPacket.java | 4 +--- .../packet/config/KnownPacksPacket.java | 9 +++---- 4 files changed, 40 insertions(+), 20 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 efc0ed177..898a87db3 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java @@ -415,7 +415,10 @@ public enum ProtocolUtils { */ public static int[] readIntegerArray(ByteBuf buf) { int len = readVarInt(buf); - checkArgument(len >= 0, "Got a negative-length integer array (%s)", len); + checkFrame(len >= 0, "Got a negative-length integer array (%s)", len); + checkFrame(buf.isReadable(len), + "Trying to read an array that is too long (wanted %s, only have %s)", len, + buf.readableBytes()); int[] array = new int[len]; for (int i = 0; i < len; i++) { array[i] = readVarInt(buf); @@ -535,6 +538,10 @@ public enum ProtocolUtils { */ public static String[] readStringArray(ByteBuf buf) { int length = readVarInt(buf); + checkFrame(length >= 0, "Got a negative-length array (%s)", length); + checkFrame(buf.isReadable(length), + "Trying to read an array that is too long (wanted %s, only have %s)", length, + buf.readableBytes()); String[] ret = new String[length]; for (int i = 0; i < length; i++) { ret[i] = readString(buf); @@ -646,6 +653,9 @@ public enum ProtocolUtils { checkArgument(len <= FORGE_MAX_ARRAY_LENGTH, "Cannot receive array longer than %s (got %s bytes)", FORGE_MAX_ARRAY_LENGTH, len); + checkFrame(buf.isReadable(len), + "Trying to read an array that is too long (wanted %s, only have %s)", len, + buf.readableBytes()); byte[] ret = new byte[len]; buf.readBytes(ret); @@ -844,6 +854,17 @@ public enum ProtocolUtils { writeVarInt(buf, source.ordinal()); } + /** + * Returns a pre-sized list with a max initial size of {@code Short.MAX_VALUE} + * + * @param initialCapacity expected initial capacity + * @param entry type + * @return pre-sized list + */ + public static List newList(int initialCapacity) { + return new ArrayList<>(Math.min(initialCapacity, Short.MAX_VALUE)); + } + /** * Represents the direction in which a packet flows. */ diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/AvailableCommandsPacket.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/AvailableCommandsPacket.java index 2746b23f4..6653d7633 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/AvailableCommandsPacket.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/AvailableCommandsPacket.java @@ -44,9 +44,9 @@ import io.netty.buffer.ByteBuf; import it.unimi.dsi.fastutil.objects.Object2IntLinkedOpenCustomHashMap; import it.unimi.dsi.fastutil.objects.Object2IntMap; import java.util.ArrayDeque; -import java.util.Arrays; import java.util.Deque; import java.util.Iterator; +import java.util.List; import java.util.Queue; import java.util.concurrent.CompletableFuture; import java.util.function.Predicate; @@ -85,14 +85,14 @@ public class AvailableCommandsPacket implements MinecraftPacket { @Override public void decode(ByteBuf buf, Direction direction, ProtocolVersion protocolVersion) { int commands = ProtocolUtils.readVarInt(buf); - WireNode[] wireNodes = new WireNode[commands]; + List wireNodes = ProtocolUtils.newList(commands); for (int i = 0; i < commands; i++) { - wireNodes[i] = deserializeNode(buf, i, protocolVersion); + wireNodes.add(deserializeNode(buf, i, protocolVersion)); } // Iterate over the deserialized nodes and attempt to form a graph. We also resolve any cycles // that exist. - Queue nodeQueue = new ArrayDeque<>(Arrays.asList(wireNodes)); + Queue nodeQueue = new ArrayDeque<>(wireNodes); while (!nodeQueue.isEmpty()) { boolean cycling = false; @@ -111,7 +111,7 @@ public class AvailableCommandsPacket implements MinecraftPacket { } int rootIdx = ProtocolUtils.readVarInt(buf); - rootNode = (RootCommandNode) wireNodes[rootIdx].built; + rootNode = (RootCommandNode) wireNodes.get(rootIdx).built; } @Override @@ -245,17 +245,17 @@ public class AvailableCommandsPacket implements MinecraftPacket { this.validated = false; } - void validate(WireNode[] wireNodes) { + void validate(List wireNodes) { // Ensure all children exist. Note that we delay checking if the node has been built yet; // that needs to come after this node is built. for (int child : children) { - if (child < 0 || child >= wireNodes.length) { + if (child < 0 || child >= wireNodes.size()) { throw new IllegalStateException("Node points to non-existent index " + child); } } if (redirectTo != -1) { - if (redirectTo < 0 || redirectTo >= wireNodes.length) { + if (redirectTo < 0 || redirectTo >= wireNodes.size()) { throw new IllegalStateException("Redirect node points to non-existent index " + redirectTo); } @@ -264,7 +264,7 @@ public class AvailableCommandsPacket implements MinecraftPacket { this.validated = true; } - boolean toNode(WireNode[] wireNodes) { + boolean toNode(List wireNodes) { if (!this.validated) { this.validate(wireNodes); } @@ -280,7 +280,7 @@ public class AvailableCommandsPacket implements MinecraftPacket { // Add any redirects if (redirectTo != -1) { - WireNode redirect = wireNodes[redirectTo]; + WireNode redirect = wireNodes.get(redirectTo); if (redirect.built != null) { args.redirect(redirect.built); } else { @@ -304,7 +304,7 @@ public class AvailableCommandsPacket implements MinecraftPacket { } for (int child : children) { - if (wireNodes[child].built == null) { + if (wireNodes.get(child).built == null) { // The child is not yet deserialized. The node can't be built now. return false; } @@ -312,7 +312,7 @@ public class AvailableCommandsPacket implements MinecraftPacket { // Associate children with nodes for (int child : children) { - CommandNode childNode = wireNodes[child].built; + CommandNode childNode = wireNodes.get(child).built; if (!(childNode instanceof RootCommandNode)) { built.addChild(childNode); } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/config/ClientboundServerLinksPacket.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/config/ClientboundServerLinksPacket.java index d37866d86..e30c8a7d2 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/config/ClientboundServerLinksPacket.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/config/ClientboundServerLinksPacket.java @@ -18,13 +18,11 @@ package com.velocitypowered.proxy.protocol.packet.config; import com.velocitypowered.api.network.ProtocolVersion; -import com.velocitypowered.api.util.ServerLink; import com.velocitypowered.proxy.connection.MinecraftSessionHandler; import com.velocitypowered.proxy.protocol.MinecraftPacket; import com.velocitypowered.proxy.protocol.ProtocolUtils; import com.velocitypowered.proxy.protocol.packet.chat.ComponentHolder; import io.netty.buffer.ByteBuf; -import java.util.ArrayList; import java.util.List; public class ClientboundServerLinksPacket implements MinecraftPacket { @@ -42,7 +40,7 @@ public class ClientboundServerLinksPacket implements MinecraftPacket { public void decode(ByteBuf buf, ProtocolUtils.Direction direction, ProtocolVersion version) { int linksCount = ProtocolUtils.readVarInt(buf); - this.serverLinks = new ArrayList<>(linksCount); + this.serverLinks = ProtocolUtils.newList(linksCount); for (int i = 0; i < linksCount; i++) { serverLinks.add(ServerLink.read(buf, version)); } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/config/KnownPacksPacket.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/config/KnownPacksPacket.java index b3fb0de4f..196e87678 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/config/KnownPacksPacket.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/config/KnownPacksPacket.java @@ -23,6 +23,7 @@ import com.velocitypowered.proxy.protocol.MinecraftPacket; import com.velocitypowered.proxy.protocol.ProtocolUtils; import com.velocitypowered.proxy.util.except.QuietDecoderException; import io.netty.buffer.ByteBuf; +import java.util.List; public class KnownPacksPacket implements MinecraftPacket { @@ -30,7 +31,7 @@ public class KnownPacksPacket implements MinecraftPacket { private static final QuietDecoderException TOO_MANY_PACKS = new QuietDecoderException("too many known packs"); - private KnownPack[] packs; + private List packs; @Override public void decode(ByteBuf buf, ProtocolUtils.Direction direction, @@ -40,10 +41,10 @@ public class KnownPacksPacket implements MinecraftPacket { throw TOO_MANY_PACKS; } - final KnownPack[] packs = new KnownPack[packCount]; + final List packs = ProtocolUtils.newList(packCount); for (int i = 0; i < packCount; i++) { - packs[i] = KnownPack.read(buf); + packs.add(KnownPack.read(buf)); } this.packs = packs; @@ -52,7 +53,7 @@ public class KnownPacksPacket implements MinecraftPacket { @Override public void encode(ByteBuf buf, ProtocolUtils.Direction direction, ProtocolVersion protocolVersion) { - ProtocolUtils.writeVarInt(buf, packs.length); + ProtocolUtils.writeVarInt(buf, packs.size()); for (KnownPack pack : packs) { pack.write(buf);