Security and stability fixes from CloudburstMC/Network

- Fix RakSessionCodec#sendStaleDatagrams() undefined behavior
- Use sane split packet limits
Co-authored-by: Koncloud <138350973+Koncloud@users.noreply.github.com>
Co-authored-by: SupremeMortal
SupremeMortal <owen@ziax.com>

Signed-off-by: GitHub <noreply@github.com>

Author: Kas-tle

transport-raknet/src/main/java/org/cloudburstmc/netty/handler/codec/raknet/common/RakSessionCodec.java

@@ -36,6 +36,7 @@
 import java.net.Inet6Address;
 import java.net.InetSocketAddress;
 import java.util.ArrayDeque;
+import java.util.Iterator;
 import java.util.Queue;
 import java.util.concurrent.TimeUnit;
 
@@ -183,6 +184,13 @@ private void initHeapWeights() {
 
     @Override
     public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise) {
+        if (!this.channel.parent().eventLoop().inEventLoop()) {
+            // Make sure this runs on correct thread
+            log.error("Tried to write packet from wrong thread: {}", Thread.currentThread().getName(), new Throwable());
+            final Object finalMsg = msg;
+            this.channel.parent().eventLoop().execute(() -> this.write(ctx, finalMsg, promise));
+            return;
+        }
         if (msg instanceof ByteBuf) {
             msg = new RakMessage((ByteBuf) msg);
         } else if (!(msg instanceof RakMessage)) {
@@ -521,7 +529,7 @@ private void onIncomingNack(ChannelHandlerContext ctx, RakDatagramPacket datagra
         }
 
         this.slidingWindow.onNak(); // TODO: verify this
-        this.sendDatagram(ctx, datagram, curTime);
+        this.sendDatagram(ctx, datagram, curTime, this.sentDatagrams);
     }
 
     private int sendStaleDatagrams(ChannelHandlerContext ctx, long curTime) {
@@ -533,7 +541,10 @@ private int sendStaleDatagrams(ChannelHandlerContext ctx, long curTime) {
         int resendCount = 0;
         int transmissionBandwidth = this.slidingWindow.getRetransmissionBandwidth();
 
-        for (RakDatagramPacket datagram : this.sentDatagrams.values()) {
+        IntObjectMap<RakDatagramPacket> sent = new IntObjectHashMap<>();
+        Iterator<RakDatagramPacket> iterator = this.sentDatagrams.values().iterator();
+        while (iterator.hasNext()) {
+            RakDatagramPacket datagram = iterator.next();
             if (datagram.getNextSend() <= curTime) {
                 int size = datagram.getSize();
                 if (transmissionBandwidth < size) {
@@ -548,9 +559,13 @@ private int sendStaleDatagrams(ChannelHandlerContext ctx, long curTime) {
                     log.trace("Stale datagram {} from {}", datagram.getSequenceIndex(), this.getRemoteAddress());
                 }
                 resendCount++;
-                this.sendDatagram(ctx, datagram, curTime);
+                iterator.remove();
+                this.sendDatagram(ctx, datagram, curTime, sent);
             }
         }
+        for (IntObjectMap.PrimitiveEntry<RakDatagramPacket> entry : sent.entries()) {
+            this.sentDatagrams.put(entry.key(), entry.value());
+        }
 
         if (hasResent) {
             this.slidingWindow.onResend(curTime);
@@ -580,7 +595,7 @@ private void sendDatagrams(ChannelHandlerContext ctx, long curTime, int mtuSize)
 
             // Send full datagram
             if (!datagram.tryAddPacket(packet, mtuSize)) {
-                this.sendDatagram(ctx, datagram, curTime);
+                this.sendDatagram(ctx, datagram, curTime, this.sentDatagrams);
 
                 datagram = RakDatagramPacket.newInstance();
                 datagram.setSendTime(curTime);
@@ -591,7 +606,7 @@ private void sendDatagrams(ChannelHandlerContext ctx, long curTime, int mtuSize)
         }
 
         if (!datagram.getPackets().isEmpty()) {
-            this.sendDatagram(ctx, datagram, curTime);
+            this.sendDatagram(ctx, datagram, curTime, this.sentDatagrams);
         }
     }
 
@@ -603,19 +618,12 @@ private void sendImmediate(ChannelHandlerContext ctx, EncapsulatedPacket[] packe
             if (!datagram.tryAddPacket(packet, this.getMtu())) {
                 throw new IllegalArgumentException("Packet too large to fit in MTU (size: " + packet.getSize() + ", MTU: " + this.getMtu() + ")");
             }
-            this.sendDatagram(ctx, datagram, curTime);
+            this.sendDatagram(ctx, datagram, curTime, this.sentDatagrams);
         }
         ctx.flush();
     }
 
-    private void sendDatagram(ChannelHandlerContext ctx, RakDatagramPacket datagram, long time) {
-        if (!this.channel.parent().eventLoop().inEventLoop()) {
-            // Make sure this runs on correct thread
-            log.error("Tried to send datagrams from wrong thread: {}", Thread.currentThread().getName(), new Throwable());
-            this.channel.parent().eventLoop().execute(() -> this.sendDatagram(ctx, datagram, time));
-            return;
-        }
-
+    private void sendDatagram(ChannelHandlerContext ctx, RakDatagramPacket datagram, long time, IntObjectMap<RakDatagramPacket> sent) {
         if (datagram.getPackets().isEmpty()) {
             throw new IllegalArgumentException("RakNetDatagram with no packets");
         }
@@ -634,10 +642,8 @@ private void sendDatagram(ChannelHandlerContext ctx, RakDatagramPacket datagram,
                 datagram.setNextSend(time + this.slidingWindow.getRtoForRetransmission());
                 if (oldIndex == -1) {
                     this.slidingWindow.onReliableSend(datagram);
-                } else {
-                    this.sentDatagrams.remove(oldIndex, datagram);
                 }
-                this.sentDatagrams.put(datagram.getSequenceIndex(), datagram.retain()); // Keep for resending
+                sent.put(datagram.getSequenceIndex(), datagram.retain()); // Keep for resending
                 break;
             }
         }

transport-raknet/src/main/java/org/cloudburstmc/netty/util/SplitPacketHelper.java

@@ -34,6 +34,9 @@ public SplitPacketHelper(long expectedLength) {
         if (expectedLength < 2) {
             throw new IllegalArgumentException("expectedLength must be greater than 1");
         }
+        if (expectedLength > 8192) {
+            throw new IllegalArgumentException("Too many split parts, expectedLength must be less than 8192");
+        }
         this.packets = new EncapsulatedPacket[(int) expectedLength];
     }