Skip to content

Commit b8ad945

Browse files
committed
Fix out-of-bounds reads during dummy connecting
The dummy is considered connected by the engine client (`IClient::DummyConnected` function returns `true`) after receiving `NETMSG_CON_READY`, which also causes `cl_dummy` to be set to `1`. However, the game client does not have the dummy's client ID until a snapshot is received, which means `m_aLocalIds[1]` (i.e. `m_aLocalIds[g_Config.m_ClDummy]`) is still set to `-1` (or an old value from when the dummy was last connected on the server) for a moment when connecting the dummy. This was noticeable by connecting the dummy and quickly opening the emoticon HUD, which would briefly render invalid skins for the eye emote preview based on out-of-bounds `CTeeRenderInfo`. In the other cases, the use of invalid local IDs could be confirmed by adding assertions. For example the hammer direction with `cl_dummy_hammer 1` was using out-of-bounds position data when connecting the dummy and immediately switching back to the main player. This is fixed by adding checks for all uses of the local IDs, same as for the `m_Snap.m_LocalClientId` variable. The local ID of the dummy, i.e. `m_aLocalIds[1]`, is now also reset when the dummy is disconnected. Previously, the value was only considered valid when also calling the `DummyConnected` function, which, however, was not enough due to the `m_aLocalIds[1]` value not being updated at the same time as the `DummyConnected` function. By resetting `m_aLocalIds[1]` to `-1` when the dummy is disconnected, the additional calls to the `DummyConnected` function are unnecessary.
1 parent 0e9b9a8 commit b8ad945

File tree

6 files changed

+20
-18
lines changed

6 files changed

+20
-18
lines changed

src/game/client/components/chat.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -768,12 +768,12 @@ void CChat::AddLine(int ClientId, int Team, const char *pLine)
768768
// check for highlighted name
769769
if(Client()->State() != IClient::STATE_DEMOPLAYBACK)
770770
{
771-
if(ClientId >= 0 && ClientId != m_pClient->m_aLocalIds[0] && (!m_pClient->Client()->DummyConnected() || ClientId != m_pClient->m_aLocalIds[1]))
771+
if(ClientId >= 0 && ClientId != m_pClient->m_aLocalIds[0] && ClientId != m_pClient->m_aLocalIds[1])
772772
{
773-
// main character
774-
Highlighted |= LineShouldHighlight(pLine, m_pClient->m_aClients[m_pClient->m_aLocalIds[0]].m_aName);
775-
// dummy
776-
Highlighted |= m_pClient->Client()->DummyConnected() && LineShouldHighlight(pLine, m_pClient->m_aClients[m_pClient->m_aLocalIds[1]].m_aName);
773+
for(int LocalId : m_pClient->m_aLocalIds)
774+
{
775+
Highlighted |= LocalId >= 0 && LineShouldHighlight(pLine, m_pClient->m_aClients[LocalId].m_aName);
776+
}
777777
}
778778
}
779779
else

src/game/client/components/emoticon.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ void CEmoticon::OnRender()
167167
}
168168
Graphics()->WrapNormal();
169169

170-
if(GameClient()->m_GameInfo.m_AllowEyeWheel && g_Config.m_ClEyeWheel)
170+
if(GameClient()->m_GameInfo.m_AllowEyeWheel && g_Config.m_ClEyeWheel && m_pClient->m_aLocalIds[g_Config.m_ClDummy] >= 0)
171171
{
172172
Graphics()->TextureClear();
173173
Graphics()->QuadsBegin();

src/game/client/components/nameplates.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ void CNamePlates::RenderNamePlateGame(vec2 Position, const CNetObj_PlayerInfo *p
288288
if(Data.m_ShowDirection)
289289
{
290290
if(Client()->State() != IClient::STATE_DEMOPLAYBACK &&
291-
Client()->DummyConnected() && pPlayerInfo->m_ClientId == m_pClient->m_aLocalIds[!g_Config.m_ClDummy])
291+
pPlayerInfo->m_ClientId == m_pClient->m_aLocalIds[!g_Config.m_ClDummy])
292292
{
293293
const auto &InputData = m_pClient->m_Controls.m_aInputData[!g_Config.m_ClDummy];
294294
Data.m_DirLeft = InputData.m_Direction == -1;

src/game/client/components/players.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -765,7 +765,7 @@ void CPlayers::RenderPlayer(
765765
Graphics()->QuadsSetRotation(0);
766766
}
767767

768-
if(g_Config.m_ClAfkEmote && m_pClient->m_aClients[ClientId].m_Afk && !(Client()->DummyConnected() && ClientId == m_pClient->m_aLocalIds[!g_Config.m_ClDummy]))
768+
if(g_Config.m_ClAfkEmote && m_pClient->m_aClients[ClientId].m_Afk && ClientId != m_pClient->m_aLocalIds[!g_Config.m_ClDummy])
769769
{
770770
int CurEmoticon = (SPRITE_ZZZ - SPRITE_OOP);
771771
Graphics()->TextureSet(GameClient()->m_EmoticonsSkin.m_aSpriteEmoticons[CurEmoticon]);

src/game/client/components/scoreboard.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -208,10 +208,9 @@ void CScoreboard::RenderSpectators(CUIRect Spectators)
208208

209209
{
210210
const char *pClanName = GameClient()->m_aClients[pInfo->m_ClientId].m_aClan;
211-
212211
if(pClanName[0] != '\0')
213212
{
214-
if(str_comp(pClanName, GameClient()->m_aClients[GameClient()->m_aLocalIds[g_Config.m_ClDummy]].m_aClan) == 0)
213+
if(GameClient()->m_aLocalIds[g_Config.m_ClDummy] >= 0 && str_comp(pClanName, GameClient()->m_aClients[GameClient()->m_aLocalIds[g_Config.m_ClDummy]].m_aClan) == 0)
215214
{
216215
TextRender()->TextColor(color_cast<ColorRGBA>(ColorHSLA(g_Config.m_ClSameClanColor)));
217216
}
@@ -556,7 +555,7 @@ void CScoreboard::RenderScoreboard(CUIRect Scoreboard, int Team, int CountStart,
556555

557556
// clan
558557
{
559-
if(str_comp(ClientData.m_aClan, GameClient()->m_aClients[GameClient()->m_aLocalIds[g_Config.m_ClDummy]].m_aClan) == 0)
558+
if(GameClient()->m_aLocalIds[g_Config.m_ClDummy] >= 0 && str_comp(ClientData.m_aClan, GameClient()->m_aClients[GameClient()->m_aLocalIds[g_Config.m_ClDummy]].m_aClan) == 0)
560559
{
561560
TextRender()->TextColor(color_cast<ColorRGBA>(ColorHSLA(g_Config.m_ClSameClanColor)));
562561
}

src/game/client/gameclient.cpp

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -504,6 +504,10 @@ int CGameClient::OnSnapInput(int *pData, bool Dummy, bool Force)
504504
{
505505
return m_Controls.SnapInput(pData);
506506
}
507+
if(m_aLocalIds[!g_Config.m_ClDummy] < 0)
508+
{
509+
return 0;
510+
}
507511

508512
if(!g_Config.m_ClDummyHammer)
509513
{
@@ -537,11 +541,9 @@ int CGameClient::OnSnapInput(int *pData, bool Dummy, bool Force)
537541
m_DummyInput.m_WantedWeapon = WEAPON_HAMMER + 1;
538542
}
539543

540-
vec2 MainPos = m_LocalCharacterPos;
541-
vec2 DummyPos = m_aClients[m_aLocalIds[!g_Config.m_ClDummy]].m_Predicted.m_Pos;
542-
vec2 Dir = MainPos - DummyPos;
543-
m_HammerInput.m_TargetX = (int)(Dir.x);
544-
m_HammerInput.m_TargetY = (int)(Dir.y);
544+
const vec2 Dir = m_LocalCharacterPos - m_aClients[m_aLocalIds[!g_Config.m_ClDummy]].m_Predicted.m_Pos;
545+
m_HammerInput.m_TargetX = (int)Dir.x;
546+
m_HammerInput.m_TargetY = (int)Dir.y;
545547

546548
mem_copy(pData, &m_HammerInput, sizeof(m_HammerInput));
547549
return sizeof(m_HammerInput);
@@ -809,7 +811,7 @@ void CGameClient::OnRender()
809811
g_Config.m_ClDummy = 0;
810812

811813
// resend player and dummy info if it was filtered by server
812-
if(Client()->State() == IClient::STATE_ONLINE && !m_Menus.IsActive() && WasNewTick)
814+
if(m_aLocalIds[0] >= 0 && Client()->State() == IClient::STATE_ONLINE && !m_Menus.IsActive() && WasNewTick)
813815
{
814816
if(m_aCheckInfo[0] == 0)
815817
{
@@ -841,7 +843,7 @@ void CGameClient::OnRender()
841843
m_aCheckInfo[0] -= minimum(Client()->GameTick(0) - Client()->PrevGameTick(0), m_aCheckInfo[0]);
842844
}
843845

844-
if(Client()->DummyConnected())
846+
if(m_aLocalIds[1] >= 0)
845847
{
846848
if(m_aCheckInfo[1] == 0)
847849
{
@@ -878,6 +880,7 @@ void CGameClient::OnRender()
878880

879881
void CGameClient::OnDummyDisconnect()
880882
{
883+
m_aLocalIds[1] = -1;
881884
m_aDDRaceMsgSent[1] = false;
882885
m_aShowOthers[1] = SHOW_OTHERS_NOT_SET;
883886
m_aLastNewPredictedTick[1] = -1;

0 commit comments

Comments
 (0)