Add protection against large network channel lists

(cherry picked from commit 162908338bbda96431ffec48249fe5101dd67d2c)

src/main/java/net/minecraftforge/common/ForgeConfig.java

@@ -93,11 +93,23 @@ public class ForgeConfig {
      * General configuration that doesn't need to be synchronized but needs to be available before server startup
      */
     public static class Common {
+        public final ForgeConfigSpec.LongValue channelListLimit;
 
         Common(ForgeConfigSpec.Builder builder) {
-            builder.comment("[DEPRECATED / NO EFFECT]: General configuration settings")
+            builder.comment("General configuration settings")
                     .push("general");
 
+            channelListLimit = builder
+                .comment(
+                    "Maximum size to use to store the channel list for a network connection.",
+                    "The other end of the connection may send as many channels as it wishes.",
+                    "Some malicious actors may use this to use up all your memory causing OutOfMemoryException",
+                    "A sane default is 1KB (1024), but may need to be expanded for larger modpacks.",
+                    "Forge will auto-adjust this if mods register their channels using our channel manager."
+                )
+                .translation("forge.configui.channellistlimit")
+                .defineInRange("channelListLimit", 1024, 1, Long.MAX_VALUE);
+
             builder.pop();
         }
     }

src/main/java/net/minecraftforge/network/ChannelBuilder.java

@@ -171,20 +171,11 @@ public class ChannelBuilder {
      * @return the {@link NetworkInstance}
      */
     private NetworkInstance createNetworkInstance() {
-        if (NetworkRegistry.lock) {
-            NetworkRegistry.LOGGER.error(NetworkRegistry.NETREGISTRY, "Attempted to register channel {} even though registry phase is over", name);
-            throw new IllegalArgumentException("Registration of impl channels is locked");
-        }
-        if (NetworkRegistry.instances.containsKey(name)) {
-            NetworkRegistry.LOGGER.error(NetworkRegistry.NETREGISTRY, "NetworkDirection channel {} already registered.", name);
-            throw new IllegalArgumentException("NetworkDirection Channel {"+ name +"} already registered");
-        }
-
         var networkInstance = new NetworkInstance(name, networkProtocolVersion,
             getClientAcceptedVersions(), getServerAcceptedVersions(),
             attributes, connectionHandler
         );
-        NetworkRegistry.instances.put(name, networkInstance);
+        NetworkRegistry.register(networkInstance);
         return networkInstance;
 
     }

src/main/java/net/minecraftforge/network/ChannelListManager.java

@@ -10,7 +10,9 @@ import io.netty.buffer.Unpooled;
 import net.minecraft.ResourceLocationException;
 import net.minecraft.network.Connection;
 import net.minecraft.network.FriendlyByteBuf;
+import net.minecraft.network.chat.Component;
 import net.minecraft.resources.ResourceLocation;
+import net.minecraftforge.common.ForgeConfig;
 import net.minecraftforge.event.ForgeEventFactory;
 import net.minecraftforge.event.network.ChannelRegistrationChangeEvent;
 import net.minecraftforge.event.network.CustomPayloadEvent;
@@ -22,6 +24,7 @@ import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashSet;
+import java.util.stream.Collectors;
 
 @ApiStatus.Internal // TODO: Decide what I want to have public, Right now it should just be add/remove channels.
 public class ChannelListManager {
@@ -39,7 +42,7 @@ public class ChannelListManager {
 
     private static void registerListener(CustomPayloadEvent evt) {
         var ctx = evt.getSource();
-        updateFrom(ctx, evt.getPayload(), ChannelRegistrationChangeEvent.Type.REGISTER);
+        updateFrom(ctx, evt.getPayload(), ChannelRegistrationChangeEvent.Type.REGISTER, ctx.isClientSide());
         ctx.setPacketHandled(true);
         // Send the client's channels to the server whenever asked. If we're in the login state then our login wrapper will unwrap for us.
         if (ctx.isClientSide())
@@ -48,7 +51,7 @@ public class ChannelListManager {
 
     private static void unregisterListener(CustomPayloadEvent evt) {
         var ctx = evt.getSource();
-        updateFrom(ctx, evt.getPayload(), ChannelRegistrationChangeEvent.Type.UNREGISTER);
+        updateFrom(ctx, evt.getPayload(), ChannelRegistrationChangeEvent.Type.UNREGISTER, ctx.isClientSide());
         ctx.setPacketHandled(true);
     }
 
@@ -100,7 +103,7 @@ public class ChannelListManager {
         channel.send(buf, connection);
     }
 
-    private static void updateFrom(CustomPayloadEvent.Context source, FriendlyByteBuf buffer, final ChannelRegistrationChangeEvent.Type changeType) {
+    private static void updateFrom(CustomPayloadEvent.Context source, FriendlyByteBuf buffer, final ChannelRegistrationChangeEvent.Type changeType, boolean isClient) {
         byte[] data = new byte[Math.max(buffer.readableBytes(), 0)];
         buffer.readBytes(data);
         var channels = new HashSet<String>();
@@ -133,11 +136,20 @@ public class ChannelListManager {
 
         ForgeEventFactory.onChannelRegistrationChange(source.getConnection(), changeType, changed);
 
+        // It is a known issue, that I don't really care about, that we don't get the true in-memory size of each channel due to character encoding.
+        // A sane limit of 1KB will be enough to catch any malicious actors even with non-8-bit strings.
         var list = NetworkContext.get(source.getConnection());
         for (var channel : changed) {
-            boolean fire = changeType == ChannelRegistrationChangeEvent.Type.UNREGISTER
-                ? list.remoteChannels.remove(channel)
-                : list.remoteChannels.add(channel);
+            boolean fire = false;
+            if (changeType == ChannelRegistrationChangeEvent.Type.UNREGISTER) {
+                fire = list.remoteChannels.remove(channel);
+                if (fire)
+                    list.remoteChannelsSize -= channel.toString().length();
+            } else {
+                fire = list.remoteChannels.add(channel);
+                if (fire)
+                    list.remoteChannelsSize += channel.toString().length();
+            }
 
             if (fire) {
                 var target = NetworkRegistry.findTarget(channel);
@@ -145,5 +157,21 @@ public class ChannelListManager {
                     target.registrationChange(changeType == ChannelRegistrationChangeEvent.Type.REGISTER);
             }
         }
+
+        if (list.remoteChannelsSize >= NetworkRegistry.getChannelMemorySize() &&  list.remoteChannelsSize >= ForgeConfig.COMMON.channelListLimit.get()) {
+            LOGGER.warn("Received too many network channels, {} channels using {} bytes, Limit is currently {}, to change edit the Forge {} in config\forge_common.toml",
+                list.remoteChannels.size(), list.remoteChannelsSize, ForgeConfig.COMMON.channelListLimit.get(), ForgeConfig.COMMON.channelListLimit.getPath().stream().collect(Collectors.joining("."))
+            );
+
+            if (isClient) {
+                // We don't want to allow the server to screw with us, so just remove the added channels
+                for (var channel : changed) {
+                    if (list.remoteChannels.remove(channel))
+                        list.remoteChannelsSize -= channel.toString().length();
+                }
+            } else {
+                source.getSender().connection.disconnect(Component.translatable("forge.server.channellistlimit.kick"));
+            }
+        }
     }
 }

src/main/java/net/minecraftforge/network/NetworkContext.java

@@ -111,6 +111,8 @@ public class NetworkContext {
 
     @ApiStatus.Internal
     Set<ResourceLocation> remoteChannels = new HashSet<>();
+    @ApiStatus.Internal
+    long remoteChannelsSize = 0;
     private Set<ResourceLocation> remoteChannelsView = Collections.unmodifiableSet(this.remoteChannels);
 
     @ApiStatus.Internal

src/main/java/net/minecraftforge/network/NetworkRegistry.java

@@ -41,7 +41,8 @@ public class NetworkRegistry {
     static final Logger LOGGER = LogManager.getLogger();
     static final Marker NETREGISTRY = MarkerManager.getMarker("NETREGISTRY");
 
-    static Map<ResourceLocation, NetworkInstance> instances = Collections.synchronizedMap(new HashMap<>());
+    private static Map<ResourceLocation, NetworkInstance> instances = Collections.synchronizedMap(new HashMap<>());
+    private static long memorySize = 0;
 
     public static boolean acceptsVanillaClientConnections() {
         return (instances.isEmpty() || listRejectedVanillaMods(n -> n.clientAcceptedVersions).isEmpty()) && DataPackRegistriesHooks.getSyncedCustomRegistries().isEmpty();
@@ -179,4 +180,30 @@ public class NetworkRegistry {
         instances.forEach((k, v) -> ret.put(k, v.getNetworkProtocolVersion()));
         return ret;
     }
+
+    static long getChannelMemorySize() {
+        return memorySize;
+    }
+
+    static void register(NetworkInstance instance) {
+        checkLock(instance);
+
+        var name = instance.getChannelName();
+        if (NetworkRegistry.instances.containsKey(name))
+            error("Channel " + name + " already registered.");
+        else
+            memorySize += name.toString().length(); // This can fail if people don't use our api. So this is a best effort.
+
+        instances.put(name, instance);
+    }
+
+    private static void checkLock(NetworkInstance instance) {
+        if (NetworkRegistry.lock)
+            error("Attempted to register channel " + instance.getChannelName() + " even though registry phase is over");
+    }
+
+    private static void error(String message) {
+        NetworkRegistry.LOGGER.error(NetworkRegistry.NETREGISTRY, message);
+        throw new IllegalArgumentException(message);
+    }
 }

src/main/resources/assets/forge/lang/en_us.json

@@ -204,6 +204,10 @@
   "forge.configgui.stabilizeDirectionGetNearest": "Stabilize Direction Get Nearest",
   "forge.configgui.stabilizeDirectionGetNearest.tooltip": "When enabled, a slightly biased Direction#getNearest calculation will be used to prevent normal fighting on 45 degree angle faces.",
 
+  "forge.configgui.channellistlimit": "Channel List Size Limit",
+  "forge.configgui.channellistlimit.tooltip": "Maximum size to use to store the channel list for a network connection. Some malicious actors may use this to use up all your memory. A sane default is 1KB (1024), but may need to be expanded for larger modpacks.",
+  "forge.server.channellistlimit.kick": "Received too many networking channels",
+
   "forge.controlsgui.shift": "SHIFT + %s",
   "forge.controlsgui.control": "CTRL + %s",
   "forge.controlsgui.control.mac": "CMD + %s",