Core/PacketIO: Validate utf8 in every client packet

src/server/game/Handlers/ChannelHandler.cpp

@@ -23,7 +23,6 @@
 #include "ObjectMgr.h"                                      // for normalizePlayerName
 #include "Player.h"
 #include <cctype>
-#include <utf8.h>
 
 static size_t const MAX_CHANNEL_PASS_STR = 31;
 
@@ -57,12 +56,6 @@ void WorldSession::HandleJoinChannel(WorldPacket& recvPacket)
         return;
     }
 
-    if (!utf8::is_valid(channelName.begin(), channelName.end()))
-    {
-        TC_LOG_ERROR("network", "Player %s tried to create a channel with an invalid UTF8 sequence - blocked", GetPlayer()->GetGUID().ToString().c_str());
-        return;
-    }
-
     if (!ValidateHyperlinksAndMaybeKick(channelName))
         return;
 

src/server/game/Handlers/ChatHandler.cpp

@@ -41,7 +41,6 @@
 #include "Util.h"
 #include "World.h"
 #include "WorldPacket.h"
-#include <utf8.h>
 #include <algorithm>
 
 inline bool isNasty(uint8 c)
@@ -207,15 +206,15 @@ void WorldSession::HandleMessagechatOpcode(WorldPacket& recvData)
         case CHAT_MSG_BATTLEGROUND_LEADER:
         case CHAT_MSG_AFK:
         case CHAT_MSG_DND:
-            recvData >> msg;
+            msg = recvData.ReadCString(lang != LANG_ADDON);
             break;
         case CHAT_MSG_WHISPER:
             recvData >> to;
-            recvData >> msg;
+            msg = recvData.ReadCString(lang != LANG_ADDON);
             break;
         case CHAT_MSG_CHANNEL:
             recvData >> channel;
-            recvData >> msg;
+            msg = recvData.ReadCString(lang != LANG_ADDON);
             break;
     }
 
@@ -259,14 +258,6 @@ void WorldSession::HandleMessagechatOpcode(WorldPacket& recvData)
                 return;
             }
 
-        // validate utf8
-        if (!utf8::is_valid(msg.begin(), msg.end()))
-        {
-            TC_LOG_ERROR("network", "Player %s (GUID: %u) sent a message containing an invalid UTF8 sequence - blocked", GetPlayer()->GetName().c_str(),
-                GetPlayer()->GetGUID().GetCounter());
-            return;
-        }
-
         // collapse multiple spaces into one
         if (sWorld->getBoolConfig(CONFIG_CHAT_FAKE_MESSAGE_PREVENTING))
         {

src/server/shared/Packets/ByteBuffer.cpp

@@ -21,6 +21,7 @@
 #include "Common.h"
 #include "Log.h"
 #include "Util.h"
+#include <utf8.h>
 #include <sstream>
 #include <ctime>
 
@@ -52,11 +53,16 @@ ByteBufferSourceException::ByteBufferSourceException(size_t pos, size_t size,
     message().assign(ss.str());
 }
 
+ByteBufferInvalidValueException::ByteBufferInvalidValueException(char const* type, size_t pos)
+{
+    message().assign(Trinity::StringFormat("Invalid %s value found in ByteBuffer at pos " SZFMTD));
+}
+
 ByteBuffer& ByteBuffer::operator>>(float& value)
 {
     value = read<float>();
     if (!std::isfinite(value))
-        throw ByteBufferException();
+        throw ByteBufferInvalidValueException("float", _rpos - sizeof(float));
     return *this;
 }
 
@@ -64,10 +70,25 @@ ByteBuffer& ByteBuffer::operator>>(double& value)
 {
     value = read<double>();
     if (!std::isfinite(value))
-        throw ByteBufferException();
+        throw ByteBufferInvalidValueException("double", _rpos - sizeof(double));
     return *this;
 }
 
+std::string ByteBuffer::ReadCString(bool requireValidUtf8 /*= true*/)
+{
+    std::string value;
+    while (rpos() < size())                         // prevent crash at wrong string format in packet
+    {
+        char c = read<char>();
+        if (c == 0)
+            break;
+        value += c;
+    }
+    if (requireValidUtf8 && !utf8::is_valid(value.begin(), value.end()))
+        throw ByteBufferInvalidValueException("string", _rpos - value.length() - 1);
+    return value;
+}
+
 uint32 ByteBuffer::ReadPackedTime()
 {
     uint32 packedDate = read<uint32>();

src/server/shared/Packets/ByteBuffer.h

@@ -30,12 +30,12 @@ class MessageBuffer;
 class TC_SHARED_API ByteBufferException : public std::exception
 {
 public:
-    ~ByteBufferException() throw() { }
+    ~ByteBufferException() noexcept = default;
 
-    char const* what() const throw() override { return msg_.c_str(); }
+    char const* what() const noexcept override { return msg_.c_str(); }
 
 protected:
-    std::string & message() throw() { return msg_; }
+    std::string & message() noexcept { return msg_; }
 
 private:
     std::string msg_;
@@ -46,7 +46,7 @@ class TC_SHARED_API ByteBufferPositionException : public ByteBufferException
 public:
     ByteBufferPositionException(bool add, size_t pos, size_t size, size_t valueSize);
 
-    ~ByteBufferPositionException() throw() { }
+    ~ByteBufferPositionException() noexcept = default;
 };
 
 class TC_SHARED_API ByteBufferSourceException : public ByteBufferException
@@ -54,13 +54,21 @@ class TC_SHARED_API ByteBufferSourceException : public ByteBufferException
 public:
     ByteBufferSourceException(size_t pos, size_t size, size_t valueSize);
 
-    ~ByteBufferSourceException() throw() { }
+    ~ByteBufferSourceException() noexcept = default;
+};
+
+class TC_SHARED_API ByteBufferInvalidValueException : public ByteBufferException
+{
+public:
+    ByteBufferInvalidValueException(char const* type, size_t pos);
+
+    ~ByteBufferInvalidValueException() noexcept = default;
 };
 
 class TC_SHARED_API ByteBuffer
 {
     public:
-        const static size_t DEFAULT_SIZE = 0x1000;
+        constexpr static size_t DEFAULT_SIZE = 0x1000;
 
         // constructor
         ByteBuffer() : _rpos(0), _wpos(0)
@@ -73,13 +81,13 @@ class TC_SHARED_API ByteBuffer
             _storage.reserve(reserve);
         }
 
-        ByteBuffer(ByteBuffer&& buf) : _rpos(buf._rpos), _wpos(buf._wpos), _storage(std::move(buf._storage))
+        ByteBuffer(ByteBuffer&& buf) noexcept : _rpos(buf._rpos), _wpos(buf._wpos), _storage(std::move(buf._storage))
         {
             buf._rpos = 0;
             buf._wpos = 0;
         }
 
-        ByteBuffer(ByteBuffer const& right) : _rpos(right._rpos), _wpos(right._wpos), _storage(right._storage) { }
+        ByteBuffer(ByteBuffer const& right) = default;
 
         ByteBuffer(MessageBuffer&& buffer);
 
@@ -95,7 +103,7 @@ class TC_SHARED_API ByteBuffer
             return *this;
         }
 
-        ByteBuffer& operator=(ByteBuffer&& right)
+        ByteBuffer& operator=(ByteBuffer&& right) noexcept
         {
             if (this != &right)
             {
@@ -109,7 +117,7 @@ class TC_SHARED_API ByteBuffer
             return *this;
         }
 
-        virtual ~ByteBuffer() { }
+        virtual ~ByteBuffer() = default;
 
         void clear()
         {
@@ -268,16 +276,9 @@ class TC_SHARED_API ByteBuffer
         ByteBuffer &operator>>(float &value);
         ByteBuffer &operator>>(double &value);
 
-        ByteBuffer &operator>>(std::string& value)
+        ByteBuffer& operator>>(std::string& value)
         {
-            value.clear();
-            while (rpos() < size())                         // prevent crash at wrong string format in packet
-            {
-                char c = read<char>();
-                if (c == 0)
-                    break;
-                value += c;
-            }
+            value = ReadCString(true);
             return *this;
         }
 
@@ -374,6 +375,8 @@ class TC_SHARED_API ByteBuffer
             }
         }
 
+        std::string ReadCString(bool requireValidUtf8 = true);
+
         uint32 ReadPackedTime();
 
         ByteBuffer& ReadPackedTime(uint32& time)