Skip to content

Commit

Permalink
Fix 'use after free' when deleting battlegrounds.
Browse files Browse the repository at this point in the history
BattleGroundMap::Update() is calling Update() on the BattleGround
associated with the map, which is typically the Update() call of a
derived class like BattleGroundAB. These start by calling the base
function BattleGround::Update(). So when BattleGround::Update() decides
to delete the battleground and return, this is kind of sawing off the
branch we're still sitting on! If the object gets deleted, the Update()
call of the derived class can't doing anything any more, but in practice
it was happily continuing with its own Update() code, leading to all
kinds of nasty things (and usually a crash).
  • Loading branch information
evil-at-wow committed Jul 23, 2023
1 parent 091374d commit 2c987ba
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 19 deletions.
18 changes: 0 additions & 18 deletions src/game/BattleGround/BattleGround.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -285,24 +285,6 @@ BattleGround::~BattleGround()
*/
void BattleGround::Update(uint32 diff)
{
if (!GetPlayersSize())
{
// BG is empty
// if there are no players invited, delete BG
// this will delete arena or bg object, where any player entered
// [[ but if you use battleground object again (more battles possible to be played on 1 instance)
// then this condition should be removed and code:
// if (!GetInvitedCount(HORDE) && !GetInvitedCount(ALLIANCE))
// this->AddToFreeBGObjectsQueue(); // not yet implemented
// should be used instead of current
// ]]
// BattleGround Template instance cannot be updated, because it would be deleted
if (!GetInvitedCount(HORDE) && !GetInvitedCount(ALLIANCE))
delete this;

return;
}

// remove offline players from bg after 5 minutes
if (!m_offlineQueue.empty())
{
Expand Down
18 changes: 17 additions & 1 deletion src/game/Maps/Map.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1887,7 +1887,23 @@ void BattleGroundMap::Update(const uint32& diff)
{
Map::Update(diff);

GetBG()->Update(diff);
if (!m_bg->GetPlayersSize())
{
// BG is empty
// if there are no players invited, delete BG
// this will delete arena or bg object, where any player entered
// [[ but if you use battleground object again (more battles possible to be played on 1 instance)
// then this condition should be removed and code:
// if (!GetInvitedCount(HORDE) && !GetInvitedCount(ALLIANCE))
// this->AddToFreeBGObjectsQueue(); // not yet implemented
// should be used instead of current
// ]]
// BattleGround Template instance cannot be updated, because it would be deleted
if (!m_bg->GetInvitedCount(HORDE) && !m_bg->GetInvitedCount(ALLIANCE))
delete m_bg;
}
else
m_bg->Update(diff);
}

BattleGroundPersistentState* BattleGroundMap::GetPersistanceState() const
Expand Down

0 comments on commit 2c987ba

Please sign in to comment.