[8754] Some guld back related packets data correctness checks

* Move when possible checks from deep code to packer/DB read level.
* Check real existed guild bank tabs amount instead just max possible amount.
* Check guild bank loaded state for packets expected to be called only after
  guild bank data loadin by another packet.

Thanks to elecyb and NetSky for original fix versions.

src/game/Guild.cpp

@@ -951,15 +951,13 @@ void Guild::LogGuildEvent(uint8 EventType, uint32 PlayerGuid1, uint32 PlayerGuid
 // Bank content related
 void Guild::DisplayGuildBankContent(WorldSession *session, uint8 TabId)
 {
-    WorldPacket data(SMSG_GUILD_BANK_LIST,1200);
+    GuildBankTab const* tab = m_TabListMap[TabId];
-
-    GuildBankTab const* tab = GetBankTab(TabId);
-    if (!tab)
-        return;
 
     if (!IsMemberHaveRights(session->GetPlayer()->GetGUIDLow(),TabId,GUILD_BANK_RIGHT_VIEW_TAB))
         return;
 
+    WorldPacket data(SMSG_GUILD_BANK_LIST,1200);
+
     data << uint64(GetGuildBankMoney());
     data << uint8(TabId);
                                                             // remaining slots for today
@@ -992,9 +990,7 @@ void Guild::DisplayGuildBankMoneyUpdate()
 
 void Guild::DisplayGuildBankContentUpdate(uint8 TabId, int32 slot1, int32 slot2)
 {
-    GuildBankTab const* tab = GetBankTab(TabId);
+    GuildBankTab const* tab = m_TabListMap[TabId];
-    if (!tab)
-        return;
 
     WorldPacket data(SMSG_GUILD_BANK_LIST,1200);
 
@@ -1042,9 +1038,7 @@ void Guild::DisplayGuildBankContentUpdate(uint8 TabId, int32 slot1, int32 slot2)
 
 void Guild::DisplayGuildBankContentUpdate(uint8 TabId, GuildItemPosCountVec const& slots)
 {
-    GuildBankTab const* tab = GetBankTab(TabId);
+    GuildBankTab const* tab = m_TabListMap[TabId];
-    if (!tab)
-        return;
 
     WorldPacket data(SMSG_GUILD_BANK_LIST,1200);
 
@@ -1134,14 +1128,6 @@ void Guild::CreateNewBankTab()
 
 void Guild::SetGuildBankTabInfo(uint8 TabId, std::string Name, std::string Icon)
 {
-    if (TabId >= GUILD_BANK_MAX_TABS)
-        return;
-    if (TabId >= m_TabListMap.size())
-        return;
-
-    if (!m_TabListMap[TabId])
-        return;
-
     if (m_TabListMap[TabId]->Name == Name && m_TabListMap[TabId]->Icon == Icon)
         return;
 
@@ -1993,12 +1979,7 @@ void Guild::SetGuildBankTabText(uint8 TabId, std::string text)
 
 void Guild::SendGuildBankTabText(WorldSession *session, uint8 TabId)
 {
-    if (TabId > GUILD_BANK_MAX_TABS)
+    GuildBankTab const* tab = m_TabListMap[TabId];
-        return;
-
-    GuildBankTab const *tab = GetBankTab(TabId);
-    if (!tab)
-        return;
 
     WorldPacket data(MSG_QUERY_GUILD_BANK_TEXT, 1+tab->Text.size()+1);
     data << uint8(TabId);

src/game/Guild.h

@@ -400,7 +400,6 @@ class Guild
         void   SetGuildBankTabText(uint8 TabId, std::string text);
         void   SendGuildBankTabText(WorldSession *session, uint8 TabId);
         void   SetGuildBankTabInfo(uint8 TabId, std::string name, std::string icon);
-        const  GuildBankTab *GetBankTab(uint8 index) { if(index >= m_TabListMap.size()) return NULL; return m_TabListMap[index]; }
         uint8  GetPurchasedTabs() const { return m_PurchasedTabs; }
         uint32 GetBankRights(uint32 rankId, uint8 TabId) const;
         bool   IsMemberHaveRights(uint32 LowGuid, uint8 TabId,uint32 rights) const;
@@ -408,6 +407,7 @@ class Guild
         // Load/unload
         void   LoadGuildBankFromDB();
         void   UnloadGuildBank();
+        bool   IsGuildBankLoaded() const { return m_GuildBankLoaded; }
         void   IncOnlineMemberCount() { ++m_OnlineMembers; }
         // Money deposit/withdraw
         void   SendMoneyInfo(WorldSession *session, uint32 LowGuid);

src/game/GuildHandler.cpp

@@ -859,7 +859,7 @@ void WorldSession::HandleGuildBankerActivate( WorldPacket & recv_data )
     {
         if(Guild *pGuild = objmgr.GetGuildById(GuildId))
         {
-            pGuild->DisplayGuildBankTabsInfo(this);
+            pGuild->DisplayGuildBankTabsInfo(this);         // this also will load guild bank if not yet
             return;
         }
     }
@@ -879,17 +879,21 @@ void WorldSession::HandleGuildBankQueryTab( WorldPacket & recv_data )
     if (!GetPlayer()->GetGameObjectIfCanInteractWith(GoGuid, GAMEOBJECT_TYPE_GUILD_BANK))
         return;
 
-    if(uint32 GuildId = GetPlayer()->GetGuildId())
+    uint32 GuildId = GetPlayer()->GetGuildId();
-    {
+    if (!GuildId)
-        if(Guild *pGuild = objmgr.GetGuildById(GuildId))
+        return;
-        {
-            // Let's update the amount of gold the player can withdraw before displaying the content
-            // This is useful if money withdraw right has changed
-            pGuild->SendMoneyInfo(this, GetPlayer()->GetGUIDLow());
 
-            pGuild->DisplayGuildBankContent(this, TabId);
+    Guild *pGuild = objmgr.GetGuildById(GuildId);
-        }
+    if (!pGuild)
-    }
+        return;
+
+    if (!pGuild->IsGuildBankLoaded() || TabId >= pGuild->GetPurchasedTabs())
+        return;
+
+    // Let's update the amount of gold the player can withdraw before displaying the content
+    // This is useful if money withdraw right has changed
+    pGuild->SendMoneyInfo(this, GetPlayer()->GetGUIDLow());
+    pGuild->DisplayGuildBankContent(this, TabId);
 }
 
 void WorldSession::HandleGuildBankDepositMoney( WorldPacket & recv_data )
@@ -909,33 +913,38 @@ void WorldSession::HandleGuildBankDepositMoney( WorldPacket & recv_data )
     if (GetPlayer()->GetMoney() < money)
         return;
 
-    if(uint32 GuildId = GetPlayer()->GetGuildId())
+    uint32 GuildId = GetPlayer()->GetGuildId();
+    if (!GuildId)
+        return;
+
+    Guild *pGuild = objmgr.GetGuildById(GuildId);
+    if (!pGuild)
+        return;
+
+    if (!pGuild->IsGuildBankLoaded() || !pGuild->GetPurchasedTabs())
+        return;
+
+    CharacterDatabase.BeginTransaction();
+
+    pGuild->SetBankMoney(pGuild->GetGuildBankMoney()+money);
+    GetPlayer()->ModifyMoney(-int(money));
+    GetPlayer()->SaveGoldToDB();
+
+    CharacterDatabase.CommitTransaction();
+
+    // logging money
+    if(_player->GetSession()->GetSecurity() > SEC_PLAYER && sWorld.getConfig(CONFIG_GM_LOG_TRADE))
     {
-        if(Guild *pGuild = objmgr.GetGuildById(GuildId))
+        sLog.outCommand(_player->GetSession()->GetAccountId(),"GM %s (Account: %u) deposit money (Amount: %u) to guild bank (Guild ID %u)",
-        {
+            _player->GetName(),_player->GetSession()->GetAccountId(),money,GuildId);
-            CharacterDatabase.BeginTransaction();
-
-            pGuild->SetBankMoney(pGuild->GetGuildBankMoney()+money);
-            GetPlayer()->ModifyMoney(-int(money));
-            GetPlayer()->SaveGoldToDB();
-
-            CharacterDatabase.CommitTransaction();
-
-            // logging money
-            if(_player->GetSession()->GetSecurity() > SEC_PLAYER && sWorld.getConfig(CONFIG_GM_LOG_TRADE))
-            {
-                sLog.outCommand(_player->GetSession()->GetAccountId(),"GM %s (Account: %u) deposit money (Amount: %u) to guild bank (Guild ID %u)",
-                    _player->GetName(),_player->GetSession()->GetAccountId(),money,GuildId);
-            }
-
-            // log
-            pGuild->LogBankEvent(GUILD_BANK_LOG_DEPOSIT_MONEY, uint8(0), GetPlayer()->GetGUIDLow(), money);
-
-            pGuild->DisplayGuildBankTabsInfo(this);
-            pGuild->DisplayGuildBankContent(this, 0);
-            pGuild->DisplayGuildBankMoneyUpdate();
-        }
     }
+
+    // log
+    pGuild->LogBankEvent(GUILD_BANK_LOG_DEPOSIT_MONEY, uint8(0), GetPlayer()->GetGUIDLow(), money);
+
+    pGuild->DisplayGuildBankTabsInfo(this);
+    pGuild->DisplayGuildBankContent(this, 0);
+    pGuild->DisplayGuildBankMoneyUpdate();
 }
 
 void WorldSession::HandleGuildBankWithdrawMoney( WorldPacket & recv_data )
@@ -960,6 +969,9 @@ void WorldSession::HandleGuildBankWithdrawMoney( WorldPacket & recv_data )
     if(!pGuild)
         return;
 
+    if (!pGuild->IsGuildBankLoaded() || !pGuild->GetPurchasedTabs())
+        return;
+
     if (pGuild->GetGuildBankMoney()<money)                  // not enough money in bank
         return;
 
@@ -1005,6 +1017,21 @@ void WorldSession::HandleGuildBankSwapItems( WorldPacket & recv_data )
     uint32 SplitedAmount = 0;
 
     recv_data >> GoGuid >> BankToBank;
+
+    uint32 GuildId = GetPlayer()->GetGuildId();
+    if (!GuildId)
+    {
+        recv_data.rpos(recv_data.wpos());                   // prevent additional spam at rejected packet
+        return;
+    }
+
+    Guild *pGuild = objmgr.GetGuildById(GuildId);
+    if (!pGuild || !pGuild->IsGuildBankLoaded())
+    {
+        recv_data.rpos(recv_data.wpos());                   // prevent additional spam at rejected packet
+        return;
+    }
+
     if (BankToBank)
     {
         recv_data >> BankTabDst;
@@ -1016,7 +1043,10 @@ void WorldSession::HandleGuildBankSwapItems( WorldPacket & recv_data )
         recv_data >> unk2;                                  // always 0
         recv_data >> SplitedAmount;
 
-        if (BankTabSlotDst >= GUILD_BANK_MAX_SLOTS || (BankTabDst == BankTab && BankTabSlotDst == BankTabSlot))
+        if (BankTabSlotDst >= GUILD_BANK_MAX_SLOTS ||
+            (BankTabDst == BankTab && BankTabSlotDst == BankTabSlot) ||
+            BankTab >= pGuild->GetPurchasedTabs() ||
+            BankTabDst >= pGuild->GetPurchasedTabs())
         {
             recv_data.rpos(recv_data.wpos());               // prevent additional spam at rejected packet
             return;
@@ -1042,7 +1072,8 @@ void WorldSession::HandleGuildBankSwapItems( WorldPacket & recv_data )
             recv_data >> SplitedAmount;
         }
 
-        if (BankTabSlot >= GUILD_BANK_MAX_SLOTS && BankTabSlot != 0xFF)
+        if (BankTabSlot >= GUILD_BANK_MAX_SLOTS && BankTabSlot != 0xFF ||
+            BankTab >= pGuild->GetPurchasedTabs())
         {
             recv_data.rpos(recv_data.wpos());               // prevent additional spam at rejected packet
             return;
@@ -1052,33 +1083,27 @@ void WorldSession::HandleGuildBankSwapItems( WorldPacket & recv_data )
     if (!GetPlayer()->GetGameObjectIfCanInteractWith(GoGuid, GAMEOBJECT_TYPE_GUILD_BANK))
         return;
 
-    if(uint32 GuildId = GetPlayer()->GetGuildId())
+    // Bank <-> Bank
+    if (BankToBank)
     {
-        if(Guild *pGuild = objmgr.GetGuildById(GuildId))
+        pGuild->SwapItems(_player, BankTab, BankTabSlot, BankTabDst, BankTabSlotDst, SplitedAmount);
-        {
+        return;
-            // Bank <-> Bank
-            if (BankToBank)
-            {
-                pGuild->SwapItems(_player, BankTab, BankTabSlot, BankTabDst, BankTabSlotDst, SplitedAmount);
-                return;
-            }
-
-            // Player <-> Bank
-
-            // allow work with inventory only
-            if(!Player::IsInventoryPos(PlayerBag, PlayerSlot) && !(PlayerBag == NULL_BAG && PlayerSlot == NULL_SLOT) )
-            {
-                _player->SendEquipError( EQUIP_ERR_NONE, NULL, NULL );
-                return;
-            }
-
-            // BankToChar swap or char to bank remaining
-            if (ToChar)                                             // Bank -> Char cases
-                pGuild->MoveFromBankToChar(_player, BankTab, BankTabSlot, PlayerBag, PlayerSlot, SplitedAmount);
-            else                                                    // Char -> Bank cases
-                pGuild->MoveFromCharToBank(_player, PlayerBag, PlayerSlot, BankTab, BankTabSlot, SplitedAmount);
-        }
     }
+
+    // Player <-> Bank
+
+    // allow work with inventory only
+    if(!Player::IsInventoryPos(PlayerBag, PlayerSlot) && !(PlayerBag == NULL_BAG && PlayerSlot == NULL_SLOT) )
+    {
+        _player->SendEquipError( EQUIP_ERR_NONE, NULL, NULL );
+        return;
+    }
+
+    // BankToChar swap or char to bank remaining
+    if (ToChar)                                             // Bank -> Char cases
+        pGuild->MoveFromBankToChar(_player, BankTab, BankTabSlot, PlayerBag, PlayerSlot, SplitedAmount);
+    else                                                    // Char -> Bank cases
+        pGuild->MoveFromCharToBank(_player, PlayerBag, PlayerSlot, BankTab, BankTabSlot, SplitedAmount);
 }
 
 void WorldSession::HandleGuildBankBuyTab( WorldPacket & recv_data )
@@ -1095,26 +1120,21 @@ void WorldSession::HandleGuildBankBuyTab( WorldPacket & recv_data )
         return;
 
     uint32 GuildId = GetPlayer()->GetGuildId();
-    if (GuildId==0)
+    if (!GuildId)
         return;
 
     Guild *pGuild = objmgr.GetGuildById(GuildId);
     if(!pGuild)
         return;
 
+    // m_PurchasedTabs = 0 when buying Tab 0, that is why this check can be made
+    if (!pGuild->IsGuildBankLoaded() || TabId != pGuild->GetPurchasedTabs())
+        return;
+
     uint32 TabCost = GetGuildBankTabPrice(TabId) * GOLD;
     if (!TabCost)
         return;
 
-    if (pGuild->GetPurchasedTabs() >= GUILD_BANK_MAX_TABS)
-        return;
-
-    if (TabId != pGuild->GetPurchasedTabs())                // m_PurchasedTabs = 0 when buying Tab 0, that is why this check can be made
-    {
-        sLog.outError("Error: trying to buy a tab non contiguous to owned ones");
-        return;
-    }
-
     if (GetPlayer()->GetMoney() < TabCost)                  // Should not happen, this is checked by client
         return;
 
@@ -1149,15 +1169,20 @@ void WorldSession::HandleGuildBankUpdateTab( WorldPacket & recv_data )
     if (!GetPlayer()->GetGameObjectIfCanInteractWith(GoGuid, GAMEOBJECT_TYPE_GUILD_BANK))
         return;
 
-    if(uint32 GuildId = GetPlayer()->GetGuildId())
+    uint32 GuildId = GetPlayer()->GetGuildId();
-    {
+    if (!GuildId)
-        if(Guild *pGuild = objmgr.GetGuildById(GuildId))
+        return;
-        {
+
-            pGuild->SetGuildBankTabInfo(TabId, Name, IconIndex);
+    Guild *pGuild = objmgr.GetGuildById(GuildId);
-            pGuild->DisplayGuildBankTabsInfo(this);
+    if (!pGuild)
-            pGuild->DisplayGuildBankContent(this, TabId);
+        return;
-        }
+
-    }
+    if (!pGuild->IsGuildBankLoaded() || TabId >= pGuild->GetPurchasedTabs())
+        return;
+
+    pGuild->SetGuildBankTabInfo(TabId, Name, IconIndex);
+    pGuild->DisplayGuildBankTabsInfo(this);
+    pGuild->DisplayGuildBankContent(this, TabId);
 }
 
 void WorldSession::HandleGuildBankLogQuery( WorldPacket & recv_data )
@@ -1167,9 +1192,22 @@ void WorldSession::HandleGuildBankLogQuery( WorldPacket & recv_data )
     uint8 TabId;
     recv_data >> TabId;
 
-    if(uint32 GuildId = GetPlayer()->GetGuildId())
+    uint32 GuildId = GetPlayer()->GetGuildId();
-        if(Guild *pGuild = objmgr.GetGuildById(GuildId))
+    if (!GuildId)
-            pGuild->DisplayGuildBankLogs(this, TabId);
+        return;
+
+    Guild *pGuild = objmgr.GetGuildById(GuildId);
+    if (!pGuild)
+        return;
+
+    if (!pGuild->IsGuildBankLoaded())
+        return;
+
+    // GUILD_BANK_MAX_TABS send by client for money log
+    if (TabId >= pGuild->GetPurchasedTabs() && TabId != GUILD_BANK_MAX_TABS)
+        return;
+
+    pGuild->DisplayGuildBankLogs(this, TabId);
 }
 
 void WorldSession::HandleQueryGuildBankTabText(WorldPacket &recv_data)
@@ -1179,9 +1217,18 @@ void WorldSession::HandleQueryGuildBankTabText(WorldPacket &recv_data)
     uint8 TabId;
     recv_data >> TabId;
 
-    if(uint32 GuildId = GetPlayer()->GetGuildId())
+    uint32 GuildId = GetPlayer()->GetGuildId();
-        if(Guild *pGuild = objmgr.GetGuildById(GuildId))
+    if (!GuildId)
-            pGuild->SendGuildBankTabText(this, TabId);
+        return;
+
+    Guild *pGuild = objmgr.GetGuildById(GuildId);
+    if (!pGuild)
+        return;
+
+    if (!pGuild->IsGuildBankLoaded() || TabId >= pGuild->GetPurchasedTabs())
+        return;
+
+    pGuild->SendGuildBankTabText(this, TabId);
 }
 
 void WorldSession::HandleSetGuildBankTabText(WorldPacket &recv_data)
@@ -1193,9 +1240,18 @@ void WorldSession::HandleSetGuildBankTabText(WorldPacket &recv_data)
     recv_data >> TabId;
     recv_data >> Text;
 
-    if(uint32 GuildId = GetPlayer()->GetGuildId())
+    uint32 GuildId = GetPlayer()->GetGuildId();
-        if(Guild *pGuild = objmgr.GetGuildById(GuildId))
+    if (!GuildId)
-            pGuild->SetGuildBankTabText(TabId, Text);
+        return;
+
+    Guild *pGuild = objmgr.GetGuildById(GuildId);
+    if (!pGuild)
+        return;
+
+    if (!pGuild->IsGuildBankLoaded() || TabId >= pGuild->GetPurchasedTabs())
+        return;
+
+    pGuild->SetGuildBankTabText(TabId, Text);
 }
 
 void WorldSession::SendSaveGuildEmblem( uint32 msg )

src/game/SharedDefines.h

@@ -2387,10 +2387,10 @@ enum PetDiet
 #define CHAIN_SPELL_JUMP_RADIUS 10
 
 // Max values for Guild & Guild Bank
-#define GUILD_BANK_MAX_TABS         6
+#define GUILD_BANK_MAX_TABS         6                       // send by client for money log also
 #define GUILD_BANK_MAX_SLOTS        98
 #define GUILD_BANK_MAX_LOGS         25
-#define GUILD_BANK_MONEY_LOGS_TAB   100
+#define GUILD_BANK_MONEY_LOGS_TAB   100                     // used for money log in DB
 #define GUILD_EVENTLOG_MAX_RECORDS  100
 #define GUILD_RANKS_MIN_COUNT       5
 #define GUILD_RANKS_MAX_COUNT       10

src/shared/revision_nr.h

@@ -1,4 +1,4 @@
 #ifndef __REVISION_NR_H__
 #define __REVISION_NR_H__
- #define REVISION_NR "8753"
+ #define REVISION_NR "8754"
 #endif // __REVISION_NR_H__