Core/Chat: Prevent client crashes with invalid chat language and message type combinations

Fixes #8083

4 files changed, 51 insertions, 46 deletions

diff --git a/src/server/game/Chat/Chat.cpp b/src/server/game/Chat/Chat.cpp
index 88c3de274fb..7faf05a577c 100755
--- a/src/server/game/Chat/Chat.cpp
+++ b/src/server/game/Chat/Chat.cpp
@@ -425,7 +425,7 @@ bool ChatHandler::SetDataForCommandInTable(ChatCommand* table, char const* text,
     return false;
 }
 
-int ChatHandler::ParseCommands(char const* text)
+bool ChatHandler::ParseCommands(char const* text)
 {
     ASSERT(text);
     ASSERT(*text);
@@ -433,23 +433,23 @@ int ChatHandler::ParseCommands(char const* text)
     std::string fullcmd = text;
 
     if (m_session && AccountMgr::IsPlayerAccount(m_session->GetSecurity()) && !sWorld->getBoolConfig(CONFIG_ALLOW_PLAYER_COMMANDS))
-       return 0;
+       return false;
 
     /// chat case (.command or !command format)
     if (m_session)
     {
         if (text[0] != '!' && text[0] != '.')
-            return 0;
+            return false;
     }
 
     /// ignore single . and ! in line
     if (strlen(text) < 2)
-        return 0;
+        return false;
     // original `text` can't be used. It content destroyed in command code processing.
 
     /// ignore messages staring from many dots.
     if ((text[0] == '.' && text[1] == '.') || (text[0] == '!' && text[1] == '!'))
-        return 0;
+        return false;
 
     /// skip first . or ! (in console allowed use command with . and ! and without its)
     if (text[0] == '!' || text[0] == '.')
@@ -458,11 +458,11 @@ int ChatHandler::ParseCommands(char const* text)
     if (!ExecuteCommandInTable(getCommandTable(), text, fullcmd))
     {
         if (m_session && AccountMgr::IsPlayerAccount(m_session->GetSecurity()))
-            return 0;
+            return false;
 
         SendSysMessage(LANG_NO_CMD);
     }
-    return 1;
+    return true;
 }
 
 bool ChatHandler::isValidChatMessage(char const* message)
diff --git a/src/server/game/Chat/Chat.h b/src/server/game/Chat/Chat.h
index d07f036f780..0c1b22182e1 100755
--- a/src/server/game/Chat/Chat.h
+++ b/src/server/game/Chat/Chat.h
@@ -74,7 +74,7 @@ class ChatHandler
         void PSendSysMessage(int32     entry, ...);
         std::string PGetParseString(int32 entry, ...) const;
 
-        int ParseCommands(const char* text);
+        bool ParseCommands(const char* text);
 
         static ChatCommand* getCommandTable();
 
diff --git a/src/server/game/Handlers/ChatHandler.cpp b/src/server/game/Handlers/ChatHandler.cpp
index 8e68275e471..8b7fc5542ab 100755
--- a/src/server/game/Handlers/ChatHandler.cpp
+++ b/src/server/game/Handlers/ChatHandler.cpp
@@ -40,28 +40,6 @@
 #include "ScriptMgr.h"
 #include "AccountMgr.h"
 
-bool WorldSession::processChatmessageFurtherAfterSecurityChecks(std::string& msg, uint32 lang)
-{
-    if (lang != LANG_ADDON)
-    {
-        // strip invisible characters for non-addon messages
-        if (sWorld->getBoolConfig(CONFIG_CHAT_FAKE_MESSAGE_PREVENTING))
-            stripLineInvisibleChars(msg);
-
-        if (sWorld->getIntConfig(CONFIG_CHAT_STRICT_LINK_CHECKING_SEVERITY) && AccountMgr::IsPlayerAccount(GetSecurity())
-                && !ChatHandler(this).isValidChatMessage(msg.c_str()))
-        {
-            sLog->outError(LOG_FILTER_NETWORKIO, "Player %s (GUID: %u) sent a chatmessage with an invalid link: %s", GetPlayer()->GetName().c_str(),
-                GetPlayer()->GetGUIDLow(), msg.c_str());
-            if (sWorld->getIntConfig(CONFIG_CHAT_STRICT_LINK_CHECKING_KICK))
-                KickPlayer();
-            return false;
-        }
-    }
-
-    return true;
-}
-
 void WorldSession::HandleMessagechatOpcode(WorldPacket& recvData)
 {
     uint32 type;
@@ -112,20 +90,36 @@ void WorldSession::HandleMessagechatOpcode(WorldPacket& recvData)
 
     if (lang == LANG_ADDON)
     {
-        if (sWorld->getBoolConfig(CONFIG_CHATLOG_ADDON))
+        // LANG_ADDON is only valid for the following message types
+        switch (type)
         {
-            std::string msg = "";
-            recvData >> msg;
+            case CHAT_MSG_PARTY:
+            case CHAT_MSG_RAID:
+            case CHAT_MSG_GUILD:
+            case CHAT_MSG_BATTLEGROUND:
+            case CHAT_MSG_WHISPER:
+                if (sWorld->getBoolConfig(CONFIG_CHATLOG_ADDON))
+                {
+                    std::string msg = "";
+                    recvData >> msg;
 
-            if (msg.empty())
-                return;
+                    if (msg.empty())
+                        return;
 
-            sScriptMgr->OnPlayerChat(sender, uint32(CHAT_MSG_ADDON), lang, msg);
-        }
+                    sScriptMgr->OnPlayerChat(sender, uint32(CHAT_MSG_ADDON), lang, msg);
+                }
 
-        // Disabled addon channel?
-        if (!sWorld->getBoolConfig(CONFIG_ADDON_CHANNEL))
-            return;
+                // Disabled addon channel?
+                if (!sWorld->getBoolConfig(CONFIG_ADDON_CHANNEL))
+                    return;
+                break;
+            default:
+                sLog->outError(LOG_FILTER_NETWORKIO, "Player %s (GUID: %u) sent a chatmessage with an invalid language/message type combination", 
+                                                     GetPlayer()->GetName().c_str(), GetPlayer()->GetGUIDLow());
+
+                recvData.rfinish();
+                return;
+        }
     }
     // LANG_ADDON should not be changed nor be affected by flood control
     else
@@ -223,14 +217,26 @@ void WorldSession::HandleMessagechatOpcode(WorldPacket& recvData)
         if (msg.empty())
             return;
 
-        if (ChatHandler(this).ParseCommands(msg.c_str()) > 0)
+        if (ChatHandler(this).ParseCommands(msg.c_str()))
             return;
 
-        if (!processChatmessageFurtherAfterSecurityChecks(msg, lang))
-            return;
+        if (lang != LANG_ADDON)
+        {
+            // Strip invisible characters for non-addon messages
+            if (sWorld->getBoolConfig(CONFIG_CHAT_FAKE_MESSAGE_PREVENTING))
+                stripLineInvisibleChars(msg);
 
-        if (msg.empty())
-            return;
+            if (sWorld->getIntConfig(CONFIG_CHAT_STRICT_LINK_CHECKING_SEVERITY) && !ChatHandler(this).isValidChatMessage(msg.c_str()))
+            {
+                sLog->outError(LOG_FILTER_NETWORKIO, "Player %s (GUID: %u) sent a chatmessage with an invalid link: %s", GetPlayer()->GetName().c_str(),
+                    GetPlayer()->GetGUIDLow(), msg.c_str());
+
+                if (sWorld->getIntConfig(CONFIG_CHAT_STRICT_LINK_CHECKING_KICK))
+                    KickPlayer();
+
+                return;
+            }
+        }
     }
 
     switch (type)
diff --git a/src/server/game/Server/WorldSession.h b/src/server/game/Server/WorldSession.h
index caab00d8fc8..1417916b365 100755
--- a/src/server/game/Server/WorldSession.h
+++ b/src/server/game/Server/WorldSession.h
@@ -702,7 +702,6 @@ class WorldSession
         void HandlePushQuestToParty(WorldPacket& recvPacket);
         void HandleQuestPushResult(WorldPacket& recvPacket);
 
-        bool processChatmessageFurtherAfterSecurityChecks(std::string&, uint32);
         void HandleMessagechatOpcode(WorldPacket& recvPacket);
         void SendPlayerNotFoundNotice(std::string const& name);
         void SendPlayerAmbiguousNotice(std::string const& name);