fix(Kcp): V1.41 [2024-04-28]

- fix: KcpHeader is now parsed safely, handling attackers potentially sending values out of enum range
- fix: KcpClient RawSend may throw ConnectionRefused SocketException when OnDisconnected calls SendDisconnect(), which is fine
- fix: less scary cookie message and better explanation

Assets/Mirror/Transports/KCP/kcp2k/VERSION.txt

@@ -1,3 +1,8 @@
+V1.41 [2024-04-28]
+- fix: KcpHeader is now parsed safely, handling attackers potentially sending values out of enum range
+- fix: KcpClient RawSend may throw ConnectionRefused SocketException when OnDisconnected calls SendDisconnect(), which is fine
+- fix: less scary cookie message and better explanation
+
 V1.40 [2024-01-03]
 - added [KCP] to all log messages
 - fix: #3704 remove old fix for #2353 which caused log spam and isn't needed anymore since the

Assets/Mirror/Transports/KCP/kcp2k/highlevel/KcpClient.cs

@@ -183,6 +183,7 @@ protected override void RawSend(ArraySegment<byte> data)
                 // at least log a message for easier debugging.
                 Log.Info($"[KCP] Client.RawSend: looks like the other end has closed the connection. This is fine: {e}");
                 // base.Disconnect(); <- don't call this, would deadlock if SendDisconnect() already throws
+
             }
         }
 

Assets/Mirror/Transports/KCP/kcp2k/highlevel/KcpHeader.cs

@@ -1,3 +1,5 @@
+using System;
+
 namespace kcp2k
 {
     // header for messages processed by kcp.
@@ -23,4 +25,33 @@ public enum KcpHeaderUnreliable : byte
         // disconnect always goes through rapid fire unreliable (glenn fielder)
         Disconnect = 5,
     }
+
+    // save convert the enums from/to byte.
+    // attackers may attempt to send invalid values, so '255' may not convert.
+    public static class KcpHeader
+    {
+        public static bool ParseReliable(byte value, out KcpHeaderReliable header)
+        {
+            if (Enum.IsDefined(typeof(KcpHeaderReliable), value))
+            {
+                header = (KcpHeaderReliable)value;
+                return true;
+            }
+
+            header = KcpHeaderReliable.Ping; // any default
+            return false;
+        }
+
+        public static bool ParseUnreliable(byte value, out KcpHeaderUnreliable header)
+        {
+            if (Enum.IsDefined(typeof(KcpHeaderUnreliable), value))
+            {
+                header = (KcpHeaderUnreliable)value;
+                return true;
+            }
+
+            header = KcpHeaderUnreliable.Disconnect; // any default
+            return false;
+        }
+    }
 }

Assets/Mirror/Transports/KCP/kcp2k/highlevel/KcpPeer.cs

@@ -313,8 +313,16 @@ bool ReceiveNextReliable(out KcpHeaderReliable header, out ArraySegment<byte> me
                 return false;
             }
 
-            // extract header & content without header
+            // safely extract header. attackers may send values out of enum range.
-            header = (KcpHeaderReliable)kcpMessageBuffer[0];
+            byte headerByte = kcpMessageBuffer[0];
+            if (!KcpHeader.ParseReliable(headerByte, out header))
+            {
+                OnError(ErrorCode.InvalidReceive, $"{GetType()}: Receive failed to parse header: {headerByte} is not defined in {typeof(KcpHeaderReliable)}.");
+                Disconnect();
+                return false;
+            }
+
+            // extract content without header
             message = new ArraySegment<byte>(kcpMessageBuffer, 1, msgSize - 1);
             lastReceiveTime = (uint)watch.ElapsedMilliseconds;
             return true;
@@ -529,9 +537,17 @@ protected void OnRawInputUnreliable(ArraySegment<byte> message)
             // need at least one byte for the KcpHeader enum
             if (message.Count < 1) return;
 
-            // parse header and subtract it from message content.
+            // safely extract header. attackers may send values out of enum range.
+            byte headerByte = message.Array[message.Offset + 0];
+            if (!KcpHeader.ParseUnreliable(headerByte, out KcpHeaderUnreliable header))
+            {
+                OnError(ErrorCode.InvalidReceive, $"{GetType()}: Receive failed to parse header: {headerByte} is not defined in {typeof(KcpHeaderUnreliable)}.");
+                Disconnect();
+                return;
+            }
+
+            // subtract header from message content
             // (above we already ensure it's at least 1 byte long)
-            KcpHeaderUnreliable header = (KcpHeaderUnreliable)message.Array[message.Offset + 0];
             message = new ArraySegment<byte>(message.Array, message.Offset + 1, message.Count - 1);
 
             switch (header)

Assets/Mirror/Transports/KCP/kcp2k/highlevel/KcpServerConnection.cs

@@ -82,15 +82,17 @@ public void RawInput(ArraySegment<byte> segment)
             // parse the cookie and make sure it matches (except for initial hello).
             Utils.Decode32U(segment.Array, segment.Offset + 1, out uint messageCookie);
 
-            // compare cookie to protect against UDP spoofing.
+            // security: messages after authentication are expected to contain the cookie.
-            // messages won't have a cookie until after handshake.
+            // this protects against UDP spoofing.
-            // so only compare if we are authenticated.
             // simply drop the message if the cookie doesn't match.
             if (state == KcpState.Authenticated)
             {
                 if (messageCookie != cookie)
                 {
-                    Log.Warning($"[KCP] ServerConnection: dropped message with invalid cookie: {messageCookie} expected: {cookie} state: {state}");
+                    // Info is enough, don't scare users.
+                    // => this can happen for malicious messages
+                    // => it can also happen if client's Hello message was retransmitted multiple times, which is totally normal.
+                    Log.Info($"[KCP] ServerConnection: dropped message with invalid cookie: {messageCookie} from {remoteEndPoint} expected: {cookie} state: {state}. This can happen if the client's Hello message was transmitted multiple times, or if an attacker attempted UDP spoofing.");
                     return;
                 }
             }