From 02438e9934896021f71dfd235326df9b19ddd497 Mon Sep 17 00:00:00 2001 From: Rubidium Date: Fri, 16 Jan 2026 21:37:30 +0100 Subject: [PATCH] Codechange: unify behaviour of handling too short packets --- src/network/core/packet.cpp | 4 ++-- src/network/core/tcp_admin.cpp | 8 +------- src/network/core/tcp_content.cpp | 10 +++------- src/network/core/tcp_coordinator.cpp | 2 +- src/network/core/tcp_game.cpp | 8 +------- src/network/core/tcp_turn.cpp | 2 +- src/network/core/udp.cpp | 12 +++--------- 7 files changed, 12 insertions(+), 34 deletions(-) diff --git a/src/network/core/packet.cpp b/src/network/core/packet.cpp index 6446577402..7935a46b9d 100644 --- a/src/network/core/packet.cpp +++ b/src/network/core/packet.cpp @@ -285,7 +285,8 @@ bool Packet::PrepareToRead() bool valid = cs->receive_encryption_handler->Decrypt(std::span(&this->buffer[pos], mac_size), std::span(&this->buffer[pos + mac_size], this->buffer.size() - pos - mac_size)); this->pos += static_cast(mac_size); - return valid; + /* Is the decryption valid *and* is the remaining data big enough to contain the packet type? */ + return valid && this->CanReadFromPacket(EncodedLengthOfPacketType()); } /** @@ -294,7 +295,6 @@ bool Packet::PrepareToRead() */ PacketType Packet::GetPacketType() const { - assert(this->Size() >= EncodedLengthOfPacketSize() + EncodedLengthOfPacketType()); size_t offset = EncodedLengthOfPacketSize(); if (cs != nullptr && cs->send_encryption_handler != nullptr) offset += cs->send_encryption_handler->MACSize(); return static_cast(buffer[offset]); diff --git a/src/network/core/tcp_admin.cpp b/src/network/core/tcp_admin.cpp index 40e1c636f5..1e7150a569 100644 --- a/src/network/core/tcp_admin.cpp +++ b/src/network/core/tcp_admin.cpp @@ -34,13 +34,7 @@ NetworkRecvStatus NetworkAdminSocketHandler::CloseConnection(bool) */ NetworkRecvStatus NetworkAdminSocketHandler::HandlePacket(Packet &p) { - PacketAdminType type = (PacketAdminType)p.Recv_uint8(); - - if (this->HasClientQuit()) { - Debug(net, 0, "[tcp/admin] Received invalid packet from '{}' ({})", this->admin_name, this->admin_version); - this->CloseConnection(); - return NETWORK_RECV_STATUS_MALFORMED_PACKET; - } + PacketAdminType type = static_cast(p.Recv_uint8()); switch (type) { case ADMIN_PACKET_ADMIN_JOIN: return this->Receive_ADMIN_JOIN(p); diff --git a/src/network/core/tcp_content.cpp b/src/network/core/tcp_content.cpp index ddd34b5c6b..ec1309cefd 100644 --- a/src/network/core/tcp_content.cpp +++ b/src/network/core/tcp_content.cpp @@ -103,9 +103,9 @@ std::optional ContentInfo::GetTextfile(TextfileType type) const */ bool NetworkContentSocketHandler::HandlePacket(Packet &p) { - PacketContentType type = (PacketContentType)p.Recv_uint8(); + PacketContentType type = static_cast(p.Recv_uint8()); - switch (this->HasClientQuit() ? PACKET_CONTENT_END : type) { + switch (type) { case PACKET_CONTENT_CLIENT_INFO_LIST: return this->Receive_CLIENT_INFO_LIST(p); case PACKET_CONTENT_CLIENT_INFO_ID: return this->Receive_CLIENT_INFO_ID(p); case PACKET_CONTENT_CLIENT_INFO_EXTID: return this->Receive_CLIENT_INFO_EXTID(p); @@ -115,11 +115,7 @@ bool NetworkContentSocketHandler::HandlePacket(Packet &p) case PACKET_CONTENT_SERVER_CONTENT: return this->Receive_SERVER_CONTENT(p); default: - if (this->HasClientQuit()) { - Debug(net, 0, "[tcp/content] Received invalid packet type {}", type); - } else { - Debug(net, 0, "[tcp/content] Received illegal packet"); - } + Debug(net, 0, "[tcp/content] Received invalid packet type {}", type); return false; } } diff --git a/src/network/core/tcp_coordinator.cpp b/src/network/core/tcp_coordinator.cpp index ddf8a76e76..ff649bed33 100644 --- a/src/network/core/tcp_coordinator.cpp +++ b/src/network/core/tcp_coordinator.cpp @@ -22,7 +22,7 @@ */ bool NetworkCoordinatorSocketHandler::HandlePacket(Packet &p) { - PacketCoordinatorType type = (PacketCoordinatorType)p.Recv_uint8(); + PacketCoordinatorType type = static_cast(p.Recv_uint8()); switch (type) { case PACKET_COORDINATOR_GC_ERROR: return this->Receive_GC_ERROR(p); diff --git a/src/network/core/tcp_game.cpp b/src/network/core/tcp_game.cpp index e307de63bc..7b6c67eb70 100644 --- a/src/network/core/tcp_game.cpp +++ b/src/network/core/tcp_game.cpp @@ -58,13 +58,7 @@ NetworkRecvStatus NetworkGameSocketHandler::CloseConnection([[maybe_unused]] boo */ NetworkRecvStatus NetworkGameSocketHandler::HandlePacket(Packet &p) { - PacketGameType type = (PacketGameType)p.Recv_uint8(); - - if (this->HasClientQuit()) { - Debug(net, 0, "[tcp/game] Received invalid packet from client {}", this->client_id); - this->CloseConnection(); - return NETWORK_RECV_STATUS_MALFORMED_PACKET; - } + PacketGameType type = static_cast(p.Recv_uint8()); this->last_packet = std::chrono::steady_clock::now(); diff --git a/src/network/core/tcp_turn.cpp b/src/network/core/tcp_turn.cpp index 7ec80f4db9..64390df543 100644 --- a/src/network/core/tcp_turn.cpp +++ b/src/network/core/tcp_turn.cpp @@ -22,7 +22,7 @@ */ bool NetworkTurnSocketHandler::HandlePacket(Packet &p) { - PacketTurnType type = (PacketTurnType)p.Recv_uint8(); + PacketTurnType type = static_cast(p.Recv_uint8()); switch (type) { case PACKET_TURN_TURN_ERROR: return this->Receive_TURN_ERROR(p); diff --git a/src/network/core/udp.cpp b/src/network/core/udp.cpp index 2b72e17caa..f318831c09 100644 --- a/src/network/core/udp.cpp +++ b/src/network/core/udp.cpp @@ -156,23 +156,17 @@ void NetworkUDPSocketHandler::ReceivePackets() */ void NetworkUDPSocketHandler::HandleUDPPacket(Packet &p, NetworkAddress &client_addr) { - PacketUDPType type; - /* New packet == new client, which has not quit yet */ this->Reopen(); - type = (PacketUDPType)p.Recv_uint8(); + PacketUDPType type = static_cast(p.Recv_uint8()); - switch (this->HasClientQuit() ? PACKET_UDP_END : type) { + switch (type) { case PACKET_UDP_CLIENT_FIND_SERVER: this->Receive_CLIENT_FIND_SERVER(p, client_addr); break; case PACKET_UDP_SERVER_RESPONSE: this->Receive_SERVER_RESPONSE(p, client_addr); break; default: - if (this->HasClientQuit()) { - Debug(net, 0, "[udp] Received invalid packet type {} from {}", type, client_addr.GetAddressAsString()); - } else { - Debug(net, 0, "[udp] Received illegal packet from {}", client_addr.GetAddressAsString()); - } + Debug(net, 0, "[udp] Received invalid packet type {} from {}", type, client_addr.GetAddressAsString()); break; } }