From b579075dd52597093f9bc23ee07c37976bf2392d Mon Sep 17 00:00:00 2001 From: Meta Vens Date: Tue, 29 Sep 2026 19:41:56 +0200 Subject: [PATCH] GameCult.Networking: a declared length past int.MaxValue is malformed; server mode is claimed only by a Connect; a refusing session ends CultNetRudpPacketCodec.Decode did checked((int)length), so a datagram declaring 0x80000000 threw OverflowException, which no receive loop catches; the length is now compared as a long and takes the ordinary malformed-packet drop. Server mode claimed its endpoint on the first datagram of any type; only a Connect claims it now. A session that refuses a packet has already recorded its reliable sequence, so the socket transport and the listener end it: the session forgets the peer (so the goodbye does not acknowledge the refused frame), the peer gets a Disconnect, the listener drops the peer and raises PeerDisconnected, and a server-mode transport forgets its endpoint. CultNetRudpSession gains ResetPeerState, and the session and the listener reject an InitialSequence of uint.MaxValue. Co-Authored-By: Claude Opus 5.5 --- src/GameCult.Networking/CultNetTransport.cs | 130 +++++++++-- .../NetworkingTests.cs | 212 ++++++++++++++++++ 2 files changed, 328 insertions(+), 14 deletions(-) diff --git a/src/GameCult.Networking/CultNetTransport.cs b/src/GameCult.Networking/CultNetTransport.cs index 9df650cd..b354d757 100644 --- a/src/GameCult.Networking/CultNetTransport.cs +++ b/src/GameCult.Networking/CultNetTransport.cs @@ -664,12 +664,15 @@ namespace GameCult.Networking throw new InvalidOperationException("CultNet RUDP packet header length does not match the channel id length."); } - var payloadLength = checked((int)BinaryPrimitives.ReadUInt32BigEndian(wire.AsSpan(30, 4))); - if (wire.Length != headerBytes + payloadLength) + // Compared as a long: a declared length past int.MaxValue is a malformed packet, not an overflow. + var declaredPayloadLength = BinaryPrimitives.ReadUInt32BigEndian(wire.AsSpan(30, 4)); + if (declaredPayloadLength != (long)wire.Length - headerBytes) { throw new InvalidOperationException("CultNet RUDP packet payload length does not match the packet size."); } + var payloadLength = (int)declaredPayloadLength; + var flags = wire[6]; var payload = new byte[payloadLength]; Array.Copy(wire, headerBytes, payload, 0, payload.Length); @@ -923,10 +926,7 @@ namespace GameCult.Networking public CultNetRudpSession(CultNetRudpSessionOptions options) { if (options == null) throw new ArgumentNullException(nameof(options)); - if (options.MaxPendingReliablePackets.HasValue && options.MaxPendingReliablePackets.Value <= 0) - { - throw new ArgumentOutOfRangeException(nameof(options), "RUDP MaxPendingReliablePackets must be greater than zero."); - } + ValidateLimits(options.InitialSequence, options.MaxPendingReliablePackets); ConnectionId = options.ConnectionId; _nextSequence = options.InitialSequence; @@ -934,6 +934,25 @@ namespace GameCult.Networking _maxPendingReliablePackets = options.MaxPendingReliablePackets; } + internal static readonly byte[] RefusedPacketReason = Encoding.UTF8.GetBytes("session refused a packet"); + + /// + /// A session cannot admit a Connect when its sequence space starts exhausted or its reliable + /// queue holds nothing, so neither is a usable configuration. + /// + internal static void ValidateLimits(uint initialSequence, int? maxPendingReliablePackets) + { + if (initialSequence == uint.MaxValue) + { + throw new ArgumentOutOfRangeException(nameof(initialSequence), "RUDP InitialSequence must leave room for a reliable packet."); + } + + if (maxPendingReliablePackets.HasValue && maxPendingReliablePackets.Value <= 0) + { + throw new ArgumentOutOfRangeException(nameof(maxPendingReliablePackets), "RUDP MaxPendingReliablePackets must be greater than zero."); + } + } + /// /// Gets the connection/session binding id. /// @@ -1237,6 +1256,38 @@ namespace GameCult.Networking return CreatePacket(CultNetRudpPacketType.Ping, "control", payload ?? Array.Empty(), reliable: false, ordered: false, sequenced: false); } + /// + /// Forgets everything learned from the peer; sequence numbers already issued stay issued. + /// + public void ResetPeerState() + { + _connected = false; + _lastReceivedAtMs = null; + _highestReceivedSequence = null; + _receivedSequences.Clear(); + _nextSequencedByChannel.Clear(); + _latestSequencedByChannel.Clear(); + lock (_pendingReliableGate) + { + _pendingReliable.Clear(); + _queuedReliable.Clear(); + } + _orderedNextSequenceByChannel.Clear(); + _orderedBuffers.Clear(); + _fragmentBuffers.Clear(); + } + + /// + /// Ends a session that refused a packet. Receive has already recorded the packet's reliable + /// sequence, so the peer's sequences are forgotten before the goodbye is built: its ack field + /// would otherwise acknowledge the very frame the session refused. + /// + internal CultNetRudpPacket EndRefused() + { + ResetPeerState(); + return CreateDisconnect(RefusedPacketReason); + } + /// /// Creates a packet carrying a transport-level disconnect reason. /// @@ -2112,6 +2163,13 @@ namespace GameCult.Networking } if (_remoteEndPoint == null) { + // Only a Connect claims the endpoint of a server-mode transport. + if (_mode == CultNetRudpSocketMode.Server && packet.PacketType != CultNetRudpPacketType.Connect) + { + _stats.PacketsDropped++; + return true; + } + _remoteEndPoint = remote; } else if (!_remoteEndPoint.Equals(remote)) @@ -2150,7 +2208,24 @@ namespace GameCult.Networking } catch (InvalidOperationException) { + // Receive has already recorded the packet's reliable sequence, so the session + // cannot be kept: a retransmit would be acknowledged and the frame silently lost. + // End it and tell the peer; a server-mode transport also forgets its endpoint, so + // only a Connect can claim it again. _stats.PacketsDropped++; + var goodbye = _session.EndRefused(); + DisconnectReason = CultNetRudpSession.RefusedPacketReason; + try + { + SendPacket(goodbye); + } + catch (SocketException) + { + // Best effort: the session ends whether or not the peer hears it. + } + + if (_mode == CultNetRudpSocketMode.Server) + _remoteEndPoint = null; return true; } if (result.Reply != null) @@ -2388,6 +2463,7 @@ namespace GameCult.Networking _socket = options.Socket ?? throw new ArgumentNullException(nameof(options.Socket)); if (_socket.ReceiveBufferSize < MinimumReceiveBufferBytes) _socket.ReceiveBufferSize = MinimumReceiveBufferBytes; + CultNetRudpSession.ValidateLimits(options.InitialSequence, options.MaxPendingReliablePackets); _connectionId = options.ConnectionId; _initialSequence = options.InitialSequence; _resendDelayMs = options.ResendDelayMs; @@ -2561,25 +2637,51 @@ namespace GameCult.Networking return true; } - CultNetRudpReceiveResult result; + CultNetRudpReceiveResult? received; CultNetRudpPacket? acknowledgement = null; lock (existingPeer.SessionGate) { try { - result = existingPeer.Session.Receive(packet, NowMs()); + received = existingPeer.Session.Receive(packet, NowMs()); } catch (InvalidOperationException) { + // Receive has already recorded the packet's reliable sequence, so the session + // cannot be kept: a retransmit would be acknowledged and the frame silently lost. _stats.PacketsDropped++; - return true; + received = null; + } + + if (received == null) + { + try + { + SendPacket(existingPeer.RemoteEndPoint, existingPeer.Session.EndRefused()); + } + catch (SocketException) + { + // Best effort: the session ends whether or not the peer hears it. + } + } + else + { + if (received.Reply != null) + SendPacket(existingPeer.RemoteEndPoint, received.Reply); + SendPackets(existingPeer, received.ReadyToSend); + if (packet.PacketType == CultNetRudpPacketType.Data) + acknowledgement = existingPeer.Session.CreateAckForReceived(packet.Sequence); } - if (result.Reply != null) - SendPacket(existingPeer.RemoteEndPoint, result.Reply); - SendPackets(existingPeer, result.ReadyToSend); - if (packet.PacketType == CultNetRudpPacketType.Data) - acknowledgement = existingPeer.Session.CreateAckForReceived(packet.Sequence); } + if (received == null) + { + existingPeer.DisconnectReason = CultNetRudpSession.RefusedPacketReason; + _peers.Remove(peerKey); + PeerDisconnected?.Invoke(existingPeer); + return true; + } + + var result = received; if (result.Disconnected) { existingPeer.DisconnectReason = result.DisconnectReason; diff --git a/tests/GameCult.Networking.Tests/NetworkingTests.cs b/tests/GameCult.Networking.Tests/NetworkingTests.cs index d94b877e..94b3dd4a 100644 --- a/tests/GameCult.Networking.Tests/NetworkingTests.cs +++ b/tests/GameCult.Networking.Tests/NetworkingTests.cs @@ -1255,6 +1255,218 @@ namespace GameCult.Networking.Tests Assert.That(Encoding.UTF8.GetString(ReceiveRudpFrame(client).Payload), Is.EqualTo("server still here")); } + private static List DrainPackets(Socket socket) + { + var packets = new List(); + var buffer = new byte[65535]; + EndPoint from = new IPEndPoint(IPAddress.Any, 0); + while (socket.Poll(20_000, SelectMode.SelectRead)) + { + var received = socket.ReceiveFrom(buffer, ref from); + packets.Add(CultNetRudpPacketCodec.Decode(buffer.AsSpan(0, received).ToArray())); + } + return packets; + } + + /// A reliable frame no session accepts: a fragment set with no fragment id. + private static CultNetRudpPacket PoisonedFrame(CultNetRudpSession session) + { + var packet = session.Send("schema", Encoding.UTF8.GetBytes("poison"), new CultNetRudpSendOptions { Reliable = true, Ordered = true }); + packet.FragmentCount = 2; + packet.FragmentId = 0; + return packet; + } + + [Test] + public void RudpPacketCodec_TreatsADeclaredLengthPastInt32AsMalformed() + { + var wire = CultNetRudpPacketCodec.Encode(new CultNetRudpPacket + { + PacketType = CultNetRudpPacketType.Ack, + ConnectionId = 0x10203054, + ChannelId = "c" + }); + Assert.That(wire, Has.Length.EqualTo(36)); + wire[30] = 0x80; + wire[31] = 0; + wire[32] = 0; + wire[33] = 0; + Assert.Throws(() => CultNetRudpPacketCodec.Decode(wire)); + + using var connectionSocket = BindUdpSocket(); + using var listenerSocket = BindUdpSocket(); + using var senderSocket = BindUdpSocket(); + using var connection = new CultNetRudpSocketTransportConnection(new CultNetRudpSocketTransportOptions + { + RuntimeId = "csharp-rudp-connection", + Socket = connectionSocket, + Mode = CultNetRudpSocketMode.Server, + ConnectionId = 0x10203054 + }); + using var listener = new CultNetRudpSocketTransportServer(new CultNetRudpSocketTransportServerOptions + { + RuntimeId = "csharp-rudp-listener", + Socket = listenerSocket, + ConnectionId = 0x10203054 + }); + senderSocket.SendTo(wire, connectionSocket.LocalEndPoint!); + senderSocket.SendTo(wire, listenerSocket.LocalEndPoint!); + Assert.That(connection.ReceiveOnce(), Is.Null); + Assert.That(listener.ReceiveOnce(), Is.Null); + Assert.That(connection.Stats.PacketsDropped, Is.EqualTo(1)); + Assert.That(listener.Stats.PacketsDropped, Is.EqualTo(1)); + } + + [Test] + public void RudpServerMode_IsClaimedOnlyByAConnect() + { + using var serverSocket = BindUdpSocket(); + using var orphanSocket = BindUdpSocket(); + using var clientSocket = BindUdpSocket(); + const uint connectionId = 0x10203055; + using var server = new CultNetRudpSocketTransportConnection(new CultNetRudpSocketTransportOptions + { + RuntimeId = "csharp-rudp-server", + Socket = serverSocket, + Mode = CultNetRudpSocketMode.Server, + ConnectionId = connectionId + }); + using var client = new CultNetRudpSocketTransportConnection(new CultNetRudpSocketTransportOptions + { + RuntimeId = "csharp-rudp-client", + Socket = clientSocket, + Mode = CultNetRudpSocketMode.Client, + RemoteEndPoint = serverSocket.LocalEndPoint!, + ConnectionId = connectionId + }); + var orphan = new CultNetRudpSession(new CultNetRudpSessionOptions { ConnectionId = connectionId }); + + // A right-id packet that is not a Connect must claim nothing. + orphanSocket.SendTo(CultNetRudpPacketCodec.Encode(orphan.CreateAck()), serverSocket.LocalEndPoint!); + Assert.That(server.ReceiveOnce(), Is.Null); + Assert.That(server.Stats.PacketsDropped, Is.EqualTo(1)); + + client.Connect(Encoding.UTF8.GetBytes("join")); + PumpRudpHandshake(client, server); + Assert.That(client.Connected, Is.True); + Assert.That(server.Connected, Is.True); + } + + [Test] + public void RudpSocketTransport_EndsTheSessionThatRefusesAReliableFrame() + { + using var serverSocket = BindUdpSocket(); + using var peerSocket = BindUdpSocket(); + var serverEndPoint = serverSocket.LocalEndPoint!; + const uint connectionId = 0x10203056; + using var server = new CultNetRudpSocketTransportConnection(new CultNetRudpSocketTransportOptions + { + RuntimeId = "csharp-rudp-server", + Socket = serverSocket, + Mode = CultNetRudpSocketMode.Server, + ConnectionId = connectionId + }); + var peer = new CultNetRudpSession(new CultNetRudpSessionOptions { ConnectionId = connectionId }); + + peerSocket.SendTo(CultNetRudpPacketCodec.Encode(peer.CreateConnect(0)), serverEndPoint); + server.ReceiveOnce(); + peer.Receive(DrainPackets(peerSocket).Single(p => p.PacketType == CultNetRudpPacketType.Accept), 0); + + var poison = PoisonedFrame(peer); + peerSocket.SendTo(CultNetRudpPacketCodec.Encode(poison), serverEndPoint); + Assert.That(server.ReceiveOnce(), Is.Null); + Assert.That(server.Stats.PacketsDropped, Is.EqualTo(1)); + Assert.That(server.Connected, Is.False); + Assert.That(server.DisconnectReason, Is.Not.Null); + + // The peer is told, and the goodbye does not acknowledge the refused frame. + var goodbye = DrainPackets(peerSocket).Single(); + Assert.That(goodbye.PacketType, Is.EqualTo(CultNetRudpPacketType.Disconnect)); + Assert.That(peer.Receive(goodbye, 1).Disconnected, Is.True); + Assert.That(peer.PendingReliableSequences, Does.Contain(poison.Sequence)); + + // A retransmit is dropped, not acknowledged. + peerSocket.SendTo(CultNetRudpPacketCodec.Encode(poison), serverEndPoint); + Assert.That(server.ReceiveOnce(), Is.Null); + Assert.That(server.Stats.PacketsDropped, Is.EqualTo(2)); + Assert.That(DrainPackets(peerSocket), Is.Empty); + + // A Connect claims the endpoint again. + var again = new CultNetRudpSession(new CultNetRudpSessionOptions { ConnectionId = connectionId }); + peerSocket.SendTo(CultNetRudpPacketCodec.Encode(again.CreateConnect(0)), serverEndPoint); + server.ReceiveOnce(); + Assert.That(DrainPackets(peerSocket).Select(p => p.PacketType), Is.EqualTo(new[] { CultNetRudpPacketType.Accept })); + } + + [Test] + public void RudpSocketTransportServer_EndsThePeerWhoseReliableFrameItRefuses() + { + using var serverSocket = BindUdpSocket(); + using var peerSocket = BindUdpSocket(); + var serverEndPoint = serverSocket.LocalEndPoint!; + const uint connectionId = 0x10203057; + using var server = new CultNetRudpSocketTransportServer(new CultNetRudpSocketTransportServerOptions + { + RuntimeId = "csharp-rudp-listener", + Socket = serverSocket, + ConnectionId = connectionId + }); + var ended = new List(); + server.PeerDisconnected += ended.Add; + var peer = new CultNetRudpSession(new CultNetRudpSessionOptions { ConnectionId = connectionId }); + + peerSocket.SendTo(CultNetRudpPacketCodec.Encode(peer.CreateConnect(0)), serverEndPoint); + server.ReceiveOnce(); + peer.Receive(DrainPackets(peerSocket).Single(p => p.PacketType == CultNetRudpPacketType.Accept), 0); + Assert.That(server.Peers, Has.Count.EqualTo(1)); + + var poison = PoisonedFrame(peer); + peerSocket.SendTo(CultNetRudpPacketCodec.Encode(poison), serverEndPoint); + Assert.That(server.ReceiveOnce(), Is.Null); + Assert.That(server.Stats.PacketsDropped, Is.EqualTo(1)); + Assert.That(server.Peers, Is.Empty); + Assert.That(ended, Has.Count.EqualTo(1)); + + var goodbye = DrainPackets(peerSocket).Single(); + Assert.That(goodbye.PacketType, Is.EqualTo(CultNetRudpPacketType.Disconnect)); + Assert.That(peer.Receive(goodbye, 1).Disconnected, Is.True); + Assert.That(peer.PendingReliableSequences, Does.Contain(poison.Sequence)); + + peerSocket.SendTo(CultNetRudpPacketCodec.Encode(poison), serverEndPoint); + Assert.That(server.ReceiveOnce(), Is.Null); + Assert.That(server.Stats.PacketsDropped, Is.EqualTo(2)); + Assert.That(DrainPackets(peerSocket), Is.Empty); + } + + [Test] + public void RudpConfiguration_RejectsLimitsThatCannotAdmitAConnect() + { + Assert.Throws(() => + new CultNetRudpSession(new CultNetRudpSessionOptions { ConnectionId = 1, InitialSequence = uint.MaxValue })); + Assert.Throws(() => + new CultNetRudpSession(new CultNetRudpSessionOptions { ConnectionId = 1, MaxPendingReliablePackets = 0 })); + Assert.DoesNotThrow(() => + new CultNetRudpSession(new CultNetRudpSessionOptions { ConnectionId = 1, InitialSequence = uint.MaxValue - 1, MaxPendingReliablePackets = 1 })); + + using var socket = BindUdpSocket(); + Assert.Throws(() => + new CultNetRudpSocketTransportServer(new CultNetRudpSocketTransportServerOptions + { + RuntimeId = "csharp-rudp-listener", + Socket = socket, + ConnectionId = 1, + InitialSequence = uint.MaxValue + })); + Assert.Throws(() => + new CultNetRudpSocketTransportServer(new CultNetRudpSocketTransportServerOptions + { + RuntimeId = "csharp-rudp-listener", + Socket = socket, + ConnectionId = 1, + MaxPendingReliablePackets = 0 + })); + } + [Test] public void RudpSocketTransport_CarriesCultNetSchemaMessages() {