From eacd2c2cdeb1cc85d1e44cf5240e636697810a98 Mon Sep 17 00:00:00 2001 From: E Spause Date: Sun, 19 Jul 2020 02:30:50 -0400 Subject: [PATCH 1/3] Fix null pointers in group/raid pointer removal --- zone/entity.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/zone/entity.cpp b/zone/entity.cpp index b00735e4a..4cccd8a52 100644 --- a/zone/entity.cpp +++ b/zone/entity.cpp @@ -2476,7 +2476,7 @@ void EntityList::RemoveAllGroups() while (group_list.size()) { auto group = group_list.front(); group_list.pop_front(); - delete group; + safe_delete(group); } #if EQDEBUG >= 5 CheckGroupList (__FILE__, __LINE__); @@ -2488,7 +2488,7 @@ void EntityList::RemoveAllRaids() while (raid_list.size()) { auto raid = raid_list.front(); raid_list.pop_front(); - delete raid; + safe_delete(raid); } } @@ -2838,7 +2838,7 @@ bool EntityList::RemoveGroup(uint32 delete_id) } auto group = *it; group_list.erase(it); - delete group; + safe_delete(group); return true; } @@ -2850,7 +2850,7 @@ bool EntityList::RemoveRaid(uint32 delete_id) return false; auto raid = *it; raid_list.erase(it); - delete raid; + safe_delete(raid); return true; } From 42781036a8dc76ce1c1353ec3c71f31fe964242f Mon Sep 17 00:00:00 2001 From: E Spause Date: Sun, 19 Jul 2020 02:31:22 -0400 Subject: [PATCH 2/3] Fix issue where npc_types could become null in the cache but remain referenced, causing a crash due to invalid memory accessed --- zone/zone.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/zone/zone.cpp b/zone/zone.cpp index c9342f305..a98c70c73 100755 --- a/zone/zone.cpp +++ b/zone/zone.cpp @@ -773,12 +773,14 @@ void Zone::Shutdown(bool quiet) while (!zone->npctable.empty()) { itr = zone->npctable.begin(); delete itr->second; + itr->second = nullptr; zone->npctable.erase(itr); } while (!zone->merctable.empty()) { itr = zone->merctable.begin(); delete itr->second; + itr->second = nullptr; zone->merctable.erase(itr); } @@ -788,6 +790,7 @@ void Zone::Shutdown(bool quiet) while (!zone->ldon_trap_list.empty()) { itr4 = zone->ldon_trap_list.begin(); delete itr4->second; + itr4->second = nullptr; zone->ldon_trap_list.erase(itr4); } zone->ldon_trap_entry_list.clear(); @@ -1583,6 +1586,7 @@ bool Zone::Depop(bool StartSpawnTimer) { while(!npctable.empty()) { itr = npctable.begin(); delete itr->second; + itr->second = nullptr; npctable.erase(itr); } @@ -1599,6 +1603,7 @@ void Zone::ClearNPCTypeCache(int id) { auto iter = npctable.begin(); while (iter != npctable.end()) { delete iter->second; + iter->second = nullptr; ++iter; } npctable.clear(); @@ -1608,6 +1613,7 @@ void Zone::ClearNPCTypeCache(int id) { while (iter != npctable.end()) { if (iter->first == (uint32)id) { delete iter->second; + iter->second = nullptr; npctable.erase(iter); return; } From 147916ce2e94ea8a04927213c2f25ae33d41675b Mon Sep 17 00:00:00 2001 From: E Spause Date: Sun, 19 Jul 2020 02:34:59 -0400 Subject: [PATCH 3/3] Set group info to null in various places When the group is disbanded, set the leader to null. When setting a new raid leader, make sure we have a new raid leader. If we don't, avoid a crash and disband the raid. It's better than zones falling apart, and will resolve itself on the next VerifyRaid call. If a member zones, set the leader pointer to nullptr. This fixes an issue where the leader pointer is freed later (MemberZoned normally cleans up the Client object), but referenced by other entities, allowing the leader to be used in the same server process tick, post-cleanup - as the leader won't exist. --- zone/groups.cpp | 4 ++++ zone/raids.cpp | 12 ++++++++++++ 2 files changed, 16 insertions(+) diff --git a/zone/groups.cpp b/zone/groups.cpp index 5734d4672..60a58b99c 100644 --- a/zone/groups.cpp +++ b/zone/groups.cpp @@ -498,6 +498,9 @@ void Group::SendEndurancePacketFrom(Mob* member) //if the group was in the zone already bool Group::UpdatePlayer(Mob* update){ + if (!update) + return false; + bool updateSuccess = false; VerifyGroup(); @@ -1009,6 +1012,7 @@ void Group::DisbandGroup(bool joinraid) { Leader->UpdateLFP(); } + SetLeader(nullptr); safe_delete(outapp); } diff --git a/zone/raids.cpp b/zone/raids.cpp index 2dc892a9a..51c52d8d4 100644 --- a/zone/raids.cpp +++ b/zone/raids.cpp @@ -279,6 +279,8 @@ void Raid::SetRaidLeader(const char *wasLead, const char *name) Client *c = entity_list.GetClientByName(name); if(c) SetLeader(c); + else + SetLeader(nullptr); //sanity check, should never get hit but we want to prefer to NOT crash if we do VerifyRaid and leader never gets set there (raid without a leader?) LearnMembers(); VerifyRaid(); @@ -1549,6 +1551,11 @@ void Raid::VerifyRaid() SetLeader(members[x].member); strn0cpy(leadername, members[x].membername, 64); } + else + { + //should never happen, but maybe it is? + SetLeader(nullptr); + } } } } @@ -1558,6 +1565,11 @@ void Raid::MemberZoned(Client *c) if(!c) return; + if (leader == c) + { + leader = nullptr; + } + // Raid::GetGroup() goes over the members as well, this way we go over once uint32 gid = RAID_GROUPLESS; for(int x = 0; x < MAX_RAID_MEMBERS; x++)