fix(Core): Fixed a few crashes/bugs that were found via static code analysis (#2617)

src/server/game/Entities/Player/Player.cpp

@@ -501,7 +501,7 @@ inline void KillRewarder::_InitXP(Player* player)
     // * on battlegrounds;
     // * otherwise, not in PvP;
     // * not if killer is on vehicle.
-    if (_isBattleGround || (!_isPvP && !_killer->GetVehicle()))
+    if (_victim && (_isBattleGround || (!_isPvP && !_killer->GetVehicle())))
         _xp = acore::XP::Gain(player, _victim, _isBattleGround);
 
     if (_xp && !_isBattleGround && _victim) // pussywizard: npcs with relatively low hp give lower exp
@@ -7845,7 +7845,7 @@ void Player::DuelComplete(DuelCompleteType type)
                 duel->opponent->CastSpell(duel->opponent, 52994, true);
 
             // Honor points after duel (the winner) - ImpConfig
-            if (uint32 amount = sWorld->getIntConfig(CONFIG_HONOR_AFTER_DUEL))
+            if (uint32 amount = sWorld->getIntConfig(CONFIG_HONOR_AFTER_DUEL) && duel->opponent)
                 duel->opponent->RewardHonor(NULL, 1, amount);
                 
             break;
@@ -15840,8 +15840,6 @@ void Player::RewardQuest(Quest const* quest, uint32 reward, Object* questGiver,
         {
             if (quest->RequiredItemCount[i] > 0 && itemTemplate->Bonding == BIND_QUEST_ITEM && !quest->IsRepeatable() && !HasQuestForItem(quest->RequiredItemId[i], quest_id, true))
                 DestroyItemCount(quest->RequiredItemId[i], quest->RequiredItemCount[i], true);
-            else
-                DestroyItemCount(quest->RequiredItemId[i], quest->RequiredItemCount[i], true);
         }
     }
     for (uint8 i = 0; i < QUEST_SOURCE_ITEM_IDS_COUNT; ++i)
@@ -15850,8 +15848,6 @@ void Player::RewardQuest(Quest const* quest, uint32 reward, Object* questGiver,
         {
             if (quest->ItemDropQuantity[i] > 0 && itemTemplate->Bonding == BIND_QUEST_ITEM && !quest->IsRepeatable() && !HasQuestForItem(quest->ItemDrop[i], quest_id))
                 DestroyItemCount(quest->ItemDrop[i], quest->ItemDropQuantity[i], true);
-            else
-                DestroyItemCount(quest->ItemDrop[i], quest->ItemDropQuantity[i], true);
         }
     }
 

src/server/game/Groups/Group.cpp

@@ -379,7 +379,7 @@ bool Group::AddMember(Player* player)
 
     SubGroupCounterIncrease(subGroup);
 
-    //if (player)
+    if (player)
     {
         player->SetGroupInvite(NULL);
         if (player->GetGroup())

src/server/game/Handlers/GroupHandler.cpp

@@ -1050,25 +1050,25 @@ void WorldSession::HandleRequestPartyMemberStatsOpcode(WorldPacket &recvData)
 
     data.put<uint64>(maskPos, auraMask);                    // GROUP_UPDATE_FLAG_AURAS
 
-    if (updateFlags & GROUP_UPDATE_FLAG_PET_GUID)
+    if (pet && (updateFlags & GROUP_UPDATE_FLAG_PET_GUID))
         data << uint64(pet->GetGUID());
 
     data << std::string(pet ? pet->GetName() : "");         // GROUP_UPDATE_FLAG_PET_NAME
     data << uint16(pet ? pet->GetDisplayId() : 0);          // GROUP_UPDATE_FLAG_PET_MODEL_ID
 
-    if (updateFlags & GROUP_UPDATE_FLAG_PET_CUR_HP)
+    if (pet && (updateFlags & GROUP_UPDATE_FLAG_PET_CUR_HP))
         data << uint32(pet->GetHealth());
 
-    if (updateFlags & GROUP_UPDATE_FLAG_PET_MAX_HP)
+    if (pet && (updateFlags & GROUP_UPDATE_FLAG_PET_MAX_HP))
         data << uint32(pet->GetMaxHealth());
 
-    if (updateFlags & GROUP_UPDATE_FLAG_PET_POWER_TYPE)
+    if (pet && (updateFlags & GROUP_UPDATE_FLAG_PET_POWER_TYPE))
         data << (uint8)pet->getPowerType();
 
-    if (updateFlags & GROUP_UPDATE_FLAG_PET_CUR_POWER)
+    if (pet && (updateFlags & GROUP_UPDATE_FLAG_PET_CUR_POWER))
         data << uint16(pet->GetPower(pet->getPowerType()));
 
-    if (updateFlags & GROUP_UPDATE_FLAG_PET_MAX_POWER)
+    if (pet && (updateFlags & GROUP_UPDATE_FLAG_PET_MAX_POWER))
         data << uint16(pet->GetMaxPower(pet->getPowerType()));
 
     uint64 petAuraMask = 0;

src/server/game/Handlers/MailHandler.cpp

@@ -710,6 +710,7 @@ void WorldSession::HandleMailCreateTextItem(WorldPacket & recvData)
         if (!mailTemplateEntry)
         {
             player->SendMailResult(mailId, MAIL_MADE_PERMANENT, MAIL_ERR_INTERNAL_ERROR);
+            delete bodyItem;
             return;
         }
 

src/server/game/Server/WorldSession.cpp

@@ -254,7 +254,7 @@ bool WorldSession::Update(uint32 diff, PacketFilter& updater)
         
         /// If necessary, kick the player because the client didn't send anything for too long
         /// (or they've been idling in character select)
-        if (sWorld->getBoolConfig(CONFIG_CLOSE_IDLE_CONNECTIONS) && IsConnectionIdle())
+        if (sWorld->getBoolConfig(CONFIG_CLOSE_IDLE_CONNECTIONS) && IsConnectionIdle() && m_Socket)
             m_Socket->CloseSocket("Client didn't send anything for too long");
     }
 

src/server/game/Spells/Auras/SpellAuraEffects.cpp

@@ -3267,7 +3267,7 @@ void AuraEffect::HandleModStateImmunityMask(AuraApplication const* aurApp, uint8
         {
             if (!GetAmount())
             {
-                mechanic_immunity_list = (1 << MECHANIC_CHARM) | (1 << MECHANIC_SNARE) | (1 << MECHANIC_ROOT)
+                mechanic_immunity_list = (1 << MECHANIC_SNARE) | (1 << MECHANIC_ROOT)
                     | (1 << MECHANIC_FEAR) | (1 << MECHANIC_STUN)
                     | (1 << MECHANIC_SLEEP) | (1 << MECHANIC_CHARM)
                     | (1 << MECHANIC_SAPPED) | (1 << MECHANIC_HORROR)
@@ -6095,7 +6095,7 @@ void AuraEffect::HandlePeriodicDamageAurasTick(Unit* target, Unit* caster) const
     if (GetAuraType() == SPELL_AURA_PERIODIC_DAMAGE)
     {
         // xinef: leave only target depending bonuses, rest is handled in calculate amount
-        if (GetBase()->GetType() == DYNOBJ_AURA_TYPE)
+        if (GetBase()->GetType() == DYNOBJ_AURA_TYPE && caster)
             damage = caster->SpellDamageBonusDone(target, GetSpellInfo(), damage, DOT, 0.0f, GetBase()->GetStackAmount());
         damage = target->SpellDamageBonusTaken(caster, GetSpellInfo(), damage, DOT, GetBase()->GetStackAmount());
 

src/server/game/Spells/Spell.cpp

@@ -2839,7 +2839,7 @@ void Spell::DoAllEffectOnTarget(TargetInfo* target)
         }
 
         // Failed Pickpocket, reveal rogue
-        if (missInfo == SPELL_MISS_RESIST && m_spellInfo->HasAttribute(SPELL_ATTR0_CU_PICKPOCKET) && unitTarget->GetTypeId() == TYPEID_UNIT)
+        if (missInfo == SPELL_MISS_RESIST && m_spellInfo->HasAttribute(SPELL_ATTR0_CU_PICKPOCKET) && unitTarget->GetTypeId() == TYPEID_UNIT && m_caster)
         {
             m_caster->RemoveAurasWithInterruptFlags(AURA_INTERRUPT_FLAG_TALK);
             if (unitTarget->ToCreature()->IsAIEnabled)

src/server/game/Spells/SpellEffects.cpp

@@ -3141,8 +3141,9 @@ void Spell::EffectSummonPet(SpellEffIndex effIndex)
 
     uint32 petentry = m_spellInfo->Effects[effIndex].MiscValue;
     int32 duration = m_spellInfo->GetDuration();
-    if(Player* modOwner = m_originalCaster->GetSpellModOwner())
+    if (m_originalCaster)
-        modOwner->ApplySpellMod(m_spellInfo->Id, SPELLMOD_DURATION, duration);
+        if(Player* modOwner = m_originalCaster->GetSpellModOwner())
+            modOwner->ApplySpellMod(m_spellInfo->Id, SPELLMOD_DURATION, duration);
 
     if (!owner)
     {
@@ -4644,21 +4645,20 @@ void Spell::EffectSummonObject(SpellEffIndex effIndex)
         default: return;
     }
 
-    uint64 guid = m_caster->m_ObjectSlot[slot];
+    if (m_caster)
-    if (guid != 0)
     {
-        GameObject* gameObject = NULL;
+        uint64 guid = m_caster->m_ObjectSlot[slot];
-        if (m_caster)
+        if (guid != 0)
-            gameObject = m_caster->GetMap()->GetGameObject(guid);
-
-        if (gameObject)
         {
-            // Recast case - null spell id to make auras not be removed on object remove from world
+            if (GameObject* gameObject = m_caster->GetMap()->GetGameObject(guid))
-            if (m_spellInfo->Id == gameObject->GetSpellId())
+            {
-                gameObject->SetSpellId(0);
+                // Recast case - null spell id to make auras not be removed on object remove from world
-            m_caster->RemoveGameObject(gameObject, true);
+                if (m_spellInfo->Id == gameObject->GetSpellId())
+                    gameObject->SetSpellId(0);
+                m_caster->RemoveGameObject(gameObject, true);
+            }
+            m_caster->m_ObjectSlot[slot] = 0;
         }
-        m_caster->m_ObjectSlot[slot] = 0;
     }
 
     GameObject* pGameObj = sObjectMgr->IsGameObjectStaticTransport(gameobjectId) ? new StaticTransport() : new GameObject();

src/server/scripts/Commands/cs_npc.cpp

@@ -603,11 +603,12 @@ public:
     static bool HandleNpcSetOriginalFaction(ChatHandler* handler, const char* /*args*/)
     {
         Player* me = handler->GetSession()->GetPlayer();
-        Creature* creature = me->GetSelectedUnit()->ToCreature();
 
         if (!me)
             return false;
 
+        Creature* creature = me->GetSelectedUnit()->ToCreature();
+
         if (!creature)
             return false;
 

src/server/scripts/Commands/cs_wp.cpp

@@ -371,6 +371,12 @@ public:
 
         if (show == "del")
         {
+            if (!arg_id)
+            {
+                handler->SendSysMessage("|cffff33ffERROR: Waypoint script guid not present.|r");
+                return true;
+            }
+
             id = atoi(arg_id);
 
             PreparedStatement* stmt = WorldDatabase.GetPreparedStatement(WORLD_SEL_WAYPOINT_SCRIPT_ID_BY_GUID);

src/server/scripts/Northrend/CrusadersColiseum/TrialOfTheCrusader/boss_anubarak_trial.cpp

@@ -201,13 +201,14 @@ public:
                 events.RescheduleEvent(EVENT_RESPAWN_SPHERE, 4000);
 
             for( std::list<uint64>::iterator itr = summons.begin(); itr != summons.end(); ++itr )
-                if( Creature* c = pInstance->instance->GetCreature(*itr) )
+                if (pInstance)
-                {
+                    if(Creature* c = pInstance->instance->GetCreature(*itr) )
-                    c->GetMotionMaster()->MoveIdle();
+                    {
-                    c->StopMoving();
+                        c->GetMotionMaster()->MoveIdle();
-                    c->CastSpell(c, SPELL_SUBMERGE, false);
+                        c->StopMoving();
-                    c->AI()->DoAction(1);
+                        c->CastSpell(c, SPELL_SUBMERGE, false);
-                }
+                        c->AI()->DoAction(1);
+                    }
             summons.clear();
             for( uint8 i=0; i<4; ++i )
                 if( Creature* c = me->SummonCreature(NPC_BURROW, AnubLocs[i+1]) )

src/server/scripts/Northrend/Naxxramas/boss_kelthuzad.cpp

@@ -153,14 +153,12 @@ public:
                 {
                     float dist = j == 2 ? 0.0f : 8.0f; // second in middle
                     float angle = SummonPositions[i].GetOrientation() + M_PI*2/4*j;
-                    NormalizeOrientation(angle);
                     me->SummonCreature(NPC_UNSTOPPABLE_ABOMINATION, SummonPositions[i].GetPositionX()+dist*cos(angle), SummonPositions[i].GetPositionY()+dist*sin(angle), SummonPositions[i].GetPositionZ()+0.5f, SummonPositions[i].GetOrientation(), TEMPSUMMON_CORPSE_TIMED_DESPAWN, 20000);
                 }
             for (uint8 i = 6; i < 12; ++i)
                 for (uint8 j = 0; j < 1; ++j)
                 {
                     float angle = SummonPositions[i].GetOrientation() + M_PI;
-                    NormalizeOrientation(angle);
                     me->SummonCreature(NPC_SOUL_WEAVER, SummonPositions[i].GetPositionX()+6*cos(angle), SummonPositions[i].GetPositionY()+6*sin(angle), SummonPositions[i].GetPositionZ()+0.5f, SummonPositions[i].GetOrientation(), TEMPSUMMON_CORPSE_TIMED_DESPAWN, 20000);
                 }
         }

src/server/scripts/Northrend/Nexus/EyeOfEternity/boss_malygos.cpp

@@ -876,7 +876,8 @@ public:
                 despawnTimer = 0;
                 me->UpdatePosition(CenterPos.GetPositionX(), CenterPos.GetPositionY(), CenterPos.GetPositionZ()+18.0f, 0.0f, true);
                 me->StopMovingOnCurrentPos();
-                me->GetVehicleKit()->RemoveAllPassengers();
+                if (Vehicle* vehicle = me->GetVehicleKit())
+                    vehicle->RemoveAllPassengers();
                 me->DespawnOrUnsummon();
                 return;
             }

src/server/scripts/Northrend/Nexus/Oculus/oculus.cpp

@@ -778,9 +778,12 @@ class spell_oculus_rider_aura : public SpellScriptLoader
             void HandleOnEffectApply(AuraEffect const* aurEff, AuraEffectHandleModes /*mode*/)
             {
                 Unit* caster = GetCaster();
+                if (!caster)
+                    return;
+
                 Creature* drake = caster->GetVehicleCreatureBase();
 
-                if (!drake || !caster)
+                if (!drake)
                     return;
 
                 switch (aurEff->GetEffIndex())

src/server/scripts/Northrend/Ulduar/Ulduar/boss_mimiron.cpp

@@ -395,11 +395,12 @@ public:
             }
 
             // ensure LMK2 is at proper position
-            if (Creature* LMK2 = GetLMK2())
+            if (pInstance)
-            {
+                if (Creature* LMK2 = GetLMK2())
-                LMK2->UpdatePosition(LMK2->GetHomePosition(), true);
+                {
-                LMK2->StopMovingOnCurrentPos();
+                    LMK2->UpdatePosition(LMK2->GetHomePosition(), true);
-            }
+                    LMK2->StopMovingOnCurrentPos();
+                }
 
             if (pInstance && pInstance->GetData(TYPE_MIMIRON) != DONE)
                 pInstance->SetData(TYPE_MIMIRON, IN_PROGRESS);

src/server/scripts/Northrend/Ulduar/Ulduar/boss_yoggsaron.cpp

@@ -2312,8 +2312,14 @@ class spell_yogg_saron_brain_link : public SpellScriptLoader
             void OnPeriodic(AuraEffect const*  /*aurEff*/)
             {
                 Unit* owner = GetUnitOwner();
+                if (!owner)
+                {
+                    SetDuration(0);
+                    return;
+                }
+
                 Unit* _target = ObjectAccessor::GetUnit(*owner, _targetGUID);
-                if (!owner || !_target || !_target->IsAlive() || fabs(owner->GetPositionZ() - _target->GetPositionZ()) > 10.0f) // Target or owner underground
+                if (!_target || !_target->IsAlive() || fabs(owner->GetPositionZ() - _target->GetPositionZ()) > 10.0f) // Target or owner underground
                 {
                     SetDuration(0);
                     return;

src/server/scripts/Northrend/UtgardeKeep/UtgardePinnacle/boss_palehoof.cpp

@@ -749,7 +749,7 @@ public:
             {
                 case EVENT_WORGEN_MORTAL:
                 {
-                    me->CastSpell(me->GetVictim(), IsHeroic() ? SPELL_MORTAL_WOUND_H : SPELL_MORTAL_WOUND_H, false);
+                    me->CastSpell(me->GetVictim(), IsHeroic() ? SPELL_MORTAL_WOUND_H : SPELL_MORTAL_WOUND_N, false);
                     events.RepeatEvent(4000 + rand()%3000);
                     break;
                 }

src/server/scripts/Northrend/UtgardeKeep/UtgardePinnacle/boss_svala.cpp

@@ -139,7 +139,7 @@ public:
 
         void SetData(uint32 data, uint32 param)
         {
-            if (data != 1 || param != 1 || Started || instance->GetData(DATA_SVALA_SORROWGRAVE) == DONE)
+            if (data != 1 || param != 1 || Started || (instance && instance->GetData(DATA_SVALA_SORROWGRAVE) == DONE))
                 return;
 
             me->SetFlag(UNIT_FIELD_FLAGS, UNIT_FLAG_IMMUNE_TO_PC|UNIT_FLAG_IMMUNE_TO_NPC);

src/server/scripts/Northrend/VioletHold/violet_hold.cpp

@@ -220,16 +220,20 @@ public:
 
         void JustSummoned(Creature *pSummoned)
         {
-            listOfMobs.Summon(pSummoned);
             if (pSummoned)
+            {
+                listOfMobs.Summon(pSummoned);
                 pInstance->SetData64(DATA_ADD_TRASH_MOB, pSummoned->GetGUID());
+            }
         }
 
         void SummonedMobDied(Creature *pSummoned)
         {
-            listOfMobs.Despawn(pSummoned);
             if (pSummoned)
+            {
+                listOfMobs.Despawn(pSummoned);
                 pInstance->SetData64(DATA_DELETE_TRASH_MOB, pSummoned->GetGUID());
+            }
         }
     };
 };

src/server/scripts/Outland/BlackTemple/boss_reliquary_of_souls.cpp

@@ -551,11 +551,14 @@ class boss_essence_of_anger : public CreatureScript
                         events.ScheduleEvent(EVENT_ANGER_SOUL_SCREAM, 10000);
                         break;
                     case EVENT_ANGER_SEETHE:
-                        if (targetGUID && targetGUID != me->GetVictim()->GetGUID())
+                        if (Unit* victim = me->GetVictim())
-                            me->CastSpell(me, SPELL_SEETHE, false);
+                        {
-                        // victim can be lost
+                            uint64 victimGUID = victim->GetGUID();
-                        if (me->GetVictim())
+                            if (targetGUID && targetGUID != victimGUID)
-                            targetGUID = me->GetVictim()->GetGUID();
+                                me->CastSpell(me, SPELL_SEETHE, false);
+                            // victim can be lost
+                            targetGUID = victimGUID;
+                        }     
                         events.ScheduleEvent(EVENT_ANGER_SEETHE, 1000);
                         break;
 

src/server/scripts/Spells/spell_shaman.cpp

@@ -293,7 +293,7 @@ class spell_sha_feral_spirit_scaling : public SpellScriptLoader
 
             void Register()
             {
-                if (m_scriptSpellId == 35675 || m_scriptSpellId == 35675)
+                if (m_scriptSpellId == 35675)
                     DoEffectCalcAmount += AuraEffectCalcAmountFn(spell_sha_feral_spirit_scaling_AuraScript::CalculateResistanceAmount, EFFECT_ALL, SPELL_AURA_MOD_RESISTANCE);
 
                 if (m_scriptSpellId == 35674)