Skip to content

Commit

Permalink
Fix server handling when vote creator leaves
Browse files Browse the repository at this point in the history
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 "<password>"; rcon "kick <vote creator>"; 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.
  • Loading branch information
Robyt3 committed Dec 12, 2024
1 parent 3058b1c commit 78ba09b
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 20 deletions.
55 changes: 37 additions & 18 deletions src/game/server/gamecontext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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));

Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions src/game/server/score.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand All @@ -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");
}
Expand Down

0 comments on commit 78ba09b

Please sign in to comment.