From d87ae839a2daf24377fe73556c5ecbbe094e3a4d Mon Sep 17 00:00:00 2001 From: hg <4683435+hgtw@users.noreply.github.com> Date: Thu, 21 Jan 2021 19:02:00 -0500 Subject: [PATCH] Verify members in db on expedition invites Fixes an exploit where multiple accepted cross zone invites could race with cache updates and allow an expedition to exceed its max members --- zone/expedition.cpp | 10 ++++++--- zone/expedition_database.cpp | 42 ++++++++++++++++++++++++++++++++++++ zone/expedition_database.h | 5 ++--- 3 files changed, 51 insertions(+), 6 deletions(-) diff --git a/zone/expedition.cpp b/zone/expedition.cpp index cc88b8c70..f75b09c3f 100644 --- a/zone/expedition.cpp +++ b/zone/expedition.cpp @@ -782,7 +782,7 @@ bool Expedition::ProcessAddConflicts(Client* leader_client, Client* add_client, } // swapping ignores the max player count check since it's a 1:1 change - if (!swapping && GetMemberCount() >= m_max_players) + if (!swapping && ExpeditionDatabase::GetMemberCount(m_id) >= m_max_players) { SendLeaderMessage(leader_client, Chat::Red, DZADD_EXCEED_MAX, { fmt::format_int(m_max_players).str() }); has_conflict = true; @@ -834,9 +834,13 @@ void Expedition::DzInviteResponse(Client* add_client, bool accepted, const std:: } // error if swapping and character was already removed before the accept - if (was_swap_invite && !HasMember(swap_remove_name)) + if (was_swap_invite) { - has_conflicts = true; + auto swap_member = GetMemberData(swap_remove_name); + if (!swap_member.IsValid() || !ExpeditionDatabase::HasMember(m_id, swap_member.char_id)) + { + has_conflicts = true; + } } if (has_conflicts) diff --git a/zone/expedition_database.cpp b/zone/expedition_database.cpp index 7d1fd7a3c..d5bb4925d 100644 --- a/zone/expedition_database.cpp +++ b/zone/expedition_database.cpp @@ -373,6 +373,48 @@ uint32_t ExpeditionDatabase::GetExpeditionIDFromCharacterID(uint32_t character_i return expedition_id; } +uint32_t ExpeditionDatabase::GetMemberCount(uint32_t expedition_id) +{ + LogExpeditionsDetail("Getting expedition [{}] member count from db", expedition_id); + + uint32_t member_count = 0; + if (expedition_id != 0) + { + auto query = fmt::format(SQL( + SELECT COUNT(*) + FROM expedition_members + WHERE expedition_id = {} AND is_current_member = TRUE; + ), expedition_id); + + auto results = database.QueryDatabase(query); + if (results.Success() && results.RowCount() > 0) + { + auto row = results.begin(); + member_count = strtoul(row[0], nullptr, 10); + } + } + return member_count; +} + +bool ExpeditionDatabase::HasMember(uint32_t expedition_id, uint32_t character_id) +{ + LogExpeditionsDetail("Checking db expedition [{}] for character [{}]", expedition_id, character_id); + + if (expedition_id == 0 || character_id == 0) + { + return false; + } + + auto query = fmt::format(SQL( + SELECT id + FROM expedition_members + WHERE expedition_id = {} AND character_id = {} AND is_current_member = TRUE; + ), expedition_id, character_id); + + auto results = database.QueryDatabase(query); + return (results.Success() && results.RowCount() > 0); +} + void ExpeditionDatabase::InsertCharacterLockouts(uint32_t character_id, const std::vector& lockouts) { diff --git a/zone/expedition_database.h b/zone/expedition_database.h index 2efeba2f6..5484ea25a 100644 --- a/zone/expedition_database.h +++ b/zone/expedition_database.h @@ -58,9 +58,8 @@ namespace ExpeditionDatabase void DeleteMembersLockout(const std::vector& members, const std::string& expedition_name, const std::string& event_name); uint32_t GetExpeditionIDFromCharacterID(uint32_t character_id); - std::pair, std::vector> GetMembersLockout( - const std::vector& members, const std::string& expedition_name, - const std::string& event_name); + uint32_t GetMemberCount(uint32_t expedition_id); + bool HasMember(uint32_t expedition_id, uint32_t character_id); void InsertCharacterLockouts(uint32_t character_id, const std::vector& lockouts); void InsertMembersLockout(const std::vector& members,