From 944d7290ad68a401a31ad3bb5911248cbf6bf028 Mon Sep 17 00:00:00 2001 From: Shauren Date: Thu, 14 Feb 2013 11:41:20 +0100 Subject: Core/Loot: Minor change to previous commit, thanks Vincent_Michael for reminding me about this method in Map class --- src/server/game/Entities/Creature/Creature.cpp | 2 +- src/server/game/Entities/GameObject/GameObject.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/server/game/Entities/Creature/Creature.cpp b/src/server/game/Entities/Creature/Creature.cpp index 220bc4378ae..6208bcc4727 100644 --- a/src/server/game/Entities/Creature/Creature.cpp +++ b/src/server/game/Entities/Creature/Creature.cpp @@ -290,7 +290,7 @@ bool Creature::InitEntry(uint32 Entry, uint32 /*team*/, const CreatureData* data } // Initialize loot duplicate count depending on raid difficulty - if (GetMap()->IsRaid() && GetMap()->GetSpawnMode() & RAID_DIFFICULTY_MASK_25MAN) + if (GetMap()->Is25ManRaid()) loot.maxDuplicates = 3; SetEntry(Entry); // normal entry always diff --git a/src/server/game/Entities/GameObject/GameObject.cpp b/src/server/game/Entities/GameObject/GameObject.cpp index 61e7943453e..c41c8e71a44 100644 --- a/src/server/game/Entities/GameObject/GameObject.cpp +++ b/src/server/game/Entities/GameObject/GameObject.cpp @@ -259,7 +259,7 @@ bool GameObject::Create(uint32 guidlow, uint32 name_id, Map* map, uint32 phaseMa AIM_Initialize(); // Initialize loot duplicate count depending on raid difficulty - if (map->IsRaid() && map->GetSpawnMode() & RAID_DIFFICULTY_MASK_25MAN) + if (map->Is25ManRaid()) loot.maxDuplicates = 3; return true; -- cgit v1.2.3 From b4be224004fb39c3d39507cefa929d6422e7f928 Mon Sep 17 00:00:00 2001 From: Machiavelli Date: Thu, 14 Feb 2013 11:57:43 +0100 Subject: Core/RA: Fix a possible crash Caused when RASocket::handle_close (event-driven) would delete the underlying object before RASocket::commandFinished callback was executed for that object. Dereferencing freed pointers is bad. --- src/server/worldserver/RemoteAccess/RASocket.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/server/worldserver/RemoteAccess/RASocket.cpp b/src/server/worldserver/RemoteAccess/RASocket.cpp index ee05e83ad4d..3e9c12b32d8 100644 --- a/src/server/worldserver/RemoteAccess/RASocket.cpp +++ b/src/server/worldserver/RemoteAccess/RASocket.cpp @@ -59,7 +59,6 @@ int RASocket::handle_close(ACE_HANDLE, ACE_Reactor_Mask) sLog->outInfo(LOG_FILTER_REMOTECOMMAND, "Closing connection"); peer().close_reader(); wait(); - destroy(); return 0; } @@ -412,10 +411,12 @@ void RASocket::commandFinished(void* callbackArg, bool /*success*/) // the message is 0 size control message to tell that command output is finished // hence we don't put timeout, because it shouldn't increase queue size and shouldn't block - if (socket->putq(mb) == -1) - { + if (socket->peer().get_handle() == ACE_INVALID_HANDLE // this can happen if this code is triggered when handle_close has already called peer().close_writer() + || socket->putq(mb->duplicate()) == -1) // getting here is bad, command can't be marked as complete sLog->outDebug(LOG_FILTER_REMOTECOMMAND, "Failed to enqueue command end message. Error is %s", ACE_OS::strerror(errno)); - mb->release(); - } + + mb->release(); + socket->destroy(); // deletes the object + socket = NULL; } -- cgit v1.2.3 From 203cf7cbf0d860dcc12ab625226b1c7567854356 Mon Sep 17 00:00:00 2001 From: Machiavelli Date: Thu, 14 Feb 2013 13:30:51 +0100 Subject: Core/RA: Addition to previous crashfix To make sure it also works for sessions that use more than 1 command before closing. --- src/server/worldserver/RemoteAccess/RASocket.cpp | 16 ++++++++++++---- src/server/worldserver/RemoteAccess/RASocket.h | 1 + 2 files changed, 13 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/server/worldserver/RemoteAccess/RASocket.cpp b/src/server/worldserver/RemoteAccess/RASocket.cpp index 3e9c12b32d8..359abd39901 100644 --- a/src/server/worldserver/RemoteAccess/RASocket.cpp +++ b/src/server/worldserver/RemoteAccess/RASocket.cpp @@ -33,6 +33,7 @@ RASocket::RASocket() { _minLevel = uint8(ConfigMgr::GetIntDefault("RA.MinLevel", 3)); + _commandExecuting = false; } RASocket::~RASocket() @@ -59,6 +60,13 @@ int RASocket::handle_close(ACE_HANDLE, ACE_Reactor_Mask) sLog->outInfo(LOG_FILTER_REMOTECOMMAND, "Closing connection"); peer().close_reader(); wait(); + // While the above wait() will wait for the ::svc() to finish, it will not wait for the async event + // RASocket::commandfinished to be completed. Calling destroy() before the latter function ends + // will lead to using a freed pointer -> crash. + while (_commandExecuting.value()) + ACE_OS::sleep(1); + + destroy(); return 0; } @@ -149,6 +157,7 @@ int RASocket::process_command(const std::string& command) return -1; } + _commandExecuting = true; CliCommandHolder* cmd = new CliCommandHolder(this, command.c_str(), &RASocket::zprint, &RASocket::commandFinished); sWorld->QueueCliCommand(cmd); @@ -411,12 +420,11 @@ void RASocket::commandFinished(void* callbackArg, bool /*success*/) // the message is 0 size control message to tell that command output is finished // hence we don't put timeout, because it shouldn't increase queue size and shouldn't block - if (socket->peer().get_handle() == ACE_INVALID_HANDLE // this can happen if this code is triggered when handle_close has already called peer().close_writer() - || socket->putq(mb->duplicate()) == -1) + if (socket->putq(mb->duplicate()) == -1) // getting here is bad, command can't be marked as complete sLog->outDebug(LOG_FILTER_REMOTECOMMAND, "Failed to enqueue command end message. Error is %s", ACE_OS::strerror(errno)); mb->release(); - socket->destroy(); // deletes the object - socket = NULL; + + socket->_commandExecuting = false; } diff --git a/src/server/worldserver/RemoteAccess/RASocket.h b/src/server/worldserver/RemoteAccess/RASocket.h index d23d1f0d5fd..e92cb35eaf0 100644 --- a/src/server/worldserver/RemoteAccess/RASocket.h +++ b/src/server/worldserver/RemoteAccess/RASocket.h @@ -57,6 +57,7 @@ class RASocket: public ACE_Svc_Handler private: /// Minimum security level required to connect uint8 _minLevel; + ACE_Atomic_Op _commandExecuting; }; #endif /// @} -- cgit v1.2.3 From 8befbdcc5675d2249e4e252ba303c149df0ecad3 Mon Sep 17 00:00:00 2001 From: Machiavelli Date: Thu, 14 Feb 2013 15:01:33 +0100 Subject: Core/LFG: Fix a crash Fix a crash in case a player is recognized as in a LFG dungeon, but without a valid group --- src/server/game/DungeonFinding/LFGScripts.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'src') diff --git a/src/server/game/DungeonFinding/LFGScripts.cpp b/src/server/game/DungeonFinding/LFGScripts.cpp index a4716de9524..22b86a094dd 100644 --- a/src/server/game/DungeonFinding/LFGScripts.cpp +++ b/src/server/game/DungeonFinding/LFGScripts.cpp @@ -95,6 +95,20 @@ void LFGPlayerScript::OnMapChanged(Player* player) if (sLFGMgr->inLfgDungeonMap(player->GetGUID(), map->GetId(), map->GetDifficulty())) { Group* group = player->GetGroup(); + // This function is also called when players log in + // if for some reason the LFG system recognises the player as being in a LFG dungeon, + // but the player was loaded without a valid group, we'll teleport to homebind to prevent + // crashes or other undefined behaviour + if (!group) + { + sLFGMgr->LeaveLfg(player->GetGUID()); + player->RemoveAurasDueToSpell(LFG_SPELL_LUCK_OF_THE_DRAW); + player->TeleportTo(player->m_homebindMapId, player->m_homebindX, player->m_homebindY, player->m_homebindZ, 0.0f); + sLog->outError(LOG_FILTER_LFG, "LFGPlayerScript::OnMapChanged, Player %s (%u) is in LFG dungeon map but does not have a valid group! " + "Teleporting to homebind.", player->GetName().c_str(), player->GetGUIDLow()); + return; + } + for (GroupReference* itr = group->GetFirstMember(); itr != NULL; itr = itr->next()) if (Player* member = itr->getSource()) player->GetSession()->SendNameQueryOpcode(member->GetGUID()); -- cgit v1.2.3 From 877a7e9968a0d2a1e3f646a94bfb81018ef318a9 Mon Sep 17 00:00:00 2001 From: Machiavelli Date: Thu, 14 Feb 2013 15:43:26 +0100 Subject: Core/Handlers: Fix a crash in HandleCharFactionOrRaceChange Ensure there's always valid character name data present, even for characters that were manually inserted into the database, by adding character name data on char enum if needed. --- src/server/game/Handlers/CharacterHandler.cpp | 10 ++++++++++ src/server/game/World/World.h | 1 + 2 files changed, 11 insertions(+) (limited to 'src') diff --git a/src/server/game/Handlers/CharacterHandler.cpp b/src/server/game/Handlers/CharacterHandler.cpp index d8e50be11f0..1e6dfa8c94c 100644 --- a/src/server/game/Handlers/CharacterHandler.cpp +++ b/src/server/game/Handlers/CharacterHandler.cpp @@ -225,6 +225,8 @@ void WorldSession::HandleCharEnum(PreparedQueryResult result) if (Player::BuildEnumData(result, &data)) { _allowedCharsToLogin.insert(guidlow); + if (!sWorld->HasCharacterNameData(guidlow)) // This can happen if characters are inserted into the database manually. Core hasn't loaded name data yet. + sWorld->AddCharacterNameData(guidlow, (*result)[1].GetString(), (*result)[4].GetUInt8(), (*result)[2].GetUInt8(), (*result)[3].GetUInt8(), (*result)[7].GetUInt8()); ++num; } } @@ -1624,6 +1626,14 @@ void WorldSession::HandleCharFactionOrRaceChange(WorldPacket& recvData) // get the players old (at this moment current) race CharacterNameData const* nameData = sWorld->GetCharacterNameData(lowGuid); + if (!nameData) + { + WorldPacket data(SMSG_CHAR_FACTION_CHANGE, 1); + data << uint8(CHAR_CREATE_ERROR); + SendPacket(&data); + return; + } + uint8 oldRace = nameData->m_race; uint8 playerClass = nameData->m_class; uint8 level = nameData->m_level; diff --git a/src/server/game/World/World.h b/src/server/game/World/World.h index 95c10329690..30021338c4e 100644 --- a/src/server/game/World/World.h +++ b/src/server/game/World/World.h @@ -734,6 +734,7 @@ class World void UpdateCharacterNameData(uint32 guid, std::string const& name, uint8 gender = GENDER_NONE, uint8 race = RACE_NONE); void UpdateCharacterNameDataLevel(uint32 guid, uint8 level); void DeleteCharaceterNameData(uint32 guid) { _characterNameDataMap.erase(guid); } + bool HasCharacterNameData(uint32 guid) { return _characterNameDataMap.find(guid) != _characterNameDataMap.end(); } uint32 GetCleaningFlags() const { return m_CleaningFlags; } void SetCleaningFlags(uint32 flags) { m_CleaningFlags = flags; } -- cgit v1.2.3 From 1093abb1ad166546380b3b77bd83c9e934656c98 Mon Sep 17 00:00:00 2001 From: Machiavelli Date: Thu, 14 Feb 2013 16:07:36 +0100 Subject: Core/Handlers: Prevent some cheating in CharacterHandler Prevents factionchanging and char customising of characters that do not belong to the current account. --- src/server/game/Handlers/CharacterHandler.cpp | 25 +++++++++++++++++++++---- src/server/game/Server/WorldSession.h | 6 +++--- 2 files changed, 24 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/server/game/Handlers/CharacterHandler.cpp b/src/server/game/Handlers/CharacterHandler.cpp index 1e6dfa8c94c..02d4d1b1a25 100644 --- a/src/server/game/Handlers/CharacterHandler.cpp +++ b/src/server/game/Handlers/CharacterHandler.cpp @@ -215,7 +215,7 @@ void WorldSession::HandleCharEnum(PreparedQueryResult result) data << num; - _allowedCharsToLogin.clear(); + _legitCharacters.clear(); if (result) { do @@ -224,7 +224,7 @@ void WorldSession::HandleCharEnum(PreparedQueryResult result) sLog->outInfo(LOG_FILTER_NETWORKIO, "Loading char guid %u from account %u.", guidlow, GetAccountId()); if (Player::BuildEnumData(result, &data)) { - _allowedCharsToLogin.insert(guidlow); + _legitCharacters.insert(guidlow); if (!sWorld->HasCharacterNameData(guidlow)) // This can happen if characters are inserted into the database manually. Core hasn't loaded name data yet. sWorld->AddCharacterNameData(guidlow, (*result)[1].GetString(), (*result)[4].GetUInt8(), (*result)[2].GetUInt8(), (*result)[3].GetUInt8(), (*result)[7].GetUInt8()); ++num; @@ -759,7 +759,7 @@ void WorldSession::HandlePlayerLoginOpcode(WorldPacket& recvData) recvData >> playerGuid; - if (!CharCanLogin(GUID_LOPART(playerGuid))) + if (!IsLegitCharacterForAccount(GUID_LOPART(playerGuid))) { sLog->outError(LOG_FILTER_NETWORKIO, "Account (%u) can't login with that character (%u).", GetAccountId(), GUID_LOPART(playerGuid)); KickPlayer(); @@ -1386,6 +1386,14 @@ void WorldSession::HandleCharCustomize(WorldPacket& recvData) std::string newName; recvData >> guid; + if (!IsLegitCharacterForAccount(GUID_LOPART(guid))) + { + sLog->outError(LOG_FILTER_NETWORKIO, "Account %u, IP: %s tried to customise character %u, but it does not belong to their account!", + GetAccountId(), GetRemoteAddress().c_str(), GUID_LOPART(guid)); + recvData.rfinish(); + KickPlayer(); + } + recvData >> newName; uint8 gender, skin, face, hairStyle, hairColor, facialHair; @@ -1394,7 +1402,7 @@ void WorldSession::HandleCharCustomize(WorldPacket& recvData) PreparedStatement* stmt = CharacterDatabase.GetPreparedStatement(CHAR_SEL_CHARACTER_AT_LOGIN); stmt->setUInt32(0, GUID_LOPART(guid)); - + // TODO: Make async with callback PreparedQueryResult result = CharacterDatabase.Query(stmt); if (!result) @@ -1619,6 +1627,15 @@ void WorldSession::HandleCharFactionOrRaceChange(WorldPacket& recvData) std::string newname; uint8 gender, skin, face, hairStyle, hairColor, facialHair, race; recvData >> guid; + + if (!IsLegitCharacterForAccount(GUID_LOPART(guid))) + { + sLog->outError(LOG_FILTER_NETWORKIO, "Account %u, IP: %s tried to factionchange character %u, but it does not belong to their account!", + GetAccountId(), GetRemoteAddress().c_str(), GUID_LOPART(guid)); + recvData.rfinish(); + KickPlayer(); + } + recvData >> newname; recvData >> gender >> skin >> hairColor >> hairStyle >> facialHair >> face >> race; diff --git a/src/server/game/Server/WorldSession.h b/src/server/game/Server/WorldSession.h index d6877b8a18a..c1d7f0b00db 100644 --- a/src/server/game/Server/WorldSession.h +++ b/src/server/game/Server/WorldSession.h @@ -919,14 +919,14 @@ class WorldSession void LogUnprocessedTail(WorldPacket* packet); // EnumData helpers - bool CharCanLogin(uint32 lowGUID) + bool IsLegitCharacterForAccount(uint32 lowGUID) { - return _allowedCharsToLogin.find(lowGUID) != _allowedCharsToLogin.end(); + return _legitCharacters.find(lowGUID) != _legitCharacters.end(); } // this stores the GUIDs of the characters who can login // characters who failed on Player::BuildEnumData shouldn't login - std::set _allowedCharsToLogin; + std::set _legitCharacters; uint32 m_GUIDLow; // set loggined or recently logout player (while m_playerRecentlyLogout set) Player* _player; -- cgit v1.2.3 From 771598b67ad3263ad7e172d1589f1bce5e10861c Mon Sep 17 00:00:00 2001 From: Machiavelli Date: Thu, 14 Feb 2013 16:12:58 +0100 Subject: Core/Handlers: Missing returns after KickPlayer() call --- src/server/game/Handlers/CharacterHandler.cpp | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src') diff --git a/src/server/game/Handlers/CharacterHandler.cpp b/src/server/game/Handlers/CharacterHandler.cpp index 02d4d1b1a25..817a7682678 100644 --- a/src/server/game/Handlers/CharacterHandler.cpp +++ b/src/server/game/Handlers/CharacterHandler.cpp @@ -1392,6 +1392,7 @@ void WorldSession::HandleCharCustomize(WorldPacket& recvData) GetAccountId(), GetRemoteAddress().c_str(), GUID_LOPART(guid)); recvData.rfinish(); KickPlayer(); + return; } recvData >> newName; @@ -1634,6 +1635,7 @@ void WorldSession::HandleCharFactionOrRaceChange(WorldPacket& recvData) GetAccountId(), GetRemoteAddress().c_str(), GUID_LOPART(guid)); recvData.rfinish(); KickPlayer(); + return; } recvData >> newname; -- cgit v1.2.3 From d232977ec4f6bd90affc0c1d429138ba630ad92d Mon Sep 17 00:00:00 2001 From: Shauren Date: Thu, 14 Feb 2013 16:54:31 +0100 Subject: Core/Maps: Prevent creating maps without a valid Map.dbc entry Closes #9181 --- src/server/game/Entities/Player/Player.cpp | 3 +++ src/server/game/Maps/MapManager.cpp | 9 +++++---- src/server/game/Scripting/ScriptMgr.cpp | 4 ++-- 3 files changed, 10 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/server/game/Entities/Player/Player.cpp b/src/server/game/Entities/Player/Player.cpp index 37502d69456..bdf3f1780d2 100644 --- a/src/server/game/Entities/Player/Player.cpp +++ b/src/server/game/Entities/Player/Player.cpp @@ -7376,6 +7376,9 @@ uint32 Player::GetZoneIdFromDB(uint64 guid) float posy = fields[2].GetFloat(); float posz = fields[3].GetFloat(); + if (!sMapStore.LookupEntry(map)) + return 0; + zone = sMapMgr->GetZoneId(map, posx, posy, posz); if (zone > 0) diff --git a/src/server/game/Maps/MapManager.cpp b/src/server/game/Maps/MapManager.cpp index 755d443091a..f44a9dd865f 100644 --- a/src/server/game/Maps/MapManager.cpp +++ b/src/server/game/Maps/MapManager.cpp @@ -105,16 +105,17 @@ Map* MapManager::CreateBaseMap(uint32 id) { TRINITY_GUARD(ACE_Thread_Mutex, Lock); - const MapEntry* entry = sMapStore.LookupEntry(id); - if (entry && entry->Instanceable()) - { + MapEntry const* entry = sMapStore.LookupEntry(id); + ASSERT(entry); + + if (entry->Instanceable()) map = new MapInstanced(id, i_gridCleanUpDelay); - } else { map = new Map(id, i_gridCleanUpDelay, 0, REGULAR_DIFFICULTY); map->LoadRespawnTimes(); } + i_maps[id] = map; } diff --git a/src/server/game/Scripting/ScriptMgr.cpp b/src/server/game/Scripting/ScriptMgr.cpp index 8321b88962f..32fe5d2ef33 100644 --- a/src/server/game/Scripting/ScriptMgr.cpp +++ b/src/server/game/Scripting/ScriptMgr.cpp @@ -489,14 +489,14 @@ void ScriptMgr::OnGroupRateCalculation(float& rate, uint32 count, bool isRaid) } #define SCR_MAP_BGN(M, V, I, E, C, T) \ - if (V->GetEntry()->T()) \ + if (V->GetEntry() && V->GetEntry()->T()) \ { \ FOR_SCRIPTS(M, I, E) \ { \ MapEntry const* C = I->second->GetEntry(); \ if (!C) \ continue; \ - if (entry->MapID == V->GetId()) \ + if (C->MapID == V->GetId()) \ { #define SCR_MAP_END \ -- cgit v1.2.3 From 45363b8216f03f2dd4ce21cfb5ce183560374dd8 Mon Sep 17 00:00:00 2001 From: WishToDie Date: Thu, 14 Feb 2013 22:54:06 +0200 Subject: Scripts/Gundrak: Drakkari Colossus Drakkari Colossus should not move when in freeze phase. --- src/server/scripts/Northrend/Gundrak/boss_drakkari_colossus.cpp | 1 + 1 file changed, 1 insertion(+) (limited to 'src') diff --git a/src/server/scripts/Northrend/Gundrak/boss_drakkari_colossus.cpp b/src/server/scripts/Northrend/Gundrak/boss_drakkari_colossus.cpp index 58301df4ca2..76b2e7bac7e 100644 --- a/src/server/scripts/Northrend/Gundrak/boss_drakkari_colossus.cpp +++ b/src/server/scripts/Northrend/Gundrak/boss_drakkari_colossus.cpp @@ -148,6 +148,7 @@ class boss_drakkari_colossus : public CreatureScript DoCast(SPELL_EMERGE); break; case ACTION_FREEZE_COLOSSUS: + me->GetMotionMaster()->Clear(); me->GetMotionMaster()->MoveIdle(); me->SetReactState(REACT_PASSIVE); -- cgit v1.2.3