From ee4602db3d40f061b9ce69445359f47aaf78f278 Mon Sep 17 00:00:00 2001 From: jackpoz Date: Sun, 29 Jun 2014 14:03:29 +0200 Subject: Core/Battleground: Fix crash added in c28345e279f31af2fa32df2411325ccfabe06fc8 Fix a double-delete crash added in c28345e279f31af2fa32df2411325ccfabe06fc8 caused by copy-constructor copying pointers across different Battleground instances. Valgrind log: Invalid read of size 8 at : Battleground::~Battleground() (Battleground.cpp:231) by : BattlegroundWS::~BattlegroundWS() (BattlegroundWS.cpp:74) by : BattlegroundWS::~BattlegroundWS() (BattlegroundWS.cpp:74) by : BattlegroundMgr::Update(unsigned int) (BattlegroundMgr.cpp:103) by : World::Update(unsigned int) (World.cpp:2067) Address 0x5675d140 is 0 bytes inside a block of size 24 free'd at : operator delete(void*) (vg_replace_malloc.c:509) by : ArenaTeamScore::~ArenaTeamScore() (ArenaScore.h:64) by : Battleground::~Battleground() (Battleground.cpp:231) by : BattlegroundWS::~BattlegroundWS() (BattlegroundWS.cpp:74) by : BattlegroundWS::~BattlegroundWS() (BattlegroundWS.cpp:74) by : BattlegroundMgr::Update(unsigned int) (BattlegroundMgr.cpp:103) --- src/server/game/Battlegrounds/ArenaScore.h | 36 ------------------- src/server/game/Battlegrounds/Battleground.cpp | 23 +++++------- src/server/game/Battlegrounds/Battleground.h | 48 ++++++++++++++++++++++++-- 3 files changed, 55 insertions(+), 52 deletions(-) (limited to 'src') diff --git a/src/server/game/Battlegrounds/ArenaScore.h b/src/server/game/Battlegrounds/ArenaScore.h index 7b25427c078..3b976a0a0da 100644 --- a/src/server/game/Battlegrounds/ArenaScore.h +++ b/src/server/game/Battlegrounds/ArenaScore.h @@ -54,40 +54,4 @@ struct ArenaScore : public BattlegroundScore uint8 TeamId; // TEAM_ALLIANCE or TEAM_HORDE }; -struct ArenaTeamScore -{ - friend class Battleground; - - protected: - ArenaTeamScore() : RatingChange(0), MatchmakerRating(0) { } - - virtual ~ArenaTeamScore() { } - - void Assign(int32 ratingChange, uint32 matchMakerRating, std::string const& teamName) - { - RatingChange = ratingChange; - MatchmakerRating = matchMakerRating; - TeamName = teamName; - } - - void BuildRatingInfoBlock(WorldPacket& data) - { - uint32 ratingLost = std::abs(std::min(RatingChange, 0)); - uint32 ratingWon = std::max(RatingChange, 0); - - data << uint32(ratingLost); - data << uint32(ratingWon); - data << uint32(MatchmakerRating); - } - - void BuildTeamInfoBlock(WorldPacket& data) - { - data << TeamName; - } - - int32 RatingChange; - uint32 MatchmakerRating; - std::string TeamName; -}; - #endif // TRINITY_ARENA_SCORE_H diff --git a/src/server/game/Battlegrounds/Battleground.cpp b/src/server/game/Battlegrounds/Battleground.cpp index fd3e13cf583..d840c351d11 100644 --- a/src/server/game/Battlegrounds/Battleground.cpp +++ b/src/server/game/Battlegrounds/Battleground.cpp @@ -170,10 +170,6 @@ Battleground::Battleground() m_ArenaTeamMMR[TEAM_ALLIANCE] = 0; m_ArenaTeamMMR[TEAM_HORDE] = 0; - // Iterate this way for consistency's sake - client expects it to be sent in this order - for (int8 i = WINNER_ALLIANCE; i >= WINNER_HORDE; --i) - _arenaTeamScores[i] = new ArenaTeamScore(); - m_BgRaids[TEAM_ALLIANCE] = NULL; m_BgRaids[TEAM_HORDE] = NULL; @@ -225,10 +221,6 @@ Battleground::~Battleground() for (BattlegroundScoreMap::const_iterator itr = PlayerScores.begin(); itr != PlayerScores.end(); ++itr) delete itr->second; - - // Iterate this way for consistency's sake - client expects it to be sent in this order - for (int8 i = WINNER_ALLIANCE; i >= WINNER_HORDE; --i) - delete _arenaTeamScores[i]; } void Battleground::Update(uint32 diff) @@ -795,8 +787,8 @@ void Battleground::EndBattleground(uint32 winner) uint8 winnerId = GetWinner(); uint8 loserId = winnerId == WINNER_ALLIANCE ? uint8(WINNER_HORDE) : winnerId; - _arenaTeamScores[winnerId]->Assign(winnerChange, winnerMatchmakerRating + winnerMatchmakerChange, winnerArenaTeam->GetName()); - _arenaTeamScores[loserId]->Assign(loserChange, loserMatchmakerRating + loserMatchmakerChange, loserArenaTeam->GetName()); + _arenaTeamScores[winnerId].Assign(winnerChange, winnerMatchmakerRating + winnerMatchmakerChange, winnerArenaTeam->GetName()); + _arenaTeamScores[loserId].Assign(loserChange, loserMatchmakerRating + loserMatchmakerChange, loserArenaTeam->GetName()); TC_LOG_DEBUG("bg.arena", "Arena match Type: %u for Team1Id: %u - Team2Id: %u ended. WinnerTeamId: %u. Winner rating: +%d, Loser rating: %d", m_ArenaType, m_ArenaTeamIds[TEAM_ALLIANCE], m_ArenaTeamIds[TEAM_HORDE], winnerArenaTeam->GetId(), winnerChange, loserChange); if (sWorld->getBoolConfig(CONFIG_ARENA_LOG_EXTENDED_INFO)) @@ -811,8 +803,8 @@ void Battleground::EndBattleground(uint32 winner) // Deduct 16 points from each teams arena-rating if there are no winners after 45+2 minutes else { - _arenaTeamScores[WINNER_ALLIANCE]->Assign(ARENA_TIMELIMIT_POINTS_LOSS, winnerMatchmakerRating + winnerMatchmakerChange, winnerArenaTeam->GetName()); - _arenaTeamScores[WINNER_HORDE]->Assign(ARENA_TIMELIMIT_POINTS_LOSS, loserMatchmakerRating + loserMatchmakerChange, loserArenaTeam->GetName()); + _arenaTeamScores[WINNER_ALLIANCE].Assign(ARENA_TIMELIMIT_POINTS_LOSS, winnerMatchmakerRating + winnerMatchmakerChange, winnerArenaTeam->GetName()); + _arenaTeamScores[WINNER_HORDE].Assign(ARENA_TIMELIMIT_POINTS_LOSS, loserMatchmakerRating + loserMatchmakerChange, loserArenaTeam->GetName()); winnerArenaTeam->FinishGame(ARENA_TIMELIMIT_POINTS_LOSS); loserArenaTeam->FinishGame(ARENA_TIMELIMIT_POINTS_LOSS); @@ -1107,6 +1099,9 @@ void Battleground::Reset() delete itr->second; PlayerScores.clear(); + for (int8 i = WINNER_ALLIANCE; i >= WINNER_HORDE; --i) + _arenaTeamScores[i].Reset(); + ResetBGSubclass(); } @@ -1368,10 +1363,10 @@ void Battleground::BuildPvPLogDataPacket(WorldPacket& data) { // it seems this must be according to BG_WINNER_A/H and _NOT_ TEAM_A/H for (int8 i = WINNER_ALLIANCE; i >= WINNER_HORDE; --i) - _arenaTeamScores[i]->BuildRatingInfoBlock(data); + _arenaTeamScores[i].BuildRatingInfoBlock(data); for (int8 i = WINNER_ALLIANCE; i >= WINNER_HORDE; --i) - _arenaTeamScores[i]->BuildTeamInfoBlock(data); + _arenaTeamScores[i].BuildTeamInfoBlock(data); } if (GetStatus() == STATUS_WAIT_LEAVE) diff --git a/src/server/game/Battlegrounds/Battleground.h b/src/server/game/Battlegrounds/Battleground.h index 7d1bf550695..e353cd1dc62 100644 --- a/src/server/game/Battlegrounds/Battleground.h +++ b/src/server/game/Battlegrounds/Battleground.h @@ -22,6 +22,7 @@ #include "Common.h" #include "SharedDefines.h" #include "DBCEnums.h" +#include "WorldPacket.h" class Creature; class GameObject; @@ -32,7 +33,6 @@ class WorldObject; class WorldPacket; class BattlegroundMap; -struct ArenaTeamScore; struct BattlegroundScore; struct Position; struct PvPDifficultyEntry; @@ -611,7 +611,51 @@ class Battleground uint32 m_ArenaTeamIds[BG_TEAMS_COUNT]; uint32 m_ArenaTeamMMR[BG_TEAMS_COUNT]; - ArenaTeamScore* _arenaTeamScores[BG_TEAMS_COUNT]; + + struct ArenaTeamScore + { + friend class Battleground; + + protected: + ArenaTeamScore() : RatingChange(0), MatchmakerRating(0) { } + + virtual ~ArenaTeamScore() { } + + void Assign(int32 ratingChange, uint32 matchMakerRating, std::string const& teamName) + { + RatingChange = ratingChange; + MatchmakerRating = matchMakerRating; + TeamName = teamName; + } + + void BuildRatingInfoBlock(WorldPacket& data) + { + uint32 ratingLost = std::abs(std::min(RatingChange, 0)); + uint32 ratingWon = std::max(RatingChange, 0); + + data << uint32(ratingLost); + data << uint32(ratingWon); + data << uint32(MatchmakerRating); + } + + void BuildTeamInfoBlock(WorldPacket& data) + { + data << TeamName; + } + + void Reset() + { + RatingChange = 0; + MatchmakerRating = 0; + TeamName.clear(); + } + + int32 RatingChange; + uint32 MatchmakerRating; + std::string TeamName; + }; + + ArenaTeamScore _arenaTeamScores[BG_TEAMS_COUNT]; // Limits uint32 m_LevelMin; -- cgit v1.2.3 From c674f229cb3d9abc723df9e7f87ffb15c9655cca Mon Sep 17 00:00:00 2001 From: jackpoz Date: Sun, 29 Jun 2014 15:51:34 +0200 Subject: Core/Mail: Fix exploit that didn't allow recipient to delete the email --- src/server/game/Handlers/MailHandler.cpp | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src') diff --git a/src/server/game/Handlers/MailHandler.cpp b/src/server/game/Handlers/MailHandler.cpp index ffcf74ea7c2..1270f4e6419 100644 --- a/src/server/game/Handlers/MailHandler.cpp +++ b/src/server/game/Handlers/MailHandler.cpp @@ -322,6 +322,10 @@ void WorldSession::HandleSendMail(WorldPacket& recvData) // If theres is an item, there is a one hour delivery delay if sent to another account's character. uint32 deliver_delay = needItemDelay ? sWorld->getIntConfig(CONFIG_MAIL_DELIVERY_DELAY) : 0; + // don't ask for COD if there are no items + if (items_count == 0) + COD = 0; + // will delete item or place to receiver mail list draft .AddMoney(money) -- cgit v1.2.3 From 876e9dde01265edef31eb28167dd82797800bb1f Mon Sep 17 00:00:00 2001 From: joschiwald Date: Sun, 29 Jun 2014 16:53:28 +0200 Subject: Core/Battleground: addition to c28345e279f31af2fa32df2411325ccfabe06fc8 --- src/server/game/Battlegrounds/ArenaScore.h | 8 +++++++- src/server/game/Battlegrounds/Battleground.cpp | 23 ++++++++++++----------- src/server/game/Battlegrounds/Battleground.h | 1 + src/server/game/Battlegrounds/BattlegroundScore.h | 2 +- 4 files changed, 21 insertions(+), 13 deletions(-) (limited to 'src') diff --git a/src/server/game/Battlegrounds/ArenaScore.h b/src/server/game/Battlegrounds/ArenaScore.h index 3b976a0a0da..fdeb13adc3d 100644 --- a/src/server/game/Battlegrounds/ArenaScore.h +++ b/src/server/game/Battlegrounds/ArenaScore.h @@ -40,6 +40,12 @@ struct ArenaScore : public BattlegroundScore data << uint8(TeamId); data << uint32(DamageDone); data << uint32(HealingDone); + + BuildObjectivesBlock(data); + } + + void BuildObjectivesBlock(WorldPacket& data) final + { data << uint32(0); // Objectives Count } @@ -51,7 +57,7 @@ struct ArenaScore : public BattlegroundScore return stream.str(); } - uint8 TeamId; // TEAM_ALLIANCE or TEAM_HORDE + uint8 TeamId; // bgTeamId }; #endif // TRINITY_ARENA_SCORE_H diff --git a/src/server/game/Battlegrounds/Battleground.cpp b/src/server/game/Battlegrounds/Battleground.cpp index d840c351d11..4e077979b3e 100644 --- a/src/server/game/Battlegrounds/Battleground.cpp +++ b/src/server/game/Battlegrounds/Battleground.cpp @@ -755,7 +755,7 @@ void Battleground::EndBattleground(uint32 winner) } else { - SetWinner(3); + SetWinner(3); // weird } SetStatus(STATUS_WAIT_LEAVE); @@ -784,11 +784,13 @@ void Battleground::EndBattleground(uint32 winner) SetArenaMatchmakerRating(winner, winnerMatchmakerRating + winnerMatchmakerChange); SetArenaMatchmakerRating(GetOtherTeam(winner), loserMatchmakerRating + loserMatchmakerChange); - uint8 winnerId = GetWinner(); - uint8 loserId = winnerId == WINNER_ALLIANCE ? uint8(WINNER_HORDE) : winnerId; + // bg team that the client expects is different to TeamId + // alliance 1, horde 0 + uint8 winnerTeam = winner == ALLIANCE ? WINNER_ALLIANCE : WINNER_HORDE; + uint8 loserTeam = winner == ALLIANCE ? WINNER_HORDE : WINNER_ALLIANCE; - _arenaTeamScores[winnerId].Assign(winnerChange, winnerMatchmakerRating + winnerMatchmakerChange, winnerArenaTeam->GetName()); - _arenaTeamScores[loserId].Assign(loserChange, loserMatchmakerRating + loserMatchmakerChange, loserArenaTeam->GetName()); + _arenaTeamScores[winnerTeam].Assign(winnerChange, winnerMatchmakerRating, winnerArenaTeam->GetName()); + _arenaTeamScores[loserTeam].Assign(loserChange, loserMatchmakerRating, loserArenaTeam->GetName()); TC_LOG_DEBUG("bg.arena", "Arena match Type: %u for Team1Id: %u - Team2Id: %u ended. WinnerTeamId: %u. Winner rating: +%d, Loser rating: %d", m_ArenaType, m_ArenaTeamIds[TEAM_ALLIANCE], m_ArenaTeamIds[TEAM_HORDE], winnerArenaTeam->GetId(), winnerChange, loserChange); if (sWorld->getBoolConfig(CONFIG_ARENA_LOG_EXTENDED_INFO)) @@ -803,8 +805,8 @@ void Battleground::EndBattleground(uint32 winner) // Deduct 16 points from each teams arena-rating if there are no winners after 45+2 minutes else { - _arenaTeamScores[WINNER_ALLIANCE].Assign(ARENA_TIMELIMIT_POINTS_LOSS, winnerMatchmakerRating + winnerMatchmakerChange, winnerArenaTeam->GetName()); - _arenaTeamScores[WINNER_HORDE].Assign(ARENA_TIMELIMIT_POINTS_LOSS, loserMatchmakerRating + loserMatchmakerChange, loserArenaTeam->GetName()); + _arenaTeamScores[WINNER_ALLIANCE].Assign(ARENA_TIMELIMIT_POINTS_LOSS, winnerMatchmakerRating, winnerArenaTeam->GetName()); + _arenaTeamScores[WINNER_HORDE].Assign(ARENA_TIMELIMIT_POINTS_LOSS, loserMatchmakerRating, loserArenaTeam->GetName()); winnerArenaTeam->FinishGame(ARENA_TIMELIMIT_POINTS_LOSS); loserArenaTeam->FinishGame(ARENA_TIMELIMIT_POINTS_LOSS); @@ -1099,7 +1101,7 @@ void Battleground::Reset() delete itr->second; PlayerScores.clear(); - for (int8 i = WINNER_ALLIANCE; i >= WINNER_HORDE; --i) + for (uint8 i = 0; i < BG_TEAMS_COUNT; ++i) _arenaTeamScores[i].Reset(); ResetBGSubclass(); @@ -1361,11 +1363,10 @@ void Battleground::BuildPvPLogDataPacket(WorldPacket& data) if (type) // arena { - // it seems this must be according to BG_WINNER_A/H and _NOT_ TEAM_A/H - for (int8 i = WINNER_ALLIANCE; i >= WINNER_HORDE; --i) + for (uint8 i = 0; i < BG_TEAMS_COUNT; ++i) _arenaTeamScores[i].BuildRatingInfoBlock(data); - for (int8 i = WINNER_ALLIANCE; i >= WINNER_HORDE; --i) + for (uint8 i = 0; i < BG_TEAMS_COUNT; ++i) _arenaTeamScores[i].BuildTeamInfoBlock(data); } diff --git a/src/server/game/Battlegrounds/Battleground.h b/src/server/game/Battlegrounds/Battleground.h index e353cd1dc62..2904ec1dc7e 100644 --- a/src/server/game/Battlegrounds/Battleground.h +++ b/src/server/game/Battlegrounds/Battleground.h @@ -633,6 +633,7 @@ class Battleground uint32 ratingLost = std::abs(std::min(RatingChange, 0)); uint32 ratingWon = std::max(RatingChange, 0); + // should be old rating, new rating, and client will calculate rating change itself data << uint32(ratingLost); data << uint32(ratingWon); data << uint32(MatchmakerRating); diff --git a/src/server/game/Battlegrounds/BattlegroundScore.h b/src/server/game/Battlegrounds/BattlegroundScore.h index 7fedc4dbc4d..ae65721b516 100644 --- a/src/server/game/Battlegrounds/BattlegroundScore.h +++ b/src/server/game/Battlegrounds/BattlegroundScore.h @@ -101,7 +101,7 @@ struct BattlegroundScore BuildObjectivesBlock(data); } - virtual void BuildObjectivesBlock(WorldPacket& /*data*/) { } + virtual void BuildObjectivesBlock(WorldPacket& /*data*/) = 0; // For Logging purpose virtual std::string ToString() const { return ""; } -- cgit v1.2.3