Core/PacketIO: Added utility packet array class to handle loop counter limiting in packets received from the client

10 files changed, 142 insertions, 95 deletions

diff --git a/src/server/game/Handlers/CalendarHandler.cpp b/src/server/game/Handlers/CalendarHandler.cpp
index 36d04d6772a..e7c8268443a 100644
--- a/src/server/game/Handlers/CalendarHandler.cpp
+++ b/src/server/game/Handlers/CalendarHandler.cpp
@@ -173,36 +173,15 @@ void WorldSession::HandleCalendarAddEvent(WorldPackets::Calendar::CalendarAddEve
     }
     else
     {
-        // client limits the amount of players to be invited to 100
-        ObjectGuid invitee[CALENDAR_MAX_INVITES];
-        uint8 status[CALENDAR_MAX_INVITES];
-        uint8 rank[CALENDAR_MAX_INVITES];
-
-        memset(status, 0, sizeof(status));
-        memset(rank, 0, sizeof(rank));
-        try
-        {
-            for (uint32 i = 0; i < calendarAddEvent.EventInfo.Invites.size() && i < CALENDAR_MAX_INVITES; ++i)
-            {
-                invitee[i] = calendarAddEvent.EventInfo.Invites[i].Guid;
-                status[i] = calendarAddEvent.EventInfo.Invites[i].Status;
-                rank[i] = calendarAddEvent.EventInfo.Invites[i].Moderator;
-            }
-        }
-        catch (ByteBufferException const&)
-        {
-            delete calendarEvent;
-            calendarEvent = NULL;
-            throw;
-        }
-
         SQLTransaction trans;
         if (calendarAddEvent.EventInfo.Invites.size() > 1)
             trans = CharacterDatabase.BeginTransaction();
 
-        for (uint32 i = 0; i < calendarAddEvent.EventInfo.Invites.size() && i < CALENDAR_MAX_INVITES; ++i)
+        for (uint32 i = 0; i < calendarAddEvent.EventInfo.Invites.size(); ++i)
         {
-            CalendarInvite* invite = new CalendarInvite(sCalendarMgr->GetFreeInviteId(), calendarEvent->GetEventId(), invitee[i], guid, CALENDAR_DEFAULT_RESPONSE_TIME, CalendarInviteStatus(status[i]), CalendarModerationRank(rank[i]), "");
+            CalendarInvite* invite = new CalendarInvite(sCalendarMgr->GetFreeInviteId(), calendarEvent->GetEventId(), calendarAddEvent.EventInfo.Invites[i].Guid,
+                guid, CALENDAR_DEFAULT_RESPONSE_TIME, CalendarInviteStatus(calendarAddEvent.EventInfo.Invites[i].Status),
+                CalendarModerationRank(calendarAddEvent.EventInfo.Invites[i].Moderator), "");
             sCalendarMgr->AddInvite(calendarEvent, invite, trans);
         }
 
diff --git a/src/server/game/Handlers/VoidStorageHandler.cpp b/src/server/game/Handlers/VoidStorageHandler.cpp
index 46e2f1270c1..4ad507895a4 100644
--- a/src/server/game/Handlers/VoidStorageHandler.cpp
+++ b/src/server/game/Handlers/VoidStorageHandler.cpp
@@ -95,18 +95,6 @@ void WorldSession::HandleVoidStorageQuery(WorldPackets::VoidStorage::QueryVoidSt
 
 void WorldSession::HandleVoidStorageTransfer(WorldPackets::VoidStorage::VoidStorageTransfer& voidStorageTransfer)
 {
-    if (voidStorageTransfer.DepositsCount > VOID_STORAGE_MAX_DEPOSIT)
-    {
-        TC_LOG_DEBUG("network", "WORLD: HandleVoidStorageTransfer - Player (%s, name: %s) wants to deposit more than 9 items (%u).", _player->GetGUID().ToString().c_str(), _player->GetName().c_str(), voidStorageTransfer.DepositsCount);
-        return;
-    }
-
-    if (voidStorageTransfer.WithdrawalsCount > VOID_STORAGE_MAX_WITHDRAW)
-    {
-        TC_LOG_DEBUG("network", "WORLD: HandleVoidStorageTransfer - Player (%s, name: %s) wants to withdraw more than 9 items (%u).", _player->GetGUID().ToString().c_str(), _player->GetName().c_str(), voidStorageTransfer.WithdrawalsCount);
-        return;
-    }
-
     Creature* unit = _player->GetNPCIfCanInteractWith(voidStorageTransfer.Npc, UNIT_NPC_FLAG_VAULTKEEPER);
     if (!unit)
     {
@@ -120,14 +108,14 @@ void WorldSession::HandleVoidStorageTransfer(WorldPackets::VoidStorage::VoidStor
         return;
     }
 
-    if (voidStorageTransfer.DepositsCount > _player->GetNumOfVoidStorageFreeSlots())
+    if (voidStorageTransfer.Deposits.size() > _player->GetNumOfVoidStorageFreeSlots())
     {
         SendVoidStorageTransferResult(VOID_TRANSFER_ERROR_FULL);
         return;
     }
 
     uint32 freeBagSlots = 0;
-    if (voidStorageTransfer.WithdrawalsCount)
+    if (!voidStorageTransfer.Withdrawals.empty())
     {
         // make this a Player function
         for (uint8 i = INVENTORY_SLOT_BAG_START; i < INVENTORY_SLOT_BAG_END; i++)
@@ -139,13 +127,13 @@ void WorldSession::HandleVoidStorageTransfer(WorldPackets::VoidStorage::VoidStor
                 ++freeBagSlots;
     }
 
-    if (voidStorageTransfer.WithdrawalsCount > freeBagSlots)
+    if (voidStorageTransfer.Withdrawals.size() > freeBagSlots)
     {
         SendVoidStorageTransferResult(VOID_TRANSFER_ERROR_INVENTORY_FULL);
         return;
     }
 
-    if (!_player->HasEnoughMoney(uint64(voidStorageTransfer.DepositsCount * VOID_STORAGE_STORE_ITEM_COST)))
+    if (!_player->HasEnoughMoney(uint64(voidStorageTransfer.Deposits.size() * VOID_STORAGE_STORE_ITEM_COST)))
     {
         SendVoidStorageTransferResult(VOID_TRANSFER_ERROR_NOT_ENOUGH_MONEY);
         return;
@@ -153,7 +141,7 @@ void WorldSession::HandleVoidStorageTransfer(WorldPackets::VoidStorage::VoidStor
 
     std::pair<VoidStorageItem, uint8> depositItems[VOID_STORAGE_MAX_DEPOSIT];
     uint8 depositCount = 0;
-    for (uint32 i = 0; i < voidStorageTransfer.DepositsCount; ++i)
+    for (uint32 i = 0; i < voidStorageTransfer.Deposits.size(); ++i)
     {
         Item* item = _player->GetItemByGuid(voidStorageTransfer.Deposits[i]);
         if (!item)
@@ -181,7 +169,7 @@ void WorldSession::HandleVoidStorageTransfer(WorldPackets::VoidStorage::VoidStor
 
     VoidStorageItem withdrawItems[VOID_STORAGE_MAX_WITHDRAW];
     uint8 withdrawCount = 0;
-    for (uint32 i = 0; i < voidStorageTransfer.WithdrawalsCount; ++i)
+    for (uint32 i = 0; i < voidStorageTransfer.Withdrawals.size(); ++i)
     {
         uint8 slot = 0;
         VoidStorageItem* itemVS = _player->GetVoidStorageItem(voidStorageTransfer.Withdrawals[i].GetCounter(), slot);
diff --git a/src/server/game/Server/Packets/CalendarPackets.cpp b/src/server/game/Server/Packets/CalendarPackets.cpp
index 655d33da73d..ca80093ba00 100644
--- a/src/server/game/Server/Packets/CalendarPackets.cpp
+++ b/src/server/game/Server/Packets/CalendarPackets.cpp
@@ -97,30 +97,37 @@ void WorldPackets::Calendar::CalendarGuildFilter::Read()
     _worldPacket >> MaxRankOrder;
 }
 
-void WorldPackets::Calendar::CalendarAddEvent::Read()
+ByteBuffer& operator>>(ByteBuffer& buffer, WorldPackets::Calendar::CalendarAddEventInviteInfo& invite)
 {
-    uint8 titleLength = _worldPacket.ReadBits(8);
-    uint16 descriptionLength = _worldPacket.ReadBits(11);
+    buffer >> invite.Guid;
+    buffer >> invite.Status;
+    buffer >> invite.Moderator;
+    return buffer;
+}
 
-    _worldPacket >> EventInfo.EventType;
-    _worldPacket >> EventInfo.TextureID;
-    EventInfo.Time = _worldPacket.ReadPackedTime();
-    _worldPacket >> EventInfo.Flags;
-    uint32 count = _worldPacket.read<uint32>();
+ByteBuffer& operator>>(ByteBuffer& buffer, WorldPackets::Calendar::CalendarAddEventInfo& addEventInfo)
+{
+    uint8 titleLength = buffer.ReadBits(8);
+    uint16 descriptionLength = buffer.ReadBits(11);
 
-    EventInfo.Title = _worldPacket.ReadString(titleLength);
-    EventInfo.Description = _worldPacket.ReadString(descriptionLength);
+    buffer >> addEventInfo.EventType;
+    buffer >> addEventInfo.TextureID;
+    addEventInfo.Time = buffer.ReadPackedTime();
+    buffer >> addEventInfo.Flags;
+    addEventInfo.Invites.resize(buffer.read<uint32>());
 
-    for (uint32 i = 0; i < count && i < CALENDAR_MAX_INVITES; i++)
-    {
-        WorldPackets::Calendar::CalendarAddEventInviteInfo invite;
-        _worldPacket >> invite.Guid;
-        _worldPacket >> invite.Status;
-        _worldPacket >> invite.Moderator;
+    addEventInfo.Title = buffer.ReadString(titleLength);
+    addEventInfo.Description = buffer.ReadString(descriptionLength);
 
-        EventInfo.Invites.push_back(invite);
-    }
+    for (WorldPackets::Calendar::CalendarAddEventInviteInfo& invite : addEventInfo.Invites)
+        buffer >> invite;
+
+    return buffer;
+}
 
+void WorldPackets::Calendar::CalendarAddEvent::Read()
+{
+    _worldPacket >> EventInfo;
     _worldPacket >> MaxSize;
 }
 
diff --git a/src/server/game/Server/Packets/CalendarPackets.h b/src/server/game/Server/Packets/CalendarPackets.h
index 023041d427f..d753113e816 100644
--- a/src/server/game/Server/Packets/CalendarPackets.h
+++ b/src/server/game/Server/Packets/CalendarPackets.h
@@ -14,28 +14,14 @@
  * You should have received a copy of the GNU General Public License along
  * with this program. If not, see <http://www.gnu.org/licenses/>.
  */
-/*
- * Copyright (C) 2008-2015 TrinityCore <http://www.trinitycore.org/>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License as published by the
- * Free Software Foundation; either version 2 of the License, or (at your
- * option) any later version.
- *
- * This program is distributed in the hope that it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
- * more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program. If not, see <http://www.gnu.org/licenses/>.
- */
 
 #ifndef CalendarPackets_h__
 #define CalendarPackets_h__
 
 #include "ObjectGuid.h"
 #include "Packet.h"
+#include "PacketUtilities.h"
+#include "CalendarMgr.h"
 
 namespace WorldPackets
 {
@@ -86,7 +72,7 @@ namespace WorldPackets
             int32 TextureID = 0;
             time_t Time = time_t(0);
             uint32 Flags = 0;
-            std::vector<CalendarAddEventInviteInfo> Invites;
+            Array<CalendarAddEventInviteInfo, CALENDAR_MAX_INVITES> Invites;
         };
 
         class CalendarAddEvent final : public ClientPacket
diff --git a/src/server/game/Server/Packets/CharacterPackets.cpp b/src/server/game/Server/Packets/CharacterPackets.cpp
index 9c6bb07f56e..afeb3e2a6b6 100644
--- a/src/server/game/Server/Packets/CharacterPackets.cpp
+++ b/src/server/game/Server/Packets/CharacterPackets.cpp
@@ -316,10 +316,15 @@ WorldPacket const* WorldPackets::Character::GenerateRandomCharacterNameResult::W
     return &_worldPacket;
 }
 
+WorldPackets::Character::ReorderCharacters::ReorderCharacters(WorldPacket&& packet) : ClientPacket(CMSG_REORDER_CHARACTERS, std::move(packet)),
+    Entries(sWorld->getIntConfig(CONFIG_CHARACTERS_PER_REALM))
+{
+
+}
+
 void WorldPackets::Character::ReorderCharacters::Read()
 {
-    uint32 count = std::min<uint32>(_worldPacket.ReadBits(9), sWorld->getIntConfig(CONFIG_CHARACTERS_PER_REALM));
-    Entries.resize(count);
+    Entries.resize(_worldPacket.ReadBits(9));
     for (ReorderInfo& reorderInfo : Entries)
     {
         _worldPacket >> reorderInfo.PlayerGUID;
diff --git a/src/server/game/Server/Packets/CharacterPackets.h b/src/server/game/Server/Packets/CharacterPackets.h
index 10f1053f8ee..018564350b0 100644
--- a/src/server/game/Server/Packets/CharacterPackets.h
+++ b/src/server/game/Server/Packets/CharacterPackets.h
@@ -20,6 +20,7 @@
 
 #include "Packet.h"
 #include "Player.h"
+#include "PacketUtilities.h"
 
 namespace WorldPackets
 {
@@ -351,11 +352,11 @@ namespace WorldPackets
                 uint8 NewPosition = 0;
             };
 
-            ReorderCharacters(WorldPacket&& packet) : ClientPacket(CMSG_REORDER_CHARACTERS, std::move(packet)) { }
+            ReorderCharacters(WorldPacket&& packet);
 
             void Read() override;
 
-            std::list<ReorderInfo> Entries;
+            Array<ReorderInfo> Entries;
         };
 
         class UndeleteCharacter final : public ClientPacket
diff --git a/src/server/game/Server/Packets/PacketUtilities.h b/src/server/game/Server/Packets/PacketUtilities.h
index f92d949d1be..d620938194c 100644
--- a/src/server/game/Server/Packets/PacketUtilities.h
+++ b/src/server/game/Server/Packets/PacketUtilities.h
@@ -21,6 +21,7 @@
 #include "ByteBuffer.h"
 #include <G3D/Vector2.h>
 #include <G3D/Vector3.h>
+#include <sstream>
 
 inline ByteBuffer& operator<<(ByteBuffer& data, G3D::Vector2 const& v)
 {
@@ -48,6 +49,85 @@ inline ByteBuffer& operator>>(ByteBuffer& data, G3D::Vector3& v)
 
 namespace WorldPackets
 {
+    class PacketArrayMaxCapacityException : public ByteBufferException
+    {
+    public:
+        PacketArrayMaxCapacityException(std::size_t requestedSize, std::size_t sizeLimit)
+        {
+            std::ostringstream builder;
+            builder << "Attempted to read more array elements from packet " << requestedSize << " than allowed " << sizeLimit;
+            message().assign(builder.str());
+        }
+    };
+
+    /**
+     * Utility class for automated prevention of loop counter spoofing in client packets
+     */
+    template<typename T, std::size_t N = 1000 /*select a sane default limit*/>
+    class Array
+    {
+        typedef std::vector<T> storage_type;
+
+        typedef typename storage_type::value_type value_type;
+        typedef typename storage_type::size_type size_type;
+        typedef typename storage_type::reference reference;
+        typedef typename storage_type::const_reference const_reference;
+        typedef typename storage_type::iterator iterator;
+        typedef typename storage_type::const_iterator const_iterator;
+
+    public:
+        Array() : _limit(N) { }
+        Array(size_type limit) : _limit(limit) { }
+
+        iterator begin() { return _storage.begin(); }
+        const_iterator begin() const { return _storage.begin(); }
+
+        iterator end() { return _storage.end(); }
+        const_iterator end() const { return _storage.end(); }
+
+        size_type size() const { return _storage.size(); }
+        bool empty() const { return _storage.empty(); }
+
+        reference operator[](size_type i) { return _storage[i]; }
+        const_reference operator[](size_type i) const { return _storage[i]; }
+
+        void resize(size_type newSize)
+        {
+            if (newSize > _limit)
+                throw PacketArrayMaxCapacityException(newSize, _limit);
+
+            _storage.resize(newSize);
+        }
+
+        void reserve(size_type newSize)
+        {
+            if (newSize > _limit)
+                throw PacketArrayMaxCapacityException(newSize, _limit);
+
+            _storage.reserve(newSize);
+        }
+
+        void push_back(value_type const& value)
+        {
+            if (_storage.size() >= _limit)
+                throw PacketArrayMaxCapacityException(newSize, _limit);
+
+            _storage.push_back(value);
+        }
+
+        void push_back(value_type&& value)
+        {
+            if (_storage.size() >= _limit)
+                throw PacketArrayMaxCapacityException(newSize, _limit);
+
+            _storage.push_back(std::forward<value_type>(value));
+        }
+
+    private:
+        storage_type _storage;
+        size_type _limit;
+    };
+
     template <typename T>
     class CompactArray
     {
diff --git a/src/server/game/Server/Packets/VoidStoragePackets.cpp b/src/server/game/Server/Packets/VoidStoragePackets.cpp
index a635d57666d..43a1f070d8d 100644
--- a/src/server/game/Server/Packets/VoidStoragePackets.cpp
+++ b/src/server/game/Server/Packets/VoidStoragePackets.cpp
@@ -65,17 +65,14 @@ WorldPacket const* WorldPackets::VoidStorage::VoidStorageContents::Write()
 void WorldPackets::VoidStorage::VoidStorageTransfer::Read()
 {
     _worldPacket >> Npc;
-    _worldPacket >> DepositsCount;
-    _worldPacket >> WithdrawalsCount;
+    Deposits.resize(_worldPacket.read<uint32>());
+    Withdrawals.resize(_worldPacket.read<uint32>());
 
-    if (WithdrawalsCount > VOID_STORAGE_MAX_WITHDRAW || DepositsCount > VOID_STORAGE_MAX_DEPOSIT)
-        return;
+    for (ObjectGuid& deposit : Deposits)
+        _worldPacket >> deposit;
 
-    for (uint32 i = 0; i < DepositsCount; ++i)
-        _worldPacket >> Deposits[i];
-
-    for (uint32 i = 0; i < WithdrawalsCount; ++i)
-        _worldPacket >> Withdrawals[i];
+    for (ObjectGuid& withdrawal : Withdrawals)
+        _worldPacket >> withdrawal;
 }
 
 WorldPacket const* WorldPackets::VoidStorage::VoidStorageTransferChanges::Write()
diff --git a/src/server/game/Server/Packets/VoidStoragePackets.h b/src/server/game/Server/Packets/VoidStoragePackets.h
index 9790b328e72..b84f0cf2678 100644
--- a/src/server/game/Server/Packets/VoidStoragePackets.h
+++ b/src/server/game/Server/Packets/VoidStoragePackets.h
@@ -90,10 +90,8 @@ namespace WorldPackets
 
             void Read() override;
 
-            std::array<ObjectGuid, VOID_STORAGE_MAX_WITHDRAW> Withdrawals;
-            uint32 WithdrawalsCount = 0;
-            std::array<ObjectGuid, VOID_STORAGE_MAX_DEPOSIT> Deposits;
-            uint32 DepositsCount = 0;
+            Array<ObjectGuid, VOID_STORAGE_MAX_WITHDRAW> Withdrawals;
+            Array<ObjectGuid, VOID_STORAGE_MAX_DEPOSIT> Deposits;
             ObjectGuid Npc;
         };
 
diff --git a/src/server/game/Server/WorldSession.cpp b/src/server/game/Server/WorldSession.cpp
index d9bd2d8ecc6..bc5b2ef69b6 100644
--- a/src/server/game/Server/WorldSession.cpp
+++ b/src/server/game/Server/WorldSession.cpp
@@ -48,6 +48,7 @@
 #include "ClientConfigPackets.h"
 #include "MiscPackets.h"
 #include "ChatPackets.h"
+#include "PacketUtilities.h"
 
 #include <zlib.h>
 
@@ -427,6 +428,11 @@ bool WorldSession::Update(uint32 diff, PacketFilter& updater)
                     break;
             }
         }
+        catch (WorldPackets::PacketArrayMaxCapacityException const& pamce)
+        {
+            TC_LOG_ERROR("network", "PacketArrayMaxCapacityException: %s while parsing %s from %s.",
+                pamce.what(), GetOpcodeNameForLogging(static_cast<OpcodeClient>(packet->GetOpcode())).c_str(), GetPlayerInfo().c_str());
+        }
         catch (ByteBufferException const&)
         {
             TC_LOG_ERROR("network", "WorldSession::Update ByteBufferException occured while parsing a packet (opcode: %u) from client %s, accountid=%i. Skipped packet.",