Core/PacketIO: Validate utf8 in every client packet

(cherry picked from commit 0a2a96efaa20fe5440eb5a2f21ffaddcdcea86d3)

2 files changed, 56 insertions, 28 deletions

diff --git a/src/server/shared/Packets/ByteBuffer.cpp b/src/server/shared/Packets/ByteBuffer.cpp
index c8aecb24396..c0a4122d745 100644
--- a/src/server/shared/Packets/ByteBuffer.cpp
+++ b/src/server/shared/Packets/ByteBuffer.cpp
@@ -20,6 +20,7 @@
 #include "MessageBuffer.h"
 #include "Log.h"
 #include "Util.h"
+#include <utf8.h>
 #include <sstream>
 #include <ctime>
 
@@ -38,11 +39,16 @@ ByteBufferPositionException::ByteBufferPositionException(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;
 }
 
@@ -50,10 +56,41 @@ 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;
+}
+
+std::string ByteBuffer::ReadString(uint32 length, bool requireValidUtf8 /*= true*/)
+{
+    if (_rpos + length > size())
+        throw ByteBufferPositionException(_rpos, length, size());
+
+    ResetBitPos();
+    if (!length)
+        return std::string();
+
+    std::string value(reinterpret_cast<char const*>(&_storage[_rpos]), length);
+    _rpos += length;
+    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>();
diff --git a/src/server/shared/Packets/ByteBuffer.h b/src/server/shared/Packets/ByteBuffer.h
index c629048922d..553c1a4c4ff 100644
--- a/src/server/shared/Packets/ByteBuffer.h
+++ b/src/server/shared/Packets/ByteBuffer.h
@@ -36,7 +36,7 @@ public:
     char const* what() const noexcept override { return msg_.c_str(); }
 
 protected:
-    std::string& message() { return msg_; }
+    std::string & message() noexcept { return msg_; }
 
 private:
     std::string msg_;
@@ -50,11 +50,19 @@ public:
     ~ByteBufferPositionException() 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:
-        static size_t const DEFAULT_SIZE = 0x1000;
-        static uint8 const InitialBitPos = 8;
+        constexpr static size_t DEFAULT_SIZE = 0x1000;
+        constexpr static uint8 InitialBitPos = 8;
 
         // constructor
         ByteBuffer() : _rpos(0), _wpos(0), _bitpos(InitialBitPos), _curbitval(0)
@@ -366,16 +374,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;
         }
 
@@ -492,20 +493,6 @@ class TC_SHARED_API ByteBuffer
                     value |= (uint64(read<uint8>()) << (i * 8));
         }
 
-        std::string ReadString(uint32 length)
-        {
-            if (_rpos + length > size())
-                throw ByteBufferPositionException(_rpos, length, size());
-
-            ResetBitPos();
-            if (!length)
-                return std::string();
-
-            std::string str((char const*)&_storage[_rpos], length);
-            _rpos += length;
-            return str;
-        }
-
         //! Method for writing strings that have their length sent separately in packet
         //! without null-terminating the string
         void WriteString(std::string const& str)
@@ -520,6 +507,10 @@ class TC_SHARED_API ByteBuffer
                 append(str, len);
         }
 
+        std::string ReadCString(bool requireValidUtf8 = true);
+
+        std::string ReadString(uint32 length, bool requireValidUtf8 = true);
+
         uint32 ReadPackedTime();
 
         uint8* contents()