Core/AuctionHouse: Fixed possible use after free when auctions are removed with offline buyers

3 files changed, 19 insertions, 11 deletions

diff --git a/src/server/game/AuctionHouse/AuctionHouseMgr.cpp b/src/server/game/AuctionHouse/AuctionHouseMgr.cpp
index 639fd280074..a89cca92af9 100644
--- a/src/server/game/AuctionHouse/AuctionHouseMgr.cpp
+++ b/src/server/game/AuctionHouse/AuctionHouseMgr.cpp
@@ -1130,20 +1130,25 @@ void AuctionHouseObject::Update()
         {
             SendAuctionExpired(auction, trans);
             sScriptMgr->OnAuctionExpire(this, auction);
+
+            RemoveAuction(trans, auction, &it);
         }
         ///- 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);
+
             //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
-            SendAuctionWon(auction, nullptr, trans);
-            SendAuctionSold(auction, nullptr, trans);
+            SendAuctionSold(&copy, nullptr, trans);
+            SendAuctionWon(&copy, nullptr, trans);
             sScriptMgr->OnAuctionSuccessful(this, auction);
         }
-
-        ///- In any case clear the auction
-        RemoveAuction(trans, auction, &it);
     }
 
     // Run DB changes
diff --git a/src/server/game/AuctionHouseBot/AuctionHouseBotBuyer.cpp b/src/server/game/AuctionHouseBot/AuctionHouseBotBuyer.cpp
index 0508951226f..95e98c53a6f 100644
--- a/src/server/game/AuctionHouseBot/AuctionHouseBotBuyer.cpp
+++ b/src/server/game/AuctionHouseBot/AuctionHouseBotBuyer.cpp
@@ -412,13 +412,16 @@ void AuctionBotBuyer::BuyEntry(AuctionPosting* auction, AuctionHouseObject* auct
     auction->Bidder = newBidder;
     auction->BidAmount = auction->BuyoutOrUnitPrice;
 
-    // Mails must be under transaction control too to prevent data loss
-    auctionHouse->SendAuctionWon(auction, nullptr, trans);
-    auctionHouse->SendAuctionSold(auction, nullptr, trans);
-
-    // Remove auction
+    // 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);
 
+    // Mails must be under transaction control too to prevent data loss
+    auctionHouse->SendAuctionSold(&copy, nullptr, trans);
+    auctionHouse->SendAuctionWon(&copy, nullptr, trans);
+
     // Run SQLs
     CharacterDatabase.CommitTransaction(trans);
 }
diff --git a/src/server/game/Handlers/AuctionHouseHandler.cpp b/src/server/game/Handlers/AuctionHouseHandler.cpp
index 2082b67d451..666a1e00150 100644
--- a/src/server/game/Handlers/AuctionHouseHandler.cpp
+++ b/src/server/game/Handlers/AuctionHouseHandler.cpp
@@ -438,8 +438,8 @@ void WorldSession::HandleAuctionPlaceBid(WorldPackets::AuctionHouse::AuctionPlac
     if (canBuyout && placeBid.BidAmount == auction->BuyoutOrUnitPrice)
     {
         // buyout
-        auctionHouse->SendAuctionWon(auction, player, trans);
         auctionHouse->SendAuctionSold(auction, nullptr, trans);
+        auctionHouse->SendAuctionWon(auction, player, trans);
 
         auctionHouse->RemoveAuction(trans, auction);
     }