Core/PacketIO: Fixed crash happening when someone sends too small packet that is processed directly in WorldSocket

Closes #17536

4 files changed, 102 insertions, 30 deletions

diff --git a/src/server/game/Server/Packets/AuthenticationPackets.cpp b/src/server/game/Server/Packets/AuthenticationPackets.cpp
index 2540b9ce05a..23a27ae4114 100644
--- a/src/server/game/Server/Packets/AuthenticationPackets.cpp
+++ b/src/server/game/Server/Packets/AuthenticationPackets.cpp
@@ -18,6 +18,32 @@
 #include "AuthenticationPackets.h"
 #include "HmacHash.h"
 
+bool WorldPackets::Auth::EarlyProcessClientPacket::ReadNoThrow()
+{
+    try
+    {
+        Read();
+        return true;
+    }
+    catch (ByteBufferPositionException const& ex)
+    {
+    }
+
+    return false;
+}
+
+void WorldPackets::Auth::Ping::Read()
+{
+    _worldPacket >> Serial;
+    _worldPacket >> Latency;
+}
+
+const WorldPacket* WorldPackets::Auth::Pong::Write()
+{
+    _worldPacket << uint32(Serial);
+    return &_worldPacket;
+}
+
 WorldPacket const* WorldPackets::Auth::AuthChallenge::Write()
 {
     _worldPacket.append(DosChallenge, 8);
diff --git a/src/server/game/Server/Packets/AuthenticationPackets.h b/src/server/game/Server/Packets/AuthenticationPackets.h
index f8716521801..a127b4b2e62 100644
--- a/src/server/game/Server/Packets/AuthenticationPackets.h
+++ b/src/server/game/Server/Packets/AuthenticationPackets.h
@@ -31,6 +31,36 @@ namespace WorldPackets
 {
     namespace Auth
     {
+        class EarlyProcessClientPacket : public ClientPacket
+        {
+        public:
+            EarlyProcessClientPacket(OpcodeClient opcode, WorldPacket&& packet) : ClientPacket(opcode, std::move(packet)) { }
+
+            bool ReadNoThrow();
+        };
+
+        class Ping final : public EarlyProcessClientPacket
+        {
+        public:
+            Ping(WorldPacket&& packet) : EarlyProcessClientPacket(CMSG_PING, std::move(packet)) { }
+
+            uint32 Serial = 0;
+            uint32 Latency = 0;
+
+        private:
+            void Read();
+        };
+
+        class Pong final : public ServerPacket
+        {
+        public:
+            Pong(uint32 serial) : ServerPacket(SMSG_PONG, 4), Serial(serial) { }
+
+            WorldPacket const* Write() override;
+
+            uint32 Serial = 0;
+        };
+
         class AuthChallenge final : public ServerPacket
         {
         public:
@@ -43,19 +73,17 @@ namespace WorldPackets
             uint8 DosZeroBits = 0;
         };
 
-        class AuthSession final : public ClientPacket
+        class AuthSession final : public EarlyProcessClientPacket
         {
         public:
             static uint32 const DigestLength = 24;
 
-            AuthSession(WorldPacket&& packet) : ClientPacket(CMSG_AUTH_SESSION, std::move(packet))
+            AuthSession(WorldPacket&& packet) : EarlyProcessClientPacket(CMSG_AUTH_SESSION, std::move(packet))
             {
                 LocalChallenge.fill(0);
                 Digest.fill(0);
             }
 
-            void Read() override;
-
             uint16 Build = 0;
             int8 BuildType = 0;
             uint32 RegionID = 0;
@@ -67,6 +95,9 @@ namespace WorldPackets
             ByteBuffer AddonInfo;
             std::string RealmJoinTicket;
             bool UseIPv6 = false;
+
+        private:
+            void Read() override;
         };
 
         class AuthResponse final : public ServerPacket
@@ -173,23 +204,24 @@ namespace WorldPackets
             BigNumber iqmp;
         };
 
-        class AuthContinuedSession final : public ClientPacket
+        class AuthContinuedSession final : public EarlyProcessClientPacket
         {
         public:
             static uint32 const DigestLength = 24;
 
-            AuthContinuedSession(WorldPacket&& packet) : ClientPacket(CMSG_AUTH_CONTINUED_SESSION, std::move(packet))
+            AuthContinuedSession(WorldPacket&& packet) : EarlyProcessClientPacket(CMSG_AUTH_CONTINUED_SESSION, std::move(packet))
             {
                 LocalChallenge.fill(0);
                 Digest.fill(0);
             }
 
-            void Read() override;
-
             uint64 DosResponse = 0;
             uint64 Key = 0;
             std::array<uint8, 16> LocalChallenge;
             std::array<uint8, DigestLength> Digest;
+
+        private:
+            void Read() override;
         };
 
         class ResumeComms final : public ServerPacket
@@ -200,15 +232,16 @@ namespace WorldPackets
             WorldPacket const* Write() override { return &_worldPacket; }
         };
 
-        class ConnectToFailed final : public ClientPacket
+        class ConnectToFailed final : public EarlyProcessClientPacket
         {
         public:
-            ConnectToFailed(WorldPacket&& packet) : ClientPacket(CMSG_CONNECT_TO_FAILED, std::move(packet)) { }
-
-            void Read() override;
+            ConnectToFailed(WorldPacket&& packet) : EarlyProcessClientPacket(CMSG_CONNECT_TO_FAILED, std::move(packet)) { }
 
             ConnectToSerial Serial = ConnectToSerial::None;
             uint8 Con = 0;
+
+        private:
+            void Read() override;
         };
     }
 }
diff --git a/src/server/game/Server/WorldSocket.cpp b/src/server/game/Server/WorldSocket.cpp
index 82acf2b1811..0ad88b9edac 100644
--- a/src/server/game/Server/WorldSocket.cpp
+++ b/src/server/game/Server/WorldSocket.cpp
@@ -404,8 +404,18 @@ WorldSocket::ReadDataHandlerResult WorldSocket::ReadDataHandler()
     switch (opcode)
     {
         case CMSG_PING:
+        {
             LogOpcodeText(opcode, sessionGuard);
-            return HandlePing(packet) ? ReadDataHandlerResult::Ok : ReadDataHandlerResult::Error;
+            WorldPackets::Auth::Ping ping(std::move(packet));
+            if (!ping.ReadNoThrow())
+            {
+                TC_LOG_ERROR("network", "WorldSocket::ReadDataHandler(): client %s sent malformed CMSG_PING", GetRemoteIpAddress().to_string().c_str());
+                return ReadDataHandlerResult::Error;
+            }
+            if (!HandlePing(ping))
+                return ReadDataHandlerResult::Error;
+            break;
+        }
         case CMSG_AUTH_SESSION:
         {
             LogOpcodeText(opcode, sessionGuard);
@@ -418,7 +428,11 @@ WorldSocket::ReadDataHandlerResult WorldSocket::ReadDataHandler()
             }
 
             std::shared_ptr<WorldPackets::Auth::AuthSession> authSession = std::make_shared<WorldPackets::Auth::AuthSession>(std::move(packet));
-            authSession->Read();
+            if (!authSession->ReadNoThrow())
+            {
+                TC_LOG_ERROR("network", "WorldSocket::ReadDataHandler(): client %s sent malformed CMSG_AUTH_SESSION", GetRemoteIpAddress().to_string().c_str());
+                return ReadDataHandlerResult::Error;
+            }
             HandleAuthSession(authSession);
             return ReadDataHandlerResult::WaitingForQuery;
         }
@@ -434,7 +448,11 @@ WorldSocket::ReadDataHandlerResult WorldSocket::ReadDataHandler()
             }
 
             std::shared_ptr<WorldPackets::Auth::AuthContinuedSession> authSession = std::make_shared<WorldPackets::Auth::AuthContinuedSession>(std::move(packet));
-            authSession->Read();
+            if (!authSession->ReadNoThrow())
+            {
+                TC_LOG_ERROR("network", "WorldSocket::ReadDataHandler(): client %s sent malformed CMSG_AUTH_CONTINUED_SESSION", GetRemoteIpAddress().to_string().c_str());
+                return ReadDataHandlerResult::Error;
+            }
             HandleAuthContinuedSession(authSession);
             return ReadDataHandlerResult::WaitingForQuery;
         }
@@ -455,7 +473,11 @@ WorldSocket::ReadDataHandlerResult WorldSocket::ReadDataHandler()
 
             LogOpcodeText(opcode, sessionGuard);
             WorldPackets::Auth::ConnectToFailed connectToFailed(std::move(packet));
-            connectToFailed.Read();
+            if (!connectToFailed.ReadNoThrow())
+            {
+                TC_LOG_ERROR("network", "WorldSocket::ReadDataHandler(): client %s sent malformed CMSG_CONNECT_TO_FAILED", GetRemoteIpAddress().to_string().c_str());
+                return ReadDataHandlerResult::Error;
+            }
             HandleConnectToFailed(connectToFailed);
             break;
         }
@@ -964,17 +986,10 @@ void WorldSocket::SendAuthResponseError(uint32 code)
     SendPacketAndLogOpcode(*response.Write());
 }
 
-bool WorldSocket::HandlePing(WorldPacket& recvPacket)
+bool WorldSocket::HandlePing(WorldPackets::Auth::Ping& ping)
 {
     using namespace std::chrono;
 
-    uint32 ping;
-    uint32 latency;
-
-    // Get the ping packet content
-    recvPacket >> ping;
-    recvPacket >> latency;
-
     if (_LastPingTime == steady_clock::time_point())
     {
         _LastPingTime = steady_clock::now();
@@ -1015,7 +1030,7 @@ bool WorldSocket::HandlePing(WorldPacket& recvPacket)
 
         if (_worldSession)
         {
-            _worldSession->SetLatency(latency);
+            _worldSession->SetLatency(ping.Latency);
             _worldSession->ResetClientTimeDelay();
         }
         else
@@ -1025,8 +1040,6 @@ bool WorldSocket::HandlePing(WorldPacket& recvPacket)
         }
     }
 
-    WorldPacket packet(SMSG_PONG, 4);
-    packet << ping;
-    SendPacketAndLogOpcode(packet);
+    SendPacketAndLogOpcode(*WorldPackets::Auth::Pong(ping.Serial).Write());
     return true;
 }
diff --git a/src/server/game/Server/WorldSocket.h b/src/server/game/Server/WorldSocket.h
index 5aed861e613..6f0d314f312 100644
--- a/src/server/game/Server/WorldSocket.h
+++ b/src/server/game/Server/WorldSocket.h
@@ -42,6 +42,7 @@ namespace WorldPackets
         class AuthSession;
         class AuthContinuedSession;
         class ConnectToFailed;
+        class Ping;
     }
 }
 
@@ -129,8 +130,7 @@ private:
     void HandleAuthContinuedSessionCallback(std::shared_ptr<WorldPackets::Auth::AuthContinuedSession> authSession, PreparedQueryResult result);
     void LoadSessionPermissionsCallback(PreparedQueryResult result);
     void HandleConnectToFailed(WorldPackets::Auth::ConnectToFailed& connectToFailed);
-
-    bool HandlePing(WorldPacket& recvPacket);
+    bool HandlePing(WorldPackets::Auth::Ping& ping);
 
     void ExtractOpcodeAndSize(ClientPktHeader const* header, uint32& opcode, uint32& size) const;