[8378] Use exceptions instead of explicit size checking for each packet Author: arrai

    CHECK_PACKET_SIZE was pretty error prone; once it was forgotten mangosd
    could crash due to the asserts in ByteBuffer.h. That was exploitable by
    malicious players.
    Furthermore, there were duplicate checks: Additionally to
    CHECK_PACKET_SIZE, the ByteBuffer assertions keept an eye
    on not exceeding the packet boundaries - just to crash the server for
    sure in such a case.
    To prevent memory leaks or other undesirable states, please read in
    every handler all variables _before_ doing any concrete handling.

--HG--
branch : trunk

1 files changed, 26 insertions, 11 deletions

diff --git a/src/shared/ByteBuffer.h b/src/shared/ByteBuffer.h
index 75c33dcf3ca..5e02afd86cf 100644
--- a/src/shared/ByteBuffer.h
+++ b/src/shared/ByteBuffer.h
@@ -26,6 +26,26 @@
 #include "Log.h"
 #include "Utilities/ByteConverter.h"
 
+class ByteBufferException
+{
+    public:
+        ByteBufferException(bool add, size_t pos, size_t esize, size_t size):add(add), pos(pos), esize(esize), size(size)
+        {
+            PrintPosError();
+        }
+
+        void PrintPosError() const
+        {
+            sLog.outError("ERROR: Attempted to %s in ByteBuffer (pos: %lu size: %lu) value with size: %lu",(add ? "put" : "get"),(unsigned long)pos, (unsigned long)size, (unsigned long)esize);
+
+        }
+    private:
+        bool add;
+        size_t pos;
+        size_t esize;
+        size_t size;
+};
+
 class ByteBuffer
 {
     public:
@@ -250,7 +270,8 @@ class ByteBuffer
 
         template <typename T> T read(size_t pos) const
         {
-            ASSERT(pos + sizeof(T) <= size() || PrintPosError(false, pos, sizeof(T)));
+            if(pos + sizeof(T) > size())
+                throw ByteBufferException(false, pos, sizeof(T), size());
             T val = *((T const*)&_storage[pos]);
             EndianConvert(val);
             return val;
@@ -258,7 +279,8 @@ class ByteBuffer
 
         void read(uint8 *dest, size_t len)
         {
-            ASSERT(_rpos  + len  <= size() || PrintPosError(false, _rpos, len));
+            if(_rpos  + len > size())
+               throw ByteBufferException(false, _rpos, len, size());
             memcpy(dest, &_storage[_rpos], len);
             _rpos += len;
         }
@@ -372,7 +394,8 @@ class ByteBuffer
 
         void put(size_t pos, const uint8 *src, size_t cnt)
         {
-            ASSERT(pos + cnt <= size() || PrintPosError(true, pos, cnt));
+            if(pos + cnt > size())
+               throw ByteBufferException(true, pos, cnt, size());
             memcpy(&_storage[pos], src, cnt);
         }
 
@@ -454,14 +477,6 @@ class ByteBuffer
         }
 
     protected:
-        bool PrintPosError(bool add, size_t pos, size_t esize) const
-        {
-            sLog.outError("ERROR: Attempt %s in ByteBuffer (pos: %lu size: %lu) value with size: %lu",(add ? "put" : "get"),(unsigned long)pos, (unsigned long)size(), (unsigned long)esize);
-
-            // assert must fail after function call
-            return false;
-        }
-
         size_t _rpos, _wpos;
         std::vector<uint8> _storage;
 };