Core/AuctionHouse: Fixed use after free

Closes #30128

4 files changed, 31 insertions, 25 deletions

diff --git a/src/server/game/AuctionHouse/AuctionHouseMgr.cpp b/src/server/game/AuctionHouse/AuctionHouseMgr.cpp
index 09de6c3b0ef..bcc90f21ee4 100644
--- a/src/server/game/AuctionHouse/AuctionHouseMgr.cpp
+++ b/src/server/game/AuctionHouse/AuctionHouseMgr.cpp
@@ -1009,7 +1009,8 @@ void AuctionHouseObject::AddAuction(CharacterDatabaseTransaction trans, AuctionP
     sScriptMgr->OnAuctionAdd(this, addedAuction);
 }
 
-void AuctionHouseObject::RemoveAuction(CharacterDatabaseTransaction trans, AuctionPosting* auction, std::map<uint32, AuctionPosting>::iterator* auctionItr /*= nullptr*/)
+std::map<uint32, AuctionPosting>::node_type AuctionHouseObject::RemoveAuction(CharacterDatabaseTransaction trans, AuctionPosting* auction,
+    std::map<uint32, AuctionPosting>::iterator* auctionItr /*= nullptr*/)
 {
     AuctionsBucketData* bucket = auction->Bucket;
 
@@ -1068,7 +1069,10 @@ void AuctionHouseObject::RemoveAuction(CharacterDatabaseTransaction trans, Aucti
             bucket->QualityMask &= static_cast<AuctionHouseFilterMask>(~(1 << (quality + 4)));
     }
     else
+    {
+        auction->Bucket = nullptr;
         _buckets.erase(bucket->Key);
+    }
 
     CharacterDatabasePreparedStatement* stmt = CharacterDatabase.GetPreparedStatement(CHAR_DEL_AUCTION);
     stmt->setUInt32(0, auction->Id);
@@ -1084,9 +1088,9 @@ void AuctionHouseObject::RemoveAuction(CharacterDatabaseTransaction trans, Aucti
         Trinity::Containers::MultimapErasePair(_playerBidderAuctions, bidder, auction->Id);
 
     if (auctionItr)
-        *auctionItr = _itemsByAuctionId.erase(*auctionItr);
+        return _itemsByAuctionId.extract((*auctionItr)++);
     else
-        _itemsByAuctionId.erase(auction->Id);
+        return _itemsByAuctionId.extract(auction->Id);
 }
 
 void AuctionHouseObject::Update()
@@ -1127,29 +1131,24 @@ void AuctionHouseObject::Update()
             continue;
         }
 
+        std::map<uint32, AuctionPosting>::node_type removedAuctionNode = RemoveAuction(trans, auction, &it);
+        auction = &removedAuctionNode.mapped();
+
         ///- Either cancel the auction if there was no bidder
         if (auction->Bidder.IsEmpty())
         {
-            SendAuctionExpired(auction, trans);
             sScriptMgr->OnAuctionExpire(this, auction);
-
-            RemoveAuction(trans, auction, &it);
+            SendAuctionExpired(auction, trans);
         }
         ///- Or perform the transaction
         else
         {
-            // Copy data before freeing AuctionPosting in auctionHouse->RemoveAuction
-            // Because auctionHouse->SendAuctionWon can unload items if bidder is offline
-            // we need to RemoveAuction before sending mails
-            AuctionPosting copy = *auction;
-            RemoveAuction(trans, auction, &it);
-
+            sScriptMgr->OnAuctionSuccessful(this, auction);
             //we should send an "item sold" message if the seller is online
             //we send the item to the winner
             //we send the money to the seller
-            SendAuctionSold(&copy, nullptr, trans);
-            SendAuctionWon(&copy, nullptr, trans);
-            sScriptMgr->OnAuctionSuccessful(this, auction);
+            SendAuctionSold(auction, nullptr, trans);
+            SendAuctionWon(auction, nullptr, trans);
         }
     }
 
@@ -1823,7 +1822,10 @@ void AuctionHouseObject::SendAuctionWon(AuctionPosting const* auction, Player* b
     {
         // bidder doesn't exist, delete the item
         for (Item* item : auction->Items)
-            sAuctionMgr->RemoveAItem(item->GetGUID(), true, &trans);
+        {
+            item->FSetState(ITEM_REMOVED);
+            item->SaveToDB(trans);
+        }
     }
 }
 
@@ -1879,7 +1881,10 @@ void AuctionHouseObject::SendAuctionExpired(AuctionPosting const* auction, Chara
     {
         // owner doesn't exist, delete the item
         for (Item* item : auction->Items)
-            sAuctionMgr->RemoveAItem(item->GetGUID(), true, &trans);
+        {
+            item->FSetState(ITEM_REMOVED);
+            item->SaveToDB(trans);
+        }
     }
 }
 
diff --git a/src/server/game/AuctionHouse/AuctionHouseMgr.h b/src/server/game/AuctionHouse/AuctionHouseMgr.h
index f6354060022..1ac82af2e4d 100644
--- a/src/server/game/AuctionHouse/AuctionHouseMgr.h
+++ b/src/server/game/AuctionHouse/AuctionHouseMgr.h
@@ -291,7 +291,8 @@ public:
 
     void AddAuction(CharacterDatabaseTransaction trans, AuctionPosting auction);
 
-    void RemoveAuction(CharacterDatabaseTransaction trans, AuctionPosting* auction, std::map<uint32, AuctionPosting>::iterator* auctionItr = nullptr);
+    std::map<uint32, AuctionPosting>::node_type RemoveAuction(CharacterDatabaseTransaction trans, AuctionPosting* auction,
+        std::map<uint32, AuctionPosting>::iterator* auctionItr = nullptr);
 
     void Update();
 
diff --git a/src/server/game/AuctionHouseBot/AuctionHouseBotBuyer.cpp b/src/server/game/AuctionHouseBot/AuctionHouseBotBuyer.cpp
index 95e98c53a6f..de91078aa5d 100644
--- a/src/server/game/AuctionHouseBot/AuctionHouseBotBuyer.cpp
+++ b/src/server/game/AuctionHouseBot/AuctionHouseBotBuyer.cpp
@@ -415,12 +415,12 @@ void AuctionBotBuyer::BuyEntry(AuctionPosting* auction, AuctionHouseObject* auct
     // Copy data before freeing AuctionPosting in auctionHouse->RemoveAuction
     // Because auctionHouse->SendAuctionWon can unload items if bidder is offline
     // we need to RemoveAuction before sending mails
-    AuctionPosting copy = *auction;
-    auctionHouse->RemoveAuction(trans, auction);
+    std::map<uint32, AuctionPosting>::node_type removedAuctionNode = auctionHouse->RemoveAuction(trans, auction);
+    auction = &removedAuctionNode.mapped();
 
     // Mails must be under transaction control too to prevent data loss
-    auctionHouse->SendAuctionSold(&copy, nullptr, trans);
-    auctionHouse->SendAuctionWon(&copy, nullptr, trans);
+    auctionHouse->SendAuctionSold(auction, nullptr, trans);
+    auctionHouse->SendAuctionWon(auction, nullptr, trans);
 
     // Run SQLs
     CharacterDatabase.CommitTransaction(trans);
diff --git a/src/server/game/Handlers/AuctionHouseHandler.cpp b/src/server/game/Handlers/AuctionHouseHandler.cpp
index f87e58366b8..3c9d56ef87e 100644
--- a/src/server/game/Handlers/AuctionHouseHandler.cpp
+++ b/src/server/game/Handlers/AuctionHouseHandler.cpp
@@ -438,10 +438,10 @@ void WorldSession::HandleAuctionPlaceBid(WorldPackets::AuctionHouse::AuctionPlac
     if (canBuyout && placeBid.BidAmount == auction->BuyoutOrUnitPrice)
     {
         // buyout
-        auctionHouse->SendAuctionSold(auction, nullptr, trans);
-        auctionHouse->SendAuctionWon(auction, player, trans);
+        std::map<uint32, AuctionPosting>::node_type removedAuctionNode = auctionHouse->RemoveAuction(trans, auction);
 
-        auctionHouse->RemoveAuction(trans, auction);
+        auctionHouse->SendAuctionSold(&removedAuctionNode.mapped(), nullptr, trans);
+        auctionHouse->SendAuctionWon(&removedAuctionNode.mapped(), player, trans);
     }
     else
     {