mirror of
https://github.com/MinecraftForge/MinecraftForge.git
synced 2026-11-12 08:17:35 +00:00
Add protection against large network channel lists
This commit is contained in:
6 files changed
+63
-7
No files matched your search
@@ -109,7 +109,11 @@ public class ClientCommandHandler {
|
||||
new CommandSource() {
|
||||
@Override
|
||||
public void sendSystemMessage(Component message) {
|
||||
mc.gui.getChat().addClientSystemMessage(message);
|
||||
var chat = mc.gui.getChat();
|
||||
if (mc.isSameThread())
|
||||
chat.addClientSystemMessage(message);
|
||||
else
|
||||
mc.execute(() -> chat.addClientSystemMessage(message));
|
||||
}
|
||||
|
||||
@Override
|
||||
|
||||
@@ -75,6 +75,7 @@ public class ForgeConfig {
|
||||
}
|
||||
|
||||
public final ForgeConfigSpec.EnumValue<MigrationHelperMode> migrationHelperMode;
|
||||
public final ForgeConfigSpec.LongValue channelListLimit;
|
||||
|
||||
Common(ForgeConfigSpec.Builder builder) {
|
||||
builder.comment("General configuration settings")
|
||||
@@ -85,6 +86,17 @@ public class ForgeConfig {
|
||||
.translation("forge.configgui.migrationHelperMode")
|
||||
.defineEnum("logLegacyTagWarnings", MigrationHelperMode.OFF);
|
||||
|
||||
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();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -9,9 +9,11 @@ import com.mojang.logging.LogUtils;
|
||||
import net.minecraft.IdentifierException;
|
||||
import net.minecraft.network.Connection;
|
||||
import net.minecraft.network.FriendlyByteBuf;
|
||||
import net.minecraft.network.chat.Component;
|
||||
import net.minecraft.network.codec.StreamCodec;
|
||||
import net.minecraft.network.protocol.common.custom.CustomPacketPayload;
|
||||
import net.minecraft.resources.Identifier;
|
||||
import net.minecraftforge.common.ForgeConfig;
|
||||
import net.minecraftforge.event.ForgeEventFactory;
|
||||
import net.minecraftforge.event.network.ChannelRegistrationChangeEvent;
|
||||
import net.minecraftforge.event.network.CustomPayloadEvent;
|
||||
@@ -26,6 +28,7 @@ import java.util.Collection;
|
||||
import java.util.Collections;
|
||||
import java.util.HashSet;
|
||||
import java.util.List;
|
||||
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 {
|
||||
@@ -70,7 +73,7 @@ public class ChannelListManager {
|
||||
}
|
||||
|
||||
private static void register(Register payload, CustomPayloadEvent.Context ctx) {
|
||||
updateFrom(ctx, payload.channels(), ChannelRegistrationChangeEvent.Type.REGISTER);
|
||||
updateFrom(ctx, payload.channels(), 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())
|
||||
@@ -78,7 +81,7 @@ public class ChannelListManager {
|
||||
}
|
||||
|
||||
private static void unregister(Unregister payload, CustomPayloadEvent.Context ctx) {
|
||||
updateFrom(ctx, payload.channels(), ChannelRegistrationChangeEvent.Type.UNREGISTER);
|
||||
updateFrom(ctx, payload.channels(), ChannelRegistrationChangeEvent.Type.UNREGISTER, ctx.isClientSide());
|
||||
ctx.setPacketHandled(true);
|
||||
}
|
||||
|
||||
@@ -145,7 +148,7 @@ public class ChannelListManager {
|
||||
return Collections.unmodifiableList(channels);
|
||||
}
|
||||
|
||||
private static void updateFrom(CustomPayloadEvent.Context source, List<String> channels, final ChannelRegistrationChangeEvent.Type changeType) {
|
||||
private static void updateFrom(CustomPayloadEvent.Context source, List<String> channels, final ChannelRegistrationChangeEvent.Type changeType, boolean isClient) {
|
||||
var changed = new HashSet<Identifier>();
|
||||
for (var channel : channels) {
|
||||
// It also says nothing about the format of channels so ignore bad channels.
|
||||
@@ -162,11 +165,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);
|
||||
@@ -174,5 +186,21 @@ public class ChannelListManager {
|
||||
target.registrationChange(channel, 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"));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -111,6 +111,8 @@ public class NetworkContext {
|
||||
|
||||
@ApiStatus.Internal
|
||||
Set<Identifier> remoteChannels = new HashSet<>();
|
||||
@ApiStatus.Internal
|
||||
long remoteChannelsSize = 0;
|
||||
private Set<Identifier> remoteChannelsView = Collections.unmodifiableSet(this.remoteChannels);
|
||||
|
||||
@ApiStatus.Internal
|
||||
|
||||
@@ -43,6 +43,7 @@ public class NetworkRegistry {
|
||||
|
||||
private static Map<Identifier, NetworkInstance> instances = Collections.synchronizedMap(new HashMap<>());
|
||||
private static Map<Identifier, NetworkInstance> byName = Collections.synchronizedMap(new HashMap<>());
|
||||
private static long memorySize = 0;
|
||||
|
||||
public static boolean acceptsVanillaClientConnections() {
|
||||
return listRejectedVanillaMods(n -> n.clientAcceptedVersions).isEmpty() && DataPackRegistriesHooks.getSyncedCustomRegistries().isEmpty();
|
||||
@@ -199,9 +200,14 @@ public class NetworkRegistry {
|
||||
if (NetworkRegistry.byName.containsKey(name))
|
||||
error("Payload name " + name + " already registered.");
|
||||
|
||||
memorySize += name.toString().length(); // This can fail if people don't use our api. So this is a best effort.
|
||||
byName.put(name, instance);
|
||||
}
|
||||
|
||||
static long getChannelMemorySize() {
|
||||
return memorySize;
|
||||
}
|
||||
|
||||
static void register(NetworkInstance instance) {
|
||||
checkLock(instance);
|
||||
|
||||
|
||||
@@ -200,6 +200,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",
|
||||
|
||||
Reference in new issue
Block a user