Add additional NULL checks all around the code before dereferencing GetVictim() result.
Keep in mind UpdateVictim() result has nothing to do with GetVictim() result and the caller shouldn't assume anything about GetVictim() based on UpdateVictim().
Add a new method EnsureVictim() which asserts a valid not NULL reference will be returned to the caller. Use this whenever the GetVictim() would be dereferenced .
Increase the max walkable angle and max walkable climb in MMAPs generator to allow NPCs to jump on fences and climb steps.
This requires MMAPs to be extracted again with mmaps_generator, old extracted MMAPs are still compatible but will not provide the benefits of this commit.
Fixes#9965
Allow ".modify speed" command to target current Player if there is no target or if the selected Unit is a Creature.
This behavior could be extended to other commands that require target Players.
Fix a crash happening with GAMEOBJECT_TYPE_SUMMONING_RITUAL GameObjects if a Player clicked on the GO, logged out and let another Player click on it.
Issue added in 464c0c1611Fixes#11856
Add Line of Sight checks to WorldObject::GetNearPoint(), fixing to TargetedMovementGenerator not reaching a point with valid LoS to the target and MotionMaster::MoveKnockbackFrom() knocking creatures inside walls.
Remove unused code.
Handle BasicType btChar and custom type std::basic_string<char,std::char_traits<char>,std::allocator<char> > (std::string).
This allows WheatyExceptionReport to log the text stored in char*, char[] and std::string.
Fix a case where valid chat links would have been handled as invalid.
This happened if these conditions were true:
- the link was a profession and the link name started with the same name of the profession, like spell id 61120 in esES locale "Inscripción de la tormenta de maestro" with profession name "Inscripción"
- the profession name was the same for more than 1 locale available, like esES and esMX, or enUS and enGB
Restore memmove() instead of memcpy() wrongly replaced in 791130f6fa , it's unsafe to use memcpy() with overlapping memory regions.
Fix Position members not being always initialized.
Valgrind log:
at : Position::NormalizeOrientation(float) (Object.h:388)
by : Position::SetOrientation(float) (Object.h:315)
by : Position::Relocate(Position const&) (Object.h:310)
by : spell_ulduar_stone_grip::spell_ulduar_stone_grip_AuraScript::OnRemoveVehicle(AuraEffect const*, AuraEffectHandleModes) (boss_kologarn.cpp:592)
Move "Battleground::GetBGCreature: creature not found for BG" error log to INFO level when the caller expects no creatures to be found, like on Battleground startup.
Closes#3538
Fix uninitialized values, most of which are false positives, always initialized before being accessed.
Add some asserts and additional NULL checks as sanity checks. Use SpellMgr::EnsureSpellInfo() if the spell id is valid and always supposed to return a valid not-NULL SpellInfo* .
Fix some static analysis issues about:
- uninitialized values, most of which are false positives, always initialized before being accessed
- unchecked return values
- dead code never executed
- bad formatting leading to wrong behavior
Please ensure EventMap is never used with event id set to 0 or those events will never execute.
Fix EVENT_ANUBAR_CHARGE not being triggered due to value 0 being set to the EVENT_ANUBAR_CHARGE enum, even if 0 is a special value for EventMap to specify no event was triggered.
Issue added in a127c0e1a7
Initialize all class fields in the constructor, even if they are set in Reset() and they shouldn't be accessed before Reset() is called.
This fixes 2 Coverity reports.
Fix a NULL dereference exception happening when using .kick command caused by a strtok(NULL) call without a previous call to the string to be tokenized.
Issue added in 101cad1f28
Add an optional parameter "straightLine" to PathGenerator::CalculatePath() which will use raycast instead of path finding and will return only complete path from start to end position.
Implement this new type of path in SPELL_EFFECT_CHARGE , fixing strange behaviors when using Charge with mmaps enabled.
Partially revert e1bee86ee6 and restore 9ba3d07fc2 to store battleground queues as std::list instead of std::deque because deque::erase() invalidates all iterators.
Disable script type SMART_SCRIPT_TYPE_TIMED_ACTIONLIST actions if they are trying to overwrite the timed action list while iterating it.
This was invalidating the iterator and deleting any smart action stored in it, including the current executed one.
Valgrind log:
Invalid read of size 1
at SmartScript::OnUpdate(unsigned int) (SmartScript.cpp:3258)
by SmartAI::UpdateAI(unsigned int) (SmartAI.cpp:331)
by Creature::Update(unsigned int) (Creature.cpp:544)
Address 0x1807d9b2 is 10 bytes after a block of size 40 alloc'd
at operator new(unsigned long) (vg_replace_malloc.c:319)
...
by SmartAIMgr::LoadSmartAIFromDB() (SmartScriptMgr.cpp:231)
by World::SetInitialWorldSettings() (World.cpp:1724)
by Master::Run() (Master.cpp:169)
by main (Main.cpp:142)
Core/Warden: Fix crash
Fix NULL dereference crash in case Warden is disabled (in configs or on Mac OS X platform) and client sends a CMSG_WARDEN_DATA opcode
In case of dynamic file names don't store the FILE* handle in a shared class variable but keep it only at function scope.
Valgrind log:
at _IO_un_link (genops.c:69)
by fclose@@GLIBC_2.2.5 (iofclose.c:55)
by AppenderFile::CloseFile() (AppenderFile.cpp:94)
by AppenderFile::_write(LogMessage const&) (AppenderFile.cpp:66)
by Appender::write(LogMessage&) (Appender.cpp:102)
by Logger::write(LogMessage&) const (Logger.cpp:63)
by Log::write(LogMessage*) (Log.cpp:279)
by Log::vlog(std::string const&, LogLevel, char const*, __va_list_tag*) (Log.cpp:267)
by Log::outMessage(std::string const&, LogLevel, char const*, ...) (Log.h:129)
Address 0x2a1bd2d0 is 0 bytes inside a block of size 568 free'd
at free (vg_replace_malloc.c:468)
by fclose@@GLIBC_2.2.5 (iofclose.c:85)
by AppenderFile::CloseFile() (AppenderFile.cpp:94)
by AppenderFile::_write(LogMessage const&) (AppenderFile.cpp:66)
by Appender::write(LogMessage&) (Appender.cpp:102)
by Logger::write(LogMessage&) const (Logger.cpp:63)
by Log::write(LogMessage*) (Log.cpp:279)
by Log::vlog(std::string const&, LogLevel, char const*, __va_list_tag*) (Log.cpp:267)
by Log::outMessage(std::string const&, LogLevel, char const*, ...) (Log.h:129)
Ignore "w" flag and always use "a" for log files with dynamic name since the file handle is created at every log line and "w" would delete any other logged line from previous _write() calls.
The best solution would be to overwrite only at first call and append at any other call.
Handle SymTagData with inner type of SymTagPointerType.
Increase buffer sizes to avoid buffer overflows.
Avoid infinite loops by logging the children of each type only once.
Avoid too deep nesting by adding a max nesting level.
Print the address for arrays instead of just the name.
Correctly handle the return value of vsnprintf() which returns -1 if the buffer is too small http://msdn.microsoft.com/en-us/library/1kt27hek.aspx . In this case just truncate the output.
This caused a crash on character delete if Logger.entities.player.dump was enabled and set to 3.
Fixes#11539
Arena season sent to client with SMSG_INIT_WORLD_STATES opcode was not sent if Player was on a zone with id 0 on login. Initializing the zone id to -1 will always trigger UpdateZone() properly on login.
Fixes#10623
Fix "DoZoneInCombat called for creature that has empty threat list" error caused by DoZoneInCombat() called by IsSummonedBy() in a script before the Creature is allowed to enter combat.
Log "NULL" for NULL pointers.
Log the correct type instead of <user defined> for pointers and log its fields recursively.
Log locals of all threads.
Log the address if retrieving the value threw an exception.
Increase buffer sizes to avoid buffer overflows.
Fix few issues on WheatyExceptionReport:
- fix NULL dereference exception in GetLogicalAddress() when TC assert in triggered ( *((volatile int*)NULL) = 0; )
- fix infinite loop in DumpTypeIndex() when dumping std types like std::string
- fix FormatSymbolValue() pointing to wrong address when accessing local variable values
- use portable types instead of x86 specific types, this fixes some wrong address issues on x86 platform
- use bigger buffers to format symbols to avoid buffer overflows
Remove assertion in SmartScript added in b8625f41bd and handle SmartScripts with GetBaseObject() returning NULL , like SmartTrigger . In this case SmartScript::mTargetStorage cannot be sanitized and might contain references to free'd WorldObjects if the script doesn't last a single Map::Update() call.
Fix memory leak in InstanceSaveMgr added in 1f170c99ef .
Make InstanceSave::SetToDelete() private since it's not supposed to be used by anything other than InstanceSave or its friend class InstanceSaveMgr .
Fix CMSG_HEARTH_AND_RESURRECT handler trying to resurrect players with 100x hp/mana/energy ( SetHealth()/SetPower() were sanitizing the input anyway ) .
Fix a cooldown issue related to potions allowing Players in combat to use more than 1 potion in a row, especially with high latency.
This also fixes an exploit about using infinite potions in combat just by skipping the client-side check.
The original implementation c064c2e2e1 was missing a check in Spell::CheckCast() about this particular case since Potion cooldown is added only after the Player goes out of combat.
Fixes#1259 .
Fix an array overflow in TransportMgr::GeneratePath() spline code.
Valgrind log:
Invalid read of size 4
at : G3D::Vector3::operator*(float) const (Vector3.h:650)
by : Movement::C_Evaluate(G3D::Vector3 const*, float, G3D::Matrix4 const&, G3D::Vector3&) (Spline.cpp:103)
by : Movement::SplineBase::SegLengthCatmullRom(int) const (Spline.cpp:171)
by : Movement::SplineBase::SegLength(int) const (in /home/jackpoz/trinity/bin/worldserver)
by : Movement::Spline<double>::initLengths() (SplineImpl.h:86)
by : TransportMgr::GeneratePath(GameObjectTemplate const*, TransportTemplate*) (TransportMgr.cpp:125)
by : TransportMgr::LoadTransportTemplates() (TransportMgr.cpp:78)
Address 0x1d07d154 is 8 bytes after a block of size 300 alloc'd
Create ScriptedAIs that require a InstanceScript reference only if the InstanceScript exists, so if these Creatures are in an instance. ScriptedAIs that don't require a InstanceScript reference have not been modified.
This fixes many possible NULL dereference crashes happening when spawning a scripted Creature outside of an instance.
Fixed a GetOwner() and a ToPlayer() NULL dereference crashes too.
Fix a database race condition in authserver that would delay the account ban expiry by 1 login because the query that would have removed the ban was executed asynchronously.
Fix memory leak in Auction House when creating an auction with stackable items and a stack of the same amount in the inventory. Leak added in 15a2acfb52 .
Fix CreatureAI left uninitialized for 1 server tick for Creatures with DeathState:DEAD . This is a workaround required after c9cf2f059f changes .
Valgrind log:
Conditional jump or move depends on uninitialised value(s)
at : npc_draenei_survivor::npc_draenei_survivorAI::MoveInLineOfSight(Unit*) (zone_azuremyst_isle.cpp:104)
by : CreatureAI::MoveInLineOfSight_Safe(Unit*) (CreatureAI.cpp:123)
by : CreatureUnitRelocationWorker(Creature*, Unit*) (GridNotifiers.cpp:134)
by : Trinity::AIRelocationNotifier::Visit(GridRefManager<Creature>&) (GridNotifiers.cpp:252)
Uninitialised value was created by a heap allocation
at : operator new(unsigned long) (vg_replace_malloc.c:319)
by : npc_draenei_survivor::GetAI(Creature*) const (zone_azuremyst_isle.cpp:66)
by : ScriptMgr::GetCreatureAI(Creature*) (ScriptMgr.cpp:792)
by : FactorySelector::selectAI(Creature*) (CreatureAISelector.cpp:41)
by : Creature::AIM_Initialize(CreatureAI*) (Creature.cpp:716)
by : Creature::AddToWorld() (Creature.cpp:190)
Fix some static analysis issues about uninitialized values. Most of them are false positives, always initialized before being accessed, while some of them are real issues spotted by valgrind too.
Fix the stack implementation used in MotionMaster and added few sanity checks to ensure no underflows will be made.
Valgrind log:
Invalid read of size 8
at : MotionMaster::top() const (MotionMaster.h:115)
by : MotionMaster::pop() (MotionMaster.h:91)
by : MotionMaster::~MotionMaster() (MotionMaster.cpp:74)
by : Unit::~Unit() (Unit.cpp:296)
by : Player::~Player() (Player.cpp:880)
by : WorldSession::HandleCharCreateCallback(Trinity::AutoPtr<PreparedResultSet, ACE_Thread_Mutex>, CharacterCreateInfo*) (CharacterHandler.cpp:665)
by : WorldSession::HandleCharCreateCallback(Trinity::AutoPtr<PreparedResultSet, ACE_Thread_Mutex>, CharacterCreateInfo*) (CharacterHandler.cpp:516)
Fix NULL dereference crash in .npc set link happening when passing a not-existent Creature guid as parameter, es. ".npc set link -1" .
Issue spotted by static analysis, added in c05ed659cb
Fix some static analysis issues about missing NULL checks.
Pass safe Creature pointer instead of summoning a Creature, passing the GUID and retrieve the same Creature from GUID.
Fix some static analysis issues, mostly false positive about fields not initialized in the constructor. It's good practice anyway to always initialize them.