fix(Core/Conditions): `ConditionList` use after free (#23006)

7 files changed, 59 insertions, 65 deletions

diff --git a/src/server/game/Conditions/ConditionMgr.cpp b/src/server/game/Conditions/ConditionMgr.cpp
index bb6fa29c49..229cedd590 100644
--- a/src/server/game/Conditions/ConditionMgr.cpp
+++ b/src/server/game/Conditions/ConditionMgr.cpp
@@ -1081,7 +1081,6 @@ void ConditionMgr::LoadConditions(bool isReload)
 
         LOG_INFO("server.loading", "Reloading `gossip_menu_option` Table for Conditions!");
         sObjectMgr->LoadGossipMenuItems();
-        sSpellMgr->UnloadSpellInfoImplicitTargetConditionLists();
     }
 
     QueryResult result = WorldDatabase.Query("SELECT SourceTypeOrReferenceId, SourceGroup, SourceEntry, SourceId, ElseGroup, ConditionTypeOrReference, ConditionTarget, "
@@ -1405,7 +1404,7 @@ bool ConditionMgr::addToSpellImplicitTargetConditions(Condition* cond)
 
         // build new shared mask with found effect
         uint32         sharedMask = (1 << i);
-        ConditionList* cmp        = spellInfo->Effects[i].ImplicitTargetConditions;
+        std::shared_ptr<ConditionList> cmp = spellInfo->Effects[i].ImplicitTargetConditions;
         for (uint8 effIndex = i + 1; effIndex < MAX_SPELL_EFFECTS; ++effIndex)
         {
             if (spellInfo->Effects[effIndex].ImplicitTargetConditions == cmp)
@@ -1428,7 +1427,7 @@ bool ConditionMgr::addToSpellImplicitTargetConditions(Condition* cond)
                 return false;
 
             // get shared data
-            ConditionList* sharedList = spellInfo->Effects[firstEffIndex].ImplicitTargetConditions;
+            std::shared_ptr<ConditionList> sharedList = spellInfo->Effects[firstEffIndex].ImplicitTargetConditions;
 
             // there's already data entry for that sharedMask
             if (sharedList)
@@ -1447,22 +1446,25 @@ bool ConditionMgr::addToSpellImplicitTargetConditions(Condition* cond)
             else
             {
                 // add new list, create new shared mask
-                sharedList    = new ConditionList();
+                auto newList = std::make_shared<ConditionList>();
+
                 bool assigned = false;
                 for (uint8 i = firstEffIndex; i < MAX_SPELL_EFFECTS; ++i)
                 {
                     if ((1 << i) & commonMask)
                     {
-                        spellInfo->Effects[i].ImplicitTargetConditions = sharedList;
-                        assigned                                       = true;
+                        spellInfo->Effects[i].ImplicitTargetConditions = newList;
+                        assigned = true;
                     }
                 }
 
-                if (!assigned)
-                    delete sharedList;
+                if (assigned)
+                    sharedList = newList;
             }
+
             if (sharedList)
                 sharedList->push_back(cond);
+
             break;
         }
     }
diff --git a/src/server/game/Spells/Spell.cpp b/src/server/game/Spells/Spell.cpp
index daddbe9dde..5b2aeb1101 100644
--- a/src/server/game/Spells/Spell.cpp
+++ b/src/server/game/Spells/Spell.cpp
@@ -1126,7 +1126,7 @@ void Spell::SelectImplicitNearbyTargets(SpellEffIndex effIndex, SpellImplicitTar
             break;
     }
 
-    ConditionList* condList = m_spellInfo->Effects[effIndex].ImplicitTargetConditions;
+    std::shared_ptr<ConditionList> condList = m_spellInfo->Effects[effIndex].ImplicitTargetConditions;
 
     // handle emergency case - try to use other provided targets if no conditions provided
     if (targetType.GetCheckType() == TARGET_CHECK_ENTRY && (!condList || condList->empty()))
@@ -1221,7 +1221,7 @@ void Spell::SelectImplicitConeTargets(SpellEffIndex effIndex, SpellImplicitTarge
     std::list<WorldObject*> targets;
     SpellTargetObjectTypes objectType = targetType.GetObjectType();
     SpellTargetCheckTypes selectionType = targetType.GetCheckType();
-    ConditionList* condList = m_spellInfo->Effects[effIndex].ImplicitTargetConditions;
+    std::shared_ptr<ConditionList> condList = m_spellInfo->Effects[effIndex].ImplicitTargetConditions;
     float coneAngle = M_PI / 2;
     float radius = m_spellInfo->Effects[effIndex].CalcRadius(m_caster) * m_spellValue->RadiusMod;
 
@@ -2119,7 +2119,7 @@ void Spell::SelectEffectTypeImplicitTargets(uint8 effIndex)
     }
 }
 
-uint32 Spell::GetSearcherTypeMask(SpellTargetObjectTypes objType, ConditionList* condList)
+uint32 Spell::GetSearcherTypeMask(SpellTargetObjectTypes objType, std::shared_ptr<ConditionList> condList)
 {
     // this function selects which containers need to be searched for spell target
     uint32 retMask = GRID_MAP_TYPE_MASK_ALL;
@@ -2164,7 +2164,9 @@ void Spell::SearchTargets(SEARCHER& searcher, uint32 containerMask, Unit* refere
     Cell::VisitObjects(pos->GetPositionX(), pos->GetPositionY(), referer->GetMap(), searcher, radius);
 }
 
-WorldObject* Spell::SearchNearbyTarget(float range, SpellTargetObjectTypes objectType, SpellTargetCheckTypes selectionType, ConditionList* condList)
+WorldObject* Spell::SearchNearbyTarget(float range, SpellTargetObjectTypes objectType,
+                          SpellTargetCheckTypes selectionType,
+                          std::shared_ptr<ConditionList> condList)
 {
     WorldObject* target = nullptr;
     uint32 containerTypeMask = GetSearcherTypeMask(objectType, condList);
@@ -2176,7 +2178,11 @@ WorldObject* Spell::SearchNearbyTarget(float range, SpellTargetObjectTypes objec
     return target;
 }
 
-void Spell::SearchAreaTargets(std::list<WorldObject*>& targets, float range, Position const* position, Unit* referer, SpellTargetObjectTypes objectType, SpellTargetCheckTypes selectionType, ConditionList* condList)
+void Spell::SearchAreaTargets(std::list<WorldObject*> &targets, float range,
+                              Position const *position, Unit *referer,
+                              SpellTargetObjectTypes objectType,
+                              SpellTargetCheckTypes selectionType,
+                              std::shared_ptr<ConditionList> condList)
 {
     uint32 containerTypeMask = GetSearcherTypeMask(objectType, condList);
     if (!containerTypeMask)
@@ -2186,7 +2192,11 @@ void Spell::SearchAreaTargets(std::list<WorldObject*>& targets, float range, Pos
     SearchTargets<Acore::WorldObjectListSearcher<Acore::WorldObjectSpellAreaTargetCheck> > (searcher, containerTypeMask, m_caster, position, range);
 }
 
-void Spell::SearchChainTargets(std::list<WorldObject*>& targets, uint32 chainTargets, WorldObject* target, SpellTargetObjectTypes objectType, SpellTargetCheckTypes selectType, SpellTargetSelectionCategories  /*selectCategory*/, ConditionList* condList, bool isChainHeal)
+void Spell::SearchChainTargets(
+    std::list<WorldObject*> &targets, uint32 chainTargets, WorldObject *target,
+    SpellTargetObjectTypes objectType, SpellTargetCheckTypes selectType,
+    SpellTargetSelectionCategories /*selectCategory*/,
+    std::shared_ptr<ConditionList> condList, bool isChainHeal)
 {
     // max dist for jump target selection
     float jumpRadius = 0.0f;
@@ -2223,7 +2233,8 @@ void Spell::SearchChainTargets(std::list<WorldObject*>& targets, uint32 chainTar
 
     WorldObject* chainSource = m_spellInfo->HasAttribute(SPELL_ATTR2_CHAIN_FROM_CASTER) ? m_caster : target;
     std::list<WorldObject*> tempTargets;
-    SearchAreaTargets(tempTargets, searchRadius, chainSource, m_caster, objectType, selectType, condList);
+    SearchAreaTargets(tempTargets, searchRadius, chainSource, m_caster,
+                      objectType, selectType, condList);
     tempTargets.remove(target);
 
     // remove targets which are always invalid for chain spells
@@ -8984,7 +8995,7 @@ namespace Acore
 {
 
     WorldObjectSpellTargetCheck::WorldObjectSpellTargetCheck(Unit* caster, Unit* referer, SpellInfo const* spellInfo,
-            SpellTargetCheckTypes selectionType, ConditionList* condList) : _caster(caster), _referer(referer), _spellInfo(spellInfo),
+            SpellTargetCheckTypes selectionType, std::shared_ptr<ConditionList> condList) : _caster(caster), _referer(referer), _spellInfo(spellInfo),
         _targetSelectionType(selectionType), _condList(condList)
     {
         if (condList)
@@ -9067,7 +9078,7 @@ namespace Acore
     }
 
     WorldObjectSpellNearbyTargetCheck::WorldObjectSpellNearbyTargetCheck(float range, Unit* caster, SpellInfo const* spellInfo,
-            SpellTargetCheckTypes selectionType, ConditionList* condList)
+            SpellTargetCheckTypes selectionType, std::shared_ptr<ConditionList> condList)
         : WorldObjectSpellTargetCheck(caster, caster, spellInfo, selectionType, condList), _range(range), _position(caster)
     {
     }
@@ -9084,7 +9095,7 @@ namespace Acore
     }
 
     WorldObjectSpellAreaTargetCheck::WorldObjectSpellAreaTargetCheck(float range, Position const* position, Unit* caster,
-            Unit* referer, SpellInfo const* spellInfo, SpellTargetCheckTypes selectionType, ConditionList* condList)
+            Unit* referer, SpellInfo const* spellInfo, SpellTargetCheckTypes selectionType, std::shared_ptr<ConditionList> condList)
         : WorldObjectSpellTargetCheck(caster, referer, spellInfo, selectionType, condList), _range(range), _position(position)
     {
     }
@@ -9104,7 +9115,7 @@ namespace Acore
     }
 
     WorldObjectSpellConeTargetCheck::WorldObjectSpellConeTargetCheck(float coneAngle, float range, Unit* caster,
-            SpellInfo const* spellInfo, SpellTargetCheckTypes selectionType, ConditionList* condList)
+            SpellInfo const* spellInfo, SpellTargetCheckTypes selectionType, std::shared_ptr<ConditionList> condList)
         : WorldObjectSpellAreaTargetCheck(range, caster, caster, caster, spellInfo, selectionType, condList), _coneAngle(coneAngle)
     {
     }
@@ -9130,7 +9141,7 @@ namespace Acore
     }
 
     WorldObjectSpellTrajTargetCheck::WorldObjectSpellTrajTargetCheck(float range, Position const* position, Unit* caster,
-            SpellInfo const* spellInfo, SpellTargetCheckTypes selectionType, ConditionList* condList)
+            SpellInfo const* spellInfo, SpellTargetCheckTypes selectionType, std::shared_ptr<ConditionList> condList)
         : WorldObjectSpellAreaTargetCheck(range, position, caster, caster, spellInfo, selectionType, condList)
     {
     }
diff --git a/src/server/game/Spells/Spell.h b/src/server/game/Spells/Spell.h
index bb8f2cb7bb..50ba2a4bc6 100644
--- a/src/server/game/Spells/Spell.h
+++ b/src/server/game/Spells/Spell.h
@@ -445,12 +445,25 @@ public:
 
     void SelectEffectTypeImplicitTargets(uint8 effIndex);
 
-    uint32 GetSearcherTypeMask(SpellTargetObjectTypes objType, ConditionList* condList);
+    uint32 GetSearcherTypeMask(SpellTargetObjectTypes objType,
+                               std::shared_ptr<ConditionList> condList);
     template<class SEARCHER> void SearchTargets(SEARCHER& searcher, uint32 containerMask, Unit* referer, Position const* pos, float radius);
 
-    WorldObject* SearchNearbyTarget(float range, SpellTargetObjectTypes objectType, SpellTargetCheckTypes selectionType, ConditionList* condList = nullptr);
-    void SearchAreaTargets(std::list<WorldObject*>& targets, float range, Position const* position, Unit* referer, SpellTargetObjectTypes objectType, SpellTargetCheckTypes selectionType, ConditionList* condList);
-    void SearchChainTargets(std::list<WorldObject*>& targets, uint32 chainTargets, WorldObject* target, SpellTargetObjectTypes objectType, SpellTargetCheckTypes selectType, SpellTargetSelectionCategories selectCategory, ConditionList* condList, bool isChainHeal);
+    WorldObject* SearchNearbyTarget(float range, SpellTargetObjectTypes objectType,
+                       SpellTargetCheckTypes selectionType,
+                       std::shared_ptr<ConditionList> condList = nullptr);
+    void SearchAreaTargets(std::list<WorldObject *> &targets, float range,
+                           Position const *position, Unit *referer,
+                           SpellTargetObjectTypes objectType,
+                           SpellTargetCheckTypes selectionType,
+                           std::shared_ptr<ConditionList> condList);
+    void SearchChainTargets(std::list<WorldObject *> &targets,
+                            uint32 chainTargets, WorldObject *target,
+                            SpellTargetObjectTypes objectType,
+                            SpellTargetCheckTypes selectType,
+                            SpellTargetSelectionCategories selectCategory,
+                            std::shared_ptr<ConditionList> condList,
+                            bool isChainHeal);
 
     SpellCastResult prepare(SpellCastTargets const* targets, AuraEffect const* triggeredByAura = nullptr);
     void cancel(bool bySelf = false);
@@ -801,10 +814,10 @@ namespace Acore
         SpellInfo const* _spellInfo;
         SpellTargetCheckTypes _targetSelectionType;
         ConditionSourceInfo* _condSrcInfo;
-        ConditionList* _condList;
+        std::shared_ptr<ConditionList> _condList;
 
         WorldObjectSpellTargetCheck(Unit* caster, Unit* referer, SpellInfo const* spellInfo,
-                                    SpellTargetCheckTypes selectionType, ConditionList* condList);
+                                    SpellTargetCheckTypes selectionType, std::shared_ptr<ConditionList> condList);
         ~WorldObjectSpellTargetCheck();
         bool operator()(WorldObject* target);
     };
@@ -814,7 +827,7 @@ namespace Acore
         float _range;
         Position const* _position;
         WorldObjectSpellNearbyTargetCheck(float range, Unit* caster, SpellInfo const* spellInfo,
-                                          SpellTargetCheckTypes selectionType, ConditionList* condList);
+                                          SpellTargetCheckTypes selectionType, std::shared_ptr<ConditionList> condList);
         bool operator()(WorldObject* target);
     };
 
@@ -823,7 +836,7 @@ namespace Acore
         float _range;
         Position const* _position;
         WorldObjectSpellAreaTargetCheck(float range, Position const* position, Unit* caster,
-                                        Unit* referer, SpellInfo const* spellInfo, SpellTargetCheckTypes selectionType, ConditionList* condList);
+                                        Unit* referer, SpellInfo const* spellInfo, SpellTargetCheckTypes selectionType, std::shared_ptr<ConditionList> condList);
         bool operator()(WorldObject* target);
     };
 
@@ -831,14 +844,14 @@ namespace Acore
     {
         float _coneAngle;
         WorldObjectSpellConeTargetCheck(float coneAngle, float range, Unit* caster,
-                                        SpellInfo const* spellInfo, SpellTargetCheckTypes selectionType, ConditionList* condList);
+                                        SpellInfo const* spellInfo, SpellTargetCheckTypes selectionType, std::shared_ptr<ConditionList> condList);
         bool operator()(WorldObject* target);
     };
 
     struct WorldObjectSpellTrajTargetCheck : public WorldObjectSpellAreaTargetCheck
     {
         WorldObjectSpellTrajTargetCheck(float range, Position const* position, Unit* caster,
-                                        SpellInfo const* spellInfo, SpellTargetCheckTypes selectionType, ConditionList* condList);
+                                        SpellInfo const* spellInfo, SpellTargetCheckTypes selectionType, std::shared_ptr<ConditionList> condList);
         bool operator()(WorldObject* target);
     };
 }
diff --git a/src/server/game/Spells/SpellInfo.cpp b/src/server/game/Spells/SpellInfo.cpp
index 64a25a2b4a..58c20660b9 100644
--- a/src/server/game/Spells/SpellInfo.cpp
+++ b/src/server/game/Spells/SpellInfo.cpp
@@ -863,11 +863,6 @@ SpellInfo::SpellInfo(SpellEntry const* spellEntry)
     _requireCooldownInfo = false;
 }
 
-SpellInfo::~SpellInfo()
-{
-    _UnloadImplicitTargetConditionLists();
-}
-
 uint32 SpellInfo::GetCategory() const
 {
     return CategoryEntry ? CategoryEntry->Id : 0;
@@ -2873,23 +2868,6 @@ bool SpellInfo::_IsPositiveTarget(uint32 targetA, uint32 targetB)
     return true;
 }
 
-void SpellInfo::_UnloadImplicitTargetConditionLists()
-{
-    // find the same instances of ConditionList and delete them.
-    for (uint8 i = 0; i < MAX_SPELL_EFFECTS; ++i)
-    {
-        ConditionList* cur = Effects[i].ImplicitTargetConditions;
-        if (!cur)
-            continue;
-        for (uint8 j = i; j < MAX_SPELL_EFFECTS; ++j)
-        {
-            if (Effects[j].ImplicitTargetConditions == cur)
-                Effects[j].ImplicitTargetConditions = nullptr;
-        }
-        delete cur;
-    }
-}
-
 bool SpellInfo::CheckElixirStacking(Unit const* caster) const
 {
     if (!caster)
diff --git a/src/server/game/Spells/SpellInfo.h b/src/server/game/Spells/SpellInfo.h
index 534ad254a6..cabdb31587 100644
--- a/src/server/game/Spells/SpellInfo.h
+++ b/src/server/game/Spells/SpellInfo.h
@@ -39,6 +39,7 @@ struct SpellRadiusEntry;
 struct SpellEntry;
 struct SpellCastTimesEntry;
 struct Condition;
+typedef std::list<Condition*> ConditionList;
 
 enum SpellCastTargetFlags
 {
@@ -270,12 +271,12 @@ public:
     uint32    ItemType;
     uint32    TriggerSpell;
     flag96    SpellClassMask;
-    std::list<Condition*>* ImplicitTargetConditions;
+    std::shared_ptr<ConditionList> ImplicitTargetConditions;
 
     SpellEffectInfo() : _spellInfo(nullptr), _effIndex(0), Effect(0), ApplyAuraName(0), Amplitude(0), DieSides(0),
         RealPointsPerLevel(0), BasePoints(0), PointsPerComboPoint(0), ValueMultiplier(0), DamageMultiplier(0),
         BonusMultiplier(0), MiscValue(0), MiscValueB(0), Mechanic(MECHANIC_NONE), RadiusEntry(nullptr), ChainTarget(0),
-        ItemType(0), TriggerSpell(0), ImplicitTargetConditions(nullptr) {}
+        ItemType(0), TriggerSpell(0), ImplicitTargetConditions() {}
     SpellEffectInfo(SpellEntry const* spellEntry, SpellInfo const* spellInfo, uint8 effIndex);
 
     bool IsEffect() const;
@@ -403,7 +404,6 @@ public:
     bool _requireCooldownInfo;
 
     SpellInfo(SpellEntry const* spellEntry);
-    ~SpellInfo();
 
     uint32 GetCategory() const;
     bool HasEffect(SpellEffects effect) const;
diff --git a/src/server/game/Spells/SpellMgr.cpp b/src/server/game/Spells/SpellMgr.cpp
index 534388fd4f..cc770509a9 100644
--- a/src/server/game/Spells/SpellMgr.cpp
+++ b/src/server/game/Spells/SpellMgr.cpp
@@ -2766,15 +2766,6 @@ void SpellMgr::UnloadSpellInfoStore()
     mSpellInfoMap.clear();
 }
 
-void SpellMgr::UnloadSpellInfoImplicitTargetConditionLists()
-{
-    for (uint32 i = 0; i < GetSpellInfoStoreSize(); ++i)
-    {
-        if (mSpellInfoMap[i])
-            mSpellInfoMap[i]->_UnloadImplicitTargetConditionLists();
-    }
-}
-
 void SpellMgr::LoadSpellSpecificAndAuraState()
 {
     uint32 oldMSTime = getMSTime();
diff --git a/src/server/game/Spells/SpellMgr.h b/src/server/game/Spells/SpellMgr.h
index 3a9700b6d8..ca5ee68577 100644
--- a/src/server/game/Spells/SpellMgr.h
+++ b/src/server/game/Spells/SpellMgr.h
@@ -786,7 +786,6 @@ public:
     void LoadSpellInfoStore();
     void LoadSpellCooldownOverrides();
     void UnloadSpellInfoStore();
-    void UnloadSpellInfoImplicitTargetConditionLists();
     void LoadSpellInfoCustomAttributes();
     void LoadSpellInfoCorrections();
     void LoadSpellSpecificAndAuraState();