From 78ba09b979266f84514d21563019f2f26619666b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Wed, 11 Dec 2024 22:46:52 +0100 Subject: [PATCH] Fix server handling when vote creator leaves Reset the vote creator client ID `CGameContext::m_VoteCreator` to `-1` when the player that started the vote leaves instead of keeping the invalid client ID around. Previously, this was causing the server to crash due an assertion in the `IServer::GetAuthedState` function when a ban vote was aborted due rcon authentication change in the `CGameContext::OnSetAuthed` function while the vote creator has already left the server. It was possible to cause this situation as the target of a vote by executing `rcon_auth ""; rcon "kick "; rcon "logout"` (`ban` works for the same effect). This also caused existing votes to use the wrong vote creator if the same client ID was reused by a new client while a vote from a client that left is still running. Ban votes are now only aborted in the `CGameContext::OnSetAuthed` function when 1) a vote is actually active (this was previously not checked), 2) a client logged in (previously logout was also affected), and 3) the vote creator is unset or has a lower authentication level than the player logging in (previously used invalid vote creator). In particular also improve the handling for the `random_map` and `random_unfinished_map` commands when the vote creator leaves. Use the name `nameless tee` instead of `(invalid)` (returned by `IServer::ClientName`) as the requesting player name in queries. Handle requesting player not being set when the sending result message. Avoid overriding the `m_VoteCreator` value in the callback of the `random_unfinished_map` command, which is not necessary and would cause incorrect vote behavior when the command is used while a vote is active. Also ensure that the voting state is initialized properly on server start. --- src/game/server/gamecontext.cpp | 55 ++++++++++++++++++++++----------- src/game/server/score.cpp | 4 +-- 2 files changed, 39 insertions(+), 20 deletions(-) diff --git a/src/game/server/gamecontext.cpp b/src/game/server/gamecontext.cpp index 4230a4f9b6d..7a03e2fb7d4 100644 --- a/src/game/server/gamecontext.cpp +++ b/src/game/server/gamecontext.cpp @@ -86,12 +86,9 @@ void CGameContext::Construct(int Resetting) mem_zero(&m_aPlayerHasInput, sizeof(m_aPlayerHasInput)); m_pController = 0; - m_aVoteCommand[0] = 0; - m_VoteType = VOTE_TYPE_UNKNOWN; - m_VoteCloseTime = 0; + m_pVoteOptionFirst = 0; m_pVoteOptionLast = 0; - m_NumVoteOptions = 0; m_LastMapVote = 0; m_SqlRandomMapResult = nullptr; @@ -100,6 +97,18 @@ void CGameContext::Construct(int Resetting) m_NumMutes = 0; m_NumVoteMutes = 0; + m_VoteCreator = -1; + m_VoteType = VOTE_TYPE_UNKNOWN; + m_VoteCloseTime = 0; + m_VoteUpdate = false; + m_VotePos = 0; + m_aVoteDescription[0] = '\0'; + m_aSixupVoteDescription[0] = '\0'; + m_aVoteCommand[0] = '\0'; + m_aVoteReason[0] = '\0'; + m_NumVoteOptions = 0; + m_VoteEnforce = VOTE_ENFORCE_UNKNOWN; + m_LatestLog = 0; mem_zero(&m_aLogs, sizeof(m_aLogs)); @@ -1074,7 +1083,14 @@ void CGameContext::OnTick() else if(m_VoteEnforce == VOTE_ENFORCE_CANCEL) { char aBuf[64]; - str_format(aBuf, sizeof(aBuf), "'%s' canceled their vote", Server()->ClientName(m_VoteCreator)); + if(m_VoteCreator == -1) + { + str_copy(aBuf, "Vote canceled"); + } + else + { + str_format(aBuf, sizeof(aBuf), "'%s' canceled their vote", Server()->ClientName(m_VoteCreator)); + } SendChat(-1, TEAM_ALL, aBuf); EndVote(); } @@ -1206,7 +1222,7 @@ void CGameContext::OnTick() EndVote(); SendChat(-1, TEAM_ALL, "Vote passed", -1, FLAG_SIX); - if(m_apPlayers[m_VoteCreator] && !IsKickVote() && !IsSpecVote()) + if(m_VoteCreator != -1 && m_apPlayers[m_VoteCreator] && !IsKickVote() && !IsSpecVote()) m_apPlayers[m_VoteCreator]->m_LastVoteCall = 0; } else if(m_VoteEnforce == VOTE_ENFORCE_YES_ADMIN) @@ -1282,7 +1298,7 @@ void CGameContext::OnTick() { if(m_SqlRandomMapResult->m_Success) { - if(PlayerExists(m_SqlRandomMapResult->m_ClientId) && m_SqlRandomMapResult->m_aMessage[0] != '\0') + if(m_SqlRandomMapResult->m_ClientId != -1 && m_apPlayers[m_SqlRandomMapResult->m_ClientId] && m_SqlRandomMapResult->m_aMessage[0] != '\0') SendChat(-1, TEAM_ALL, m_SqlRandomMapResult->m_aMessage); if(m_SqlRandomMapResult->m_aMap[0] != '\0') Server()->ChangeMap(m_SqlRandomMapResult->m_aMap); @@ -1724,6 +1740,10 @@ void CGameContext::OnClientDrop(int ClientId, const char *pReason) m_aTeamMapping[ClientId] = -1; m_VoteUpdate = true; + if(m_VoteCreator == ClientId) + { + m_VoteCreator = -1; + } // update spectator modes for(auto &pPlayer : m_apPlayers) @@ -3133,20 +3153,18 @@ void CGameContext::ConRandomMap(IConsole::IResult *pResult, void *pUserData) { CGameContext *pSelf = (CGameContext *)pUserData; - int Stars = pResult->NumArguments() ? pResult->GetInteger(0) : -1; - - pSelf->m_pScore->RandomMap(pSelf->m_VoteCreator, Stars); + const int ClientId = pResult->m_ClientId == -1 ? pSelf->m_VoteCreator : pResult->m_ClientId; + const int Stars = pResult->NumArguments() ? pResult->GetInteger(0) : -1; + pSelf->m_pScore->RandomMap(ClientId, Stars); } void CGameContext::ConRandomUnfinishedMap(IConsole::IResult *pResult, void *pUserData) { CGameContext *pSelf = (CGameContext *)pUserData; - int Stars = pResult->NumArguments() ? pResult->GetInteger(0) : -1; - if(pResult->m_ClientId != -1) - pSelf->m_VoteCreator = pResult->m_ClientId; - - pSelf->m_pScore->RandomUnfinishedMap(pSelf->m_VoteCreator, Stars); + const int ClientId = pResult->m_ClientId == -1 ? pSelf->m_VoteCreator : pResult->m_ClientId; + const int Stars = pResult->NumArguments() ? pResult->GetInteger(0) : -1; + pSelf->m_pScore->RandomUnfinishedMap(ClientId, Stars); } void CGameContext::ConRestart(IConsole::IResult *pResult, void *pUserData) @@ -4468,17 +4486,18 @@ IGameServer *CreateGameServer() { return new CGameContext; } void CGameContext::OnSetAuthed(int ClientId, int Level) { - if(m_apPlayers[ClientId]) + if(m_apPlayers[ClientId] && m_VoteCloseTime && Level != AUTHED_NO) { char aBuf[512], aIp[NETADDR_MAXSTRSIZE]; Server()->GetClientAddr(ClientId, aIp, sizeof(aIp)); str_format(aBuf, sizeof(aBuf), "ban %s %d Banned by vote", aIp, g_Config.m_SvVoteKickBantime); - if(!str_comp_nocase(m_aVoteCommand, aBuf) && Level > Server()->GetAuthedState(m_VoteCreator)) + if(!str_comp_nocase(m_aVoteCommand, aBuf) && (m_VoteCreator == -1 || Level > Server()->GetAuthedState(m_VoteCreator))) { m_VoteEnforce = CGameContext::VOTE_ENFORCE_NO_ADMIN; - Console()->Print(IConsole::OUTPUT_LEVEL_STANDARD, "CGameContext", "Vote aborted by authorized login."); + Console()->Print(IConsole::OUTPUT_LEVEL_STANDARD, "game", "Vote aborted by authorized login."); } } + if(m_TeeHistorianActive) { if(Level != AUTHED_NO) diff --git a/src/game/server/score.cpp b/src/game/server/score.cpp index 408ab626387..9c3e5a8508f 100644 --- a/src/game/server/score.cpp +++ b/src/game/server/score.cpp @@ -264,7 +264,7 @@ void CScore::RandomMap(int ClientId, int Stars) Tmp->m_Stars = Stars; str_copy(Tmp->m_aCurrentMap, Server()->GetMapName(), sizeof(Tmp->m_aCurrentMap)); str_copy(Tmp->m_aServerType, g_Config.m_SvServerType, sizeof(Tmp->m_aServerType)); - str_copy(Tmp->m_aRequestingPlayer, GameServer()->Server()->ClientName(ClientId), sizeof(Tmp->m_aRequestingPlayer)); + str_copy(Tmp->m_aRequestingPlayer, ClientId == -1 ? "nameless tee" : GameServer()->Server()->ClientName(ClientId), sizeof(Tmp->m_aRequestingPlayer)); m_pPool->Execute(CScoreWorker::RandomMap, std::move(Tmp), "random map"); } @@ -278,7 +278,7 @@ void CScore::RandomUnfinishedMap(int ClientId, int Stars) Tmp->m_Stars = Stars; str_copy(Tmp->m_aCurrentMap, Server()->GetMapName(), sizeof(Tmp->m_aCurrentMap)); str_copy(Tmp->m_aServerType, g_Config.m_SvServerType, sizeof(Tmp->m_aServerType)); - str_copy(Tmp->m_aRequestingPlayer, GameServer()->Server()->ClientName(ClientId), sizeof(Tmp->m_aRequestingPlayer)); + str_copy(Tmp->m_aRequestingPlayer, ClientId == -1 ? "nameless tee" : GameServer()->Server()->ClientName(ClientId), sizeof(Tmp->m_aRequestingPlayer)); m_pPool->Execute(CScoreWorker::RandomUnfinishedMap, std::move(Tmp), "random unfinished map"); }