Fix infinite memalloc exploit in NET_SetConVar

Reported by Wanderer. Don't allow the creation of new ConVar entries in CClient::m_ConVars after the initial creation.
This commit is contained in:
Kawe Mazidjatari 2023-09-16 16:18:18 +02:00
parent ee3560e722
commit 00e79268d7
4 changed files with 113 additions and 29 deletions

View File

@ -7,6 +7,7 @@
#pragma once
#include "tier1/bitbuf.h"
#include "common/qlimits.h"
#include "public/inetchannel.h"
#include "public/inetmessage.h"
#include "public/inetmsghandler.h"
@ -440,6 +441,19 @@ struct NET_StringCmd : CNetMessage
char buffer[1024];
};
class NET_SetConVar : public CNetMessage
{
public:
typedef struct cvar_s
{
char name[MAX_OSPATH];
char value[MAX_OSPATH];
} cvar_t;
CUtlVector<cvar_t> m_ConVars;
};
///////////////////////////////////////////////////////////////////////////////////////
// This message is subclassed by 'SVC_CmdKeyValues' and 'CLC_CmdKeyValues'
class Base_CmdKeyValues : public CNetMessage

View File

@ -49,6 +49,9 @@ void CClient::VClear(CClient* pClient)
//---------------------------------------------------------------------------------
bool CClient::Connect(const char* szName, void* pNetChannel, bool bFakePlayer, void* a5, char* szMessage, int nMessageSize)
{
#ifndef CLIENT_DLL
g_ServerPlayer[GetUserID()].Reset(); // Reset ServerPlayer slot.
#endif // !CLIENT_DLL
return v_CClient_Connect(this, szName, pNetChannel, bFakePlayer, a5, szMessage, nMessageSize);
}
@ -65,11 +68,7 @@ bool CClient::Connect(const char* szName, void* pNetChannel, bool bFakePlayer, v
//---------------------------------------------------------------------------------
bool CClient::VConnect(CClient* pClient, const char* szName, void* pNetChannel, bool bFakePlayer, void* a5, char* szMessage, int nMessageSize)
{
bool bResult = v_CClient_Connect(pClient, szName, pNetChannel, bFakePlayer, a5, szMessage, nMessageSize);
#ifndef CLIENT_DLL
g_ServerPlayer[pClient->GetUserID()].Reset(); // Reset ServerPlayer slot.
#endif // !CLIENT_DLL
return bResult;
return pClient->Connect(szName, pNetChannel, bFakePlayer, a5, szMessage, nMessageSize);;
}
//---------------------------------------------------------------------------------
@ -148,6 +147,19 @@ void* CClient::VSendSnapshot(CClient* pClient, CClientFrame* pFrame, int nTick,
return v_CClient_SendSnapshot(pClient, pFrame, nTick, nTickAck);
}
//---------------------------------------------------------------------------------
// Purpose: internal hook to 'CClient::SendNetMsgEx'
// Input : *pClient -
// *pMsg -
// bLocal -
// bForceReliable -
// bVoice -
//---------------------------------------------------------------------------------
bool CClient::VSendNetMsgEx(CClient* pClient, CNetMessage* pMsg, char bLocal, bool bForceReliable, bool bVoice)
{
return pClient->SendNetMsgEx(pMsg, bLocal, bForceReliable, bVoice);
}
//---------------------------------------------------------------------------------
// Purpose: some versions of the binary have an optimization that shifts the 'this'
// pointer of the CClient structure by 8 bytes to avoid having to cache the vftable
@ -241,16 +253,60 @@ bool CClient::VProcessStringCmd(CClient* pClient, NET_StringCmd* pMsg)
}
//---------------------------------------------------------------------------------
// Purpose: internal hook to 'CClient::SendNetMsgEx'
// Input : *pClient -
// Purpose: process set convar
// Input : *pClient - (ADJ)
// *pMsg -
// bLocal -
// bForceReliable -
// bVoice -
// Output :
//---------------------------------------------------------------------------------
bool CClient::VSendNetMsgEx(CClient* pClient, CNetMessage* pMsg, char bLocal, bool bForceReliable, bool bVoice)
bool CClient::VProcessSetConVar(CClient* pClient, NET_SetConVar* pMsg)
{
return pClient->SendNetMsgEx(pMsg, bLocal, bForceReliable, bVoice);
CClient* pAdj = AdjustShiftedThisPointer(pClient);
ServerPlayer_t* pSlot = &g_ServerPlayer[pAdj->GetUserID()];
// This loop never exceeds 255 iterations, NET_SetConVar::ReadFromBuffer(...)
// reads and inserts up to 255 entries in the vector (reads a byte for size).
FOR_EACH_VEC(pMsg->m_ConVars, i)
{
const NET_SetConVar::cvar_t& entry = pMsg->m_ConVars[i];
const char* const name = entry.name;
const char* const value = entry.value;
// Discard any ConVar change request if it contains funky characters.
bool bFunky = false;
for (const char* s = name; *s != '\0'; ++s)
{
if (!isalnum(*s) && *s != '_')
{
bFunky = true;
break;
}
}
if (bFunky)
{
DevWarning(eDLL_T::SERVER, "Ignoring ConVar change request for variable '%s' from client '%s'; invalid characters in the variable name\n",
name, pAdj->GetClientName());
continue;
}
// The initial set of ConVars must contain all client ConVars that are
// flagged UserInfo. This is a simple fix to exploits that send bogus
// data later, and catches bugs, such as new UserInfo ConVars appearing
// later, which shouldn't happen.
if (pSlot->m_bInitialConVarsSet && !pAdj->m_ConVars->FindKey(name))
{
DevWarning(eDLL_T::SERVER, "UserInfo update from \"%s\" ignored: %s = %s\n",
pAdj->GetClientName(), name, value);
continue;
}
pAdj->m_ConVars->SetString(name, value);
DevMsg(eDLL_T::SERVER, "UserInfo update from \"%s\": %s = %s\n", pAdj->GetClientName(), name, value);
}
pSlot->m_bInitialConVarsSet = true;
pAdj->m_bConVarsChanged = true;
return true;
}
void VClient::Attach(void) const
@ -259,9 +315,11 @@ void VClient::Attach(void) const
DetourAttach((LPVOID*)&v_CClient_Clear, &CClient::VClear);
DetourAttach((LPVOID*)&v_CClient_Connect, &CClient::VConnect);
DetourAttach((LPVOID*)&v_CClient_ActivatePlayer, &CClient::VActivatePlayer);
DetourAttach((LPVOID*)&v_CClient_ProcessStringCmd, &CClient::VProcessStringCmd);
DetourAttach((LPVOID*)&v_CClient_SendNetMsgEx, &CClient::VSendNetMsgEx);
//DetourAttach((LPVOID*)&p_CClient_SendSnapshot, &CClient::VSendSnapshot);
DetourAttach((LPVOID*)&v_CClient_ProcessStringCmd, &CClient::VProcessStringCmd);
DetourAttach((LPVOID*)&v_CClient_ProcessSetConVar, &CClient::VProcessSetConVar);
#endif // !CLIENT_DLL
}
void VClient::Detach(void) const
@ -270,8 +328,10 @@ void VClient::Detach(void) const
DetourDetach((LPVOID*)&v_CClient_Clear, &CClient::VClear);
DetourDetach((LPVOID*)&v_CClient_Connect, &CClient::VConnect);
DetourDetach((LPVOID*)&v_CClient_ActivatePlayer, &CClient::VActivatePlayer);
DetourDetach((LPVOID*)&v_CClient_ProcessStringCmd, &CClient::VProcessStringCmd);
DetourDetach((LPVOID*)&v_CClient_SendNetMsgEx, &CClient::VSendNetMsgEx);
//DetourDetach((LPVOID*)&p_CClient_SendSnapshot, &CClient::VSendSnapshot);
DetourDetach((LPVOID*)&v_CClient_ProcessStringCmd, &CClient::VProcessStringCmd);
DetourDetach((LPVOID*)&v_CClient_ProcessSetConVar, &CClient::VProcessSetConVar);
#endif // !CLIENT_DLL
}

View File

@ -101,10 +101,12 @@ public:
public: // Hook statics:
static void VClear(CClient* pClient);
static void VActivatePlayer(CClient* pClient);
static bool VProcessStringCmd(CClient* pClient, NET_StringCmd* pMsg);
static void* VSendSnapshot(CClient* pClient, CClientFrame* pFrame, int nTick, int nTickAck);
static bool VSendNetMsgEx(CClient* pClient, CNetMessage* pMsg, char bLocal, bool bForceReliable, bool bVoice);
static bool VProcessStringCmd(CClient* pClient, NET_StringCmd* pMsg);
static bool VProcessSetConVar(CClient* pClient, NET_SetConVar* pMsg);
private:
// Stub reimplementation to avoid the 'no overrider' compiler errors in the
// CServer class (contains a static array of MAX_PLAYERS of this class).
@ -150,7 +152,7 @@ private:
char pad_0016[59];
int64_t m_iTeamNum;
KeyValues* m_ConVars;
bool m_bInitialConVarsSet;
bool m_bConVarsChanged;
bool m_bSendServerInfo;
bool m_bSendSignonData;
bool m_bFullStateAchieved;
@ -213,9 +215,6 @@ inline void(*v_CClient_Clear)(CClient* pClient);
inline CMemory p_CClient_ActivatePlayer;
inline void(*v_CClient_ActivatePlayer)(CClient* pClient);
inline CMemory p_CClient_ProcessStringCmd;
inline bool(*v_CClient_ProcessStringCmd)(CClient* pClient, NET_StringCmd* pMsg);
inline CMemory p_CClient_SetSignonState;
inline bool(*v_CClient_SetSignonState)(CClient* pClient, SIGNONSTATE signon);
@ -225,6 +224,12 @@ inline bool(*v_CClient_SendNetMsgEx)(CClient* pClient, CNetMessage* pMsg, bool b
inline CMemory p_CClient_SendSnapshot;
inline void*(*v_CClient_SendSnapshot)(CClient* pClient, CClientFrame* pFrame, int nTick, int nTickAck);
inline CMemory p_CClient_ProcessStringCmd;
inline bool(*v_CClient_ProcessStringCmd)(CClient* pClient, NET_StringCmd* pMsg);
inline CMemory p_CClient_ProcessSetConVar;
inline bool(*v_CClient_ProcessSetConVar)(CClient* pClient, NET_SetConVar* pMsg);
///////////////////////////////////////////////////////////////////////////////
class VClient : public IDetour
{
@ -234,10 +239,11 @@ class VClient : public IDetour
LogFunAdr("CClient::Disconnect", p_CClient_Disconnect.GetPtr());
LogFunAdr("CClient::Clear", p_CClient_Clear.GetPtr());
LogFunAdr("CClient::ActivatePlayer", p_CClient_ActivatePlayer.GetPtr());
LogFunAdr("CClient::ProcessStringCmd", p_CClient_ProcessStringCmd.GetPtr());
LogFunAdr("CClient::SetSignonState", p_CClient_SetSignonState.GetPtr());
LogFunAdr("CClient::SendNetMsgEx", p_CClient_SendNetMsgEx.GetPtr());
LogFunAdr("CClient::SendSnapshot", p_CClient_SendSnapshot.GetPtr());
LogFunAdr("CClient::ProcessStringCmd", p_CClient_ProcessStringCmd.GetPtr());
LogFunAdr("CClient::ProcessSetConVar", p_CClient_ProcessSetConVar.GetPtr());
}
virtual void GetFun(void) const
{
@ -250,25 +256,29 @@ class VClient : public IDetour
p_CClient_Clear = g_GameDll.FindPatternSIMD("40 53 41 56 41 57 48 83 EC 20 48 8B D9 48 89 74");
#if defined (GAMEDLL_S0) || defined (GAMEDLL_S1)
p_CClient_ActivatePlayer = g_GameDll.FindPatternSIMD("40 53 57 41 57 48 83 EC 30 8B 81 ?? ?? ?? ??");
p_CClient_ProcessStringCmd = g_GameDll.FindPatternSIMD("48 89 5C 24 ?? 55 48 81 EC ?? ?? ?? ?? 49 8B D8");
p_CClient_SendNetMsg = g_GameDll.FindPatternSIMD("48 89 5C 24 ?? 48 89 6C 24 ?? 48 89 74 24 ?? 57 41 56 41 57 48 83 EC 30 48 8B 05 ?? ?? ?? ?? 45 0F B6 F1");
p_CClient_SendSnapshot = g_GameDll.FindPatternSIMD("44 89 44 24 ?? 48 89 4C 24 ?? 55 53 56 57 41 55");
p_CClient_ProcessStringCmd = g_GameDll.FindPatternSIMD("48 89 5C 24 ?? 55 48 81 EC ?? ?? ?? ?? 49 8B D8");
#elif defined (GAMEDLL_S2) || defined (GAMEDLL_S3)
p_CClient_ActivatePlayer = g_GameDll.FindPatternSIMD("40 53 48 83 EC 20 8B 81 B0 03 ?? ?? 48 8B D9 C6");
p_CClient_ProcessStringCmd = g_GameDll.FindPatternSIMD("48 89 6C 24 ?? 57 48 81 EC ?? ?? ?? ?? 48 8B 7A 20");
p_CClient_SendNetMsgEx = g_GameDll.FindPatternSIMD("40 53 55 56 57 41 56 48 83 EC 40 48 8B 05 ?? ?? ?? ??");
p_CClient_SendSnapshot = g_GameDll.FindPatternSIMD("48 89 5C 24 ?? 55 56 41 55 41 56 41 57 48 8D 6C 24 ??");
p_CClient_ProcessStringCmd = g_GameDll.FindPatternSIMD("48 89 6C 24 ?? 57 48 81 EC ?? ?? ?? ?? 48 8B 7A 20");
#endif // !GAMEDLL_S0 || !GAMEDLL_S1
p_CClient_ProcessSetConVar = g_GameDll.FindPatternSIMD("48 83 EC 28 48 83 C2 20");
p_CClient_SetSignonState = g_GameDll.FindPatternSIMD("48 8B C4 48 89 58 10 48 89 70 18 57 48 81 EC ?? ?? ?? ?? 0F 29 70 E8 8B F2");
v_CClient_Connect = p_CClient_Connect.RCast<bool (*)(CClient*, const char*, void*, bool, void*, char*, int)>();
v_CClient_Disconnect = p_CClient_Disconnect.RCast<bool (*)(CClient*, const Reputation_t, const char*, ...)>();
v_CClient_Clear = p_CClient_Clear.RCast<void (*)(CClient*)>();
v_CClient_ActivatePlayer = p_CClient_ActivatePlayer.RCast<void (*)(CClient* pClient)>();
v_CClient_ProcessStringCmd = p_CClient_ProcessStringCmd.RCast<bool (*)(CClient*, NET_StringCmd*)>();
v_CClient_SetSignonState = p_CClient_SetSignonState.RCast<bool (*)(CClient*, SIGNONSTATE)>();
v_CClient_SendNetMsgEx = p_CClient_SendNetMsgEx.RCast<bool (*)(CClient*, CNetMessage*, bool, bool, bool)>();
v_CClient_SendSnapshot = p_CClient_SendSnapshot.RCast<void* (*)(CClient*, CClientFrame*, int, int)>();
v_CClient_ProcessStringCmd = p_CClient_ProcessStringCmd.RCast<bool (*)(CClient*, NET_StringCmd*)>();
v_CClient_ProcessSetConVar = p_CClient_ProcessSetConVar.RCast<bool (*)(CClient*, NET_SetConVar*)>();
}
virtual void GetVar(void) const { }
virtual void GetCon(void) const { }

View File

@ -13,23 +13,23 @@ inline bool* m_bIsDedicated = nullptr;
struct ServerPlayer_t
{
ServerPlayer_t(void)
: m_flCurrentNetProcessTime(0.0)
, m_flLastNetProcessTime(0.0)
, m_flStringCommandQuotaTimeStart(0.0)
, m_nStringCommandQuotaCount(0)
{}
{
Reset();
}
inline void Reset(void)
{
m_flCurrentNetProcessTime = 0.0;
m_flLastNetProcessTime = 0.0;
m_flStringCommandQuotaTimeStart = 0.0;
m_nStringCommandQuotaCount = 0;
m_nStringCommandQuotaCount = NULL;
m_bInitialConVarsSet = false;
}
double m_flCurrentNetProcessTime;
double m_flLastNetProcessTime;
double m_flStringCommandQuotaTimeStart;
int m_nStringCommandQuotaCount;
bool m_bInitialConVarsSet;
};
extern ServerPlayer_t g_ServerPlayer[MAX_PLAYERS];