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

7 files changed, 65 insertions, 59 deletions

diff --git a/src/server/game/Conditions/ConditionMgr.cpp b/src/server/game/Conditions/ConditionMgr.cpp
index 229cedd590..bb6fa29c49 100644
--- a/src/server/game/Conditions/ConditionMgr.cpp
+++ b/src/server/game/Conditions/ConditionMgr.cpp
@@ -1081,6 +1081,7 @@ 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, "
@@ -1404,7 +1405,7 @@ bool ConditionMgr::addToSpellImplicitTargetConditions(Condition* cond)
 
         // build new shared mask with found effect
         uint32         sharedMask = (1 << i);
-        std::shared_ptr<ConditionList> cmp = spellInfo->Effects[i].ImplicitTargetConditions;
+        ConditionList* cmp        = spellInfo->Effects[i].ImplicitTargetConditions;
         for (uint8 effIndex = i + 1; effIndex < MAX_SPELL_EFFECTS; ++effIndex)
         {
             if (spellInfo->Effects[effIndex].ImplicitTargetConditions == cmp)
@@ -1427,7 +1428,7 @@ bool ConditionMgr::addToSpellImplicitTargetConditions(Condition* cond)
                 return false;
 
             // get shared data
-            std::shared_ptr<ConditionList> sharedList = spellInfo->Effects[firstEffIndex].ImplicitTargetConditions;
+            ConditionList* sharedList = spellInfo->Effects[firstEffIndex].ImplicitTargetConditions;
 
             // there's already data entry for that sharedMask
             if (sharedList)
@@ -1446,25 +1447,22 @@ bool ConditionMgr::addToSpellImplicitTargetConditions(Condition* cond)
             else
             {
                 // add new list, create new shared mask
-                auto newList = std::make_shared<ConditionList>();
-
+                sharedList    = new ConditionList();
                 bool assigned = false;
                 for (uint8 i = firstEffIndex; i < MAX_SPELL_EFFECTS; ++i)
                 {
                     if ((1 << i) & commonMask)
                     {
-                        spellInfo->Effects[i].ImplicitTargetConditions = newList;
-                        assigned = true;
+                        spellInfo->Effects[i].ImplicitTargetConditions = sharedList;
+                        assigned                                       = true;
                     }
                 }
 
-                if (assigned)
-                    sharedList = newList;
+                if (!assigned)
+                    delete sharedList;
             }
-
             if (sharedList)
                 sharedList->push_back(cond);
-
             break;
         }
     }
diff --git a/src/server/game/Spells/Spell.cpp b/src/server/game/Spells/Spell.cpp
index ede719d2a2..e851a039b4 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;
     }
 
-    std::shared_ptr<ConditionList> condList = m_spellInfo->Effects[effIndex].ImplicitTargetConditions;
+    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();
-    std::shared_ptr<ConditionList> condList = m_spellInfo->Effects[effIndex].ImplicitTargetConditions;
+    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, std::shared_ptr<ConditionList> condList)
+uint32 Spell::GetSearcherTypeMask(SpellTargetObjectTypes objType, ConditionList* condList)
 {
     // this function selects which containers need to be searched for spell target
     uint32 retMask = GRID_MAP_TYPE_MASK_ALL;
@@ -2164,9 +2164,7 @@ 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,
-                          std::shared_ptr<ConditionList> condList)
+WorldObject* Spell::SearchNearbyTarget(float range, SpellTargetObjectTypes objectType, SpellTargetCheckTypes selectionType, ConditionList* condList)
 {
     WorldObject* target = nullptr;
     uint32 containerTypeMask = GetSearcherTypeMask(objectType, condList);
@@ -2178,11 +2176,7 @@ 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,
-                              std::shared_ptr<ConditionList> condList)
+void Spell::SearchAreaTargets(std::list<WorldObject*>& targets, float range, Position const* position, Unit* referer, SpellTargetObjectTypes objectType, SpellTargetCheckTypes selectionType, ConditionList* condList)
 {
     uint32 containerTypeMask = GetSearcherTypeMask(objectType, condList);
     if (!containerTypeMask)
@@ -2192,11 +2186,7 @@ void Spell::SearchAreaTargets(std::list<WorldObject*> &targets, float range,
     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*/,
-    std::shared_ptr<ConditionList> condList, bool isChainHeal)
+void Spell::SearchChainTargets(std::list<WorldObject*>& targets, uint32 chainTargets, WorldObject* target, SpellTargetObjectTypes objectType, SpellTargetCheckTypes selectType, SpellTargetSelectionCategories  /*selectCategory*/, ConditionList* condList, bool isChainHeal)
 {
     // max dist for jump target selection
     float jumpRadius = 0.0f;
@@ -2233,8 +2223,7 @@ void Spell::SearchChainTargets(
 
     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
@@ -8996,7 +8985,7 @@ namespace Acore
 {
 
     WorldObjectSpellTargetCheck::WorldObjectSpellTargetCheck(Unit* caster, Unit* referer, SpellInfo const* spellInfo,
-            SpellTargetCheckTypes selectionType, std::shared_ptr<ConditionList> condList) : _caster(caster), _referer(referer), _spellInfo(spellInfo),
+            SpellTargetCheckTypes selectionType, ConditionList* condList) : _caster(caster), _referer(referer), _spellInfo(spellInfo),
         _targetSelectionType(selectionType), _condList(condList)
     {
         if (condList)
@@ -9079,7 +9068,7 @@ namespace Acore
     }
 
     WorldObjectSpellNearbyTargetCheck::WorldObjectSpellNearbyTargetCheck(float range, Unit* caster, SpellInfo const* spellInfo,
-            SpellTargetCheckTypes selectionType, std::shared_ptr<ConditionList> condList)
+            SpellTargetCheckTypes selectionType, ConditionList* condList)
         : WorldObjectSpellTargetCheck(caster, caster, spellInfo, selectionType, condList), _range(range), _position(caster)
     {
     }
@@ -9096,7 +9085,7 @@ namespace Acore
     }
 
     WorldObjectSpellAreaTargetCheck::WorldObjectSpellAreaTargetCheck(float range, Position const* position, Unit* caster,
-            Unit* referer, SpellInfo const* spellInfo, SpellTargetCheckTypes selectionType, std::shared_ptr<ConditionList> condList)
+            Unit* referer, SpellInfo const* spellInfo, SpellTargetCheckTypes selectionType, ConditionList* condList)
         : WorldObjectSpellTargetCheck(caster, referer, spellInfo, selectionType, condList), _range(range), _position(position)
     {
     }
@@ -9116,7 +9105,7 @@ namespace Acore
     }
 
     WorldObjectSpellConeTargetCheck::WorldObjectSpellConeTargetCheck(float coneAngle, float range, Unit* caster,
-            SpellInfo const* spellInfo, SpellTargetCheckTypes selectionType, std::shared_ptr<ConditionList> condList)
+            SpellInfo const* spellInfo, SpellTargetCheckTypes selectionType, ConditionList* condList)
         : WorldObjectSpellAreaTargetCheck(range, caster, caster, caster, spellInfo, selectionType, condList), _coneAngle(coneAngle)
     {
     }
@@ -9142,7 +9131,7 @@ namespace Acore
     }
 
     WorldObjectSpellTrajTargetCheck::WorldObjectSpellTrajTargetCheck(float range, Position const* position, Unit* caster,
-            SpellInfo const* spellInfo, SpellTargetCheckTypes selectionType, std::shared_ptr<ConditionList> condList)
+            SpellInfo const* spellInfo, SpellTargetCheckTypes selectionType, 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 50ba2a4bc6..bb8f2cb7bb 100644
--- a/src/server/game/Spells/Spell.h
+++ b/src/server/game/Spells/Spell.h
@@ -445,25 +445,12 @@ public:
 
     void SelectEffectTypeImplicitTargets(uint8 effIndex);
 
-    uint32 GetSearcherTypeMask(SpellTargetObjectTypes objType,
-                               std::shared_ptr<ConditionList> condList);
+    uint32 GetSearcherTypeMask(SpellTargetObjectTypes objType, 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,
-                       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);
+    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);
 
     SpellCastResult prepare(SpellCastTargets const* targets, AuraEffect const* triggeredByAura = nullptr);
     void cancel(bool bySelf = false);
@@ -814,10 +801,10 @@ namespace Acore
         SpellInfo const* _spellInfo;
         SpellTargetCheckTypes _targetSelectionType;
         ConditionSourceInfo* _condSrcInfo;
-        std::shared_ptr<ConditionList> _condList;
+        ConditionList* _condList;
 
         WorldObjectSpellTargetCheck(Unit* caster, Unit* referer, SpellInfo const* spellInfo,
-                                    SpellTargetCheckTypes selectionType, std::shared_ptr<ConditionList> condList);
+                                    SpellTargetCheckTypes selectionType, ConditionList* condList);
         ~WorldObjectSpellTargetCheck();
         bool operator()(WorldObject* target);
     };
@@ -827,7 +814,7 @@ namespace Acore
         float _range;
         Position const* _position;
         WorldObjectSpellNearbyTargetCheck(float range, Unit* caster, SpellInfo const* spellInfo,
-                                          SpellTargetCheckTypes selectionType, std::shared_ptr<ConditionList> condList);
+                                          SpellTargetCheckTypes selectionType, ConditionList* condList);
         bool operator()(WorldObject* target);
     };
 
@@ -836,7 +823,7 @@ namespace Acore
         float _range;
         Position const* _position;
         WorldObjectSpellAreaTargetCheck(float range, Position const* position, Unit* caster,
-                                        Unit* referer, SpellInfo const* spellInfo, SpellTargetCheckTypes selectionType, std::shared_ptr<ConditionList> condList);
+                                        Unit* referer, SpellInfo const* spellInfo, SpellTargetCheckTypes selectionType, ConditionList* condList);
         bool operator()(WorldObject* target);
     };
 
@@ -844,14 +831,14 @@ namespace Acore
     {
         float _coneAngle;
         WorldObjectSpellConeTargetCheck(float coneAngle, float range, Unit* caster,
-                                        SpellInfo const* spellInfo, SpellTargetCheckTypes selectionType, std::shared_ptr<ConditionList> condList);
+                                        SpellInfo const* spellInfo, SpellTargetCheckTypes selectionType, ConditionList* condList);
         bool operator()(WorldObject* target);
     };
 
     struct WorldObjectSpellTrajTargetCheck : public WorldObjectSpellAreaTargetCheck
     {
         WorldObjectSpellTrajTargetCheck(float range, Position const* position, Unit* caster,
-                                        SpellInfo const* spellInfo, SpellTargetCheckTypes selectionType, std::shared_ptr<ConditionList> condList);
+                                        SpellInfo const* spellInfo, SpellTargetCheckTypes selectionType, ConditionList* condList);
         bool operator()(WorldObject* target);
     };
 }
diff --git a/src/server/game/Spells/SpellInfo.cpp b/src/server/game/Spells/SpellInfo.cpp
index 58c20660b9..64a25a2b4a 100644
--- a/src/server/game/Spells/SpellInfo.cpp
+++ b/src/server/game/Spells/SpellInfo.cpp
@@ -863,6 +863,11 @@ SpellInfo::SpellInfo(SpellEntry const* spellEntry)
     _requireCooldownInfo = false;
 }
 
+SpellInfo::~SpellInfo()
+{
+    _UnloadImplicitTargetConditionLists();
+}
+
 uint32 SpellInfo::GetCategory() const
 {
     return CategoryEntry ? CategoryEntry->Id : 0;
@@ -2868,6 +2873,23 @@ 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 cabdb31587..534ad254a6 100644
--- a/src/server/game/Spells/SpellInfo.h
+++ b/src/server/game/Spells/SpellInfo.h
@@ -39,7 +39,6 @@ struct SpellRadiusEntry;
 struct SpellEntry;
 struct SpellCastTimesEntry;
 struct Condition;
-typedef std::list<Condition*> ConditionList;
 
 enum SpellCastTargetFlags
 {
@@ -271,12 +270,12 @@ public:
     uint32    ItemType;
     uint32    TriggerSpell;
     flag96    SpellClassMask;
-    std::shared_ptr<ConditionList> ImplicitTargetConditions;
+    std::list<Condition*>* 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() {}
+        ItemType(0), TriggerSpell(0), ImplicitTargetConditions(nullptr) {}
     SpellEffectInfo(SpellEntry const* spellEntry, SpellInfo const* spellInfo, uint8 effIndex);
 
     bool IsEffect() const;
@@ -404,6 +403,7 @@ 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 cc770509a9..534388fd4f 100644
--- a/src/server/game/Spells/SpellMgr.cpp
+++ b/src/server/game/Spells/SpellMgr.cpp
@@ -2766,6 +2766,15 @@ 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 ca5ee68577..3a9700b6d8 100644
--- a/src/server/game/Spells/SpellMgr.h
+++ b/src/server/game/Spells/SpellMgr.h
@@ -786,6 +786,7 @@ public:
     void LoadSpellInfoStore();
     void LoadSpellCooldownOverrides();
     void UnloadSpellInfoStore();
+    void UnloadSpellInfoImplicitTargetConditionLists();
     void LoadSpellInfoCustomAttributes();
     void LoadSpellInfoCorrections();
     void LoadSpellSpecificAndAuraState();