From 65253b4f442971d68a185ec23453019c623051ae Mon Sep 17 00:00:00 2001 From: Kawe Mazidjatari <48657826+Mauler125@users.noreply.github.com> Date: Tue, 30 Jul 2024 12:24:09 +0200 Subject: [PATCH] UserCmd: clamp ucmd's on the client as well Prevent server -> client attacks, may one ever be found again after the server-side UserCmd exploit fix. --- src/game/shared/usercmd.cpp | 116 ++++++++++++++++++++---------------- src/game/shared/usercmd.h | 6 ++ 2 files changed, 72 insertions(+), 50 deletions(-) diff --git a/src/game/shared/usercmd.cpp b/src/game/shared/usercmd.cpp index d7914422..18ac6881 100644 --- a/src/game/shared/usercmd.cpp +++ b/src/game/shared/usercmd.cpp @@ -16,6 +16,64 @@ ConVar usercmd_frametime_min("usercmd_frametime_min", "0.002857", FCVAR_REPLICAT ConVar usercmd_dualwield_enable("usercmd_dualwield_enable", "0", FCVAR_REPLICATED | FCVAR_RELEASE, "Allows setting dual wield cycle slots, and activating multiple inventory weapons from UserCmd."); +//----------------------------------------------------------------------------- +// Purpose: Clamp untrusted usercommand. +// Input : *ucmd - +//----------------------------------------------------------------------------- +void ClampUserCmd(CUserCmd* ucmd) +{ + // Initialize the camera position as <0,0,0>, this should at least avoid + // crash and meme behaviors. + if (!ucmd->camerapos.IsValid()) + ucmd->camerapos.Init(); + + // Viewangles must be normalized; applying invalid angles on the client + // will result in undefined behavior. + ucmd->viewangles.Normalize(); + ucmd->pitchangles.Normalize(); + + // Some players abused a feature of the engine which allows you to perform + // custom weapon activities. After some research, it appears that only the + // 'ACT_VM_WEAPON_INSPECT' was supposed to work for standard games, all the + // other activities appears to be for development or testing, therefore, we + // should only allow 'ACT_VM_WEAPON_INSPECT' if cheats are disabled. + if (!sv_cheats->GetBool() && ucmd->weaponactivity != ACT_VM_WEAPON_INSPECT) + ucmd->weaponactivity = ACT_NONE; + + // On the client, the frame time must be within 'usercmd_frametime_min' + // and 'usercmd_frametime_max'. Testing revealed that speed hacking could + // be achieved by sending bogus frame times. Clamp the networked frame + // time to the exact values that the client should be using to make sure + // it couldn't be circumvented by busting out the client side clamps. + if (host_timescale->GetFloat() == 1.0f) + ucmd->frametime = Clamp(ucmd->frametime, + usercmd_frametime_min.GetFloat(), + usercmd_frametime_max.GetFloat()); + + // Checks are only required if cycleslot is valid; see 'CPlayer::UpdateWeaponSlots'. + if (ucmd->cycleslot != WEAPON_INVENTORY_SLOT_INVALID) + { + const bool dualWieldEnabled = usercmd_dualwield_enable.GetBool(); + + // Client could instruct the server to switch cycle slots for inventory + // weapons, however, the client could also cycle to the dual wield slots. + // dual wield weapons should be explicitly enabled to prevent exploitation. + if (!dualWieldEnabled && ucmd->cycleslot > WEAPON_INVENTORY_SLOT_ANTI_TITAN) + ucmd->cycleslot = WEAPON_INVENTORY_SLOT_ANTI_TITAN; + + // Array member 'CPlayer::m_selectedWeapons' is of size 2, values + // (up to 254) result in arbitrary memory reads on the server. + // Clamp the value to make sure it never reads out of bounds. Also, + // if dual wield is disabled on the server, reset the weapon index + // value as it would otherwise allow the client to enable several + // equipped weapons at the same time. + if (ucmd->weaponindex >= WEAPON_INVENTORY_SLOT_PRIMARY_1) + dualWieldEnabled + ? ucmd->weaponindex = WEAPON_INVENTORY_SLOT_PRIMARY_1 + : ucmd->weaponindex = WEAPON_INVENTORY_SLOT_PRIMARY_0; + } +} + //----------------------------------------------------------------------------- // Purpose: Read in a delta compressed usercommand. // Input : *buf - @@ -26,57 +84,14 @@ ConVar usercmd_dualwield_enable("usercmd_dualwield_enable", "0", FCVAR_REPLICATE int ReadUserCmd(bf_read* buf, CUserCmd* move, CUserCmd* from) { const int seed = v_ReadUserCmd(buf, move, from); + ClampUserCmd(move); - // Initialize the camera position as <0,0,0>, this should at least avoid - // crash and meme behaviors. - if (!move->camerapos.IsValid()) - move->camerapos.Init(); - - // Viewangles must be normalized; applying invalid angles on the client - // will result in undefined behavior, or a crash. - move->viewangles.Normalize(); - move->pitchangles.Normalize(); - - // Some players abused a feature of the engine which allows you to perform - // custom weapon activities. After some research, it appears that only the - // 'ACT_VM_WEAPON_INSPECT' was supposed to work for standard games, all the - // other activities appears to be for development or testing, therefore, we - // should only allow 'ACT_VM_WEAPON_INSPECT' if cheats are disabled. - if (!sv_cheats->GetBool() && move->weaponactivity != ACT_VM_WEAPON_INSPECT) - move->weaponactivity = ACT_NONE; - - // On the client, the frame time must be within 'usercmd_frametime_min' - // and 'usercmd_frametime_max'. Testing revealed that speed hacking could - // be achieved by sending bogus frame times. Clamp the networked frame - // time to the exact values that the client should be using to make sure - // it couldn't be circumvented by busting out the client side clamps. - if (host_timescale->GetFloat() == 1.0f) - move->frametime = Clamp(move->frametime, - usercmd_frametime_min.GetFloat(), - usercmd_frametime_max.GetFloat()); - - // Checks are only required if cycleslot is valid; see 'CPlayer::UpdateWeaponSlots'. - if (move->cycleslot != WEAPON_INVENTORY_SLOT_INVALID) - { - const bool dualWieldEnabled = usercmd_dualwield_enable.GetBool(); - - // Client could instruct the server to switch cycle slots for inventory - // weapons, however, the client could also cycle to the dual wield slots. - // dual wield weapons should be explicitly enabled to prevent exploitation. - if (!dualWieldEnabled && move->cycleslot > WEAPON_INVENTORY_SLOT_ANTI_TITAN) - move->cycleslot = WEAPON_INVENTORY_SLOT_ANTI_TITAN; - - // Array member 'CPlayer::m_selectedWeapons' is of size 2, values - // (up to 254) result in arbitrary memory reads on the server. - // Clamp the value to make sure it never reads out of bounds. Also, - // if dual wield is disabled on the server, reset the weapon index - // value as it would otherwise allow the client to enable several - // equipped weapons at the same time. - if (move->weaponindex >= WEAPON_INVENTORY_SLOT_PRIMARY_1) - dualWieldEnabled - ? move->weaponindex = WEAPON_INVENTORY_SLOT_PRIMARY_1 - : move->weaponindex = WEAPON_INVENTORY_SLOT_PRIMARY_0; - } + return seed; +} +int ReadUserCmdExtended(bf_read* buf, CUserCmdExtended* move, CUserCmdExtended* from) +{ + const int seed = v_ReadUserCmdExtended(buf, move, from); + ClampUserCmd(move); return seed; } @@ -85,4 +100,5 @@ int ReadUserCmd(bf_read* buf, CUserCmd* move, CUserCmd* from) void VUserCmd::Detour(const bool bAttach) const { DetourSetup(&v_ReadUserCmd, &ReadUserCmd, bAttach); + DetourSetup(&v_ReadUserCmdExtended, &ReadUserCmdExtended, bAttach); } diff --git a/src/game/shared/usercmd.h b/src/game/shared/usercmd.h index 3aba4c7a..4247a2d0 100644 --- a/src/game/shared/usercmd.h +++ b/src/game/shared/usercmd.h @@ -27,11 +27,13 @@ extern ConVar usercmd_dualwield_enable; // Forward declarations //------------------------------------------------------------------------------------- class CUserCmd; +class CUserCmdExtended; inline CUserCmd*(*CUserCmd__CUserCmd)(CUserCmd* pUserCmd); inline void(*CUserCmd__Reset)(CUserCmd* pUserCmd); inline CUserCmd*(*CUserCmd__Copy)(CUserCmd* pDest, CUserCmd* pSource); inline int(*v_ReadUserCmd)(bf_read* buf, CUserCmd* move, CUserCmd* from); +inline int(*v_ReadUserCmdExtended)(bf_read* buf, CUserCmdExtended* move, CUserCmdExtended* from); //------------------------------------------------------------------------------------- #pragma pack(push, 1) @@ -107,6 +109,7 @@ public: static_assert(sizeof(CUserCmd) == 0x1DC); +// The client-side input version of the UserCmd class class CUserCmdExtended : public CUserCmd { // todo: reverse engineer. @@ -114,6 +117,7 @@ class CUserCmdExtended : public CUserCmd }; int ReadUserCmd(bf_read* buf, CUserCmd* move, CUserCmd* from); +int ReadUserCmdExtended(bf_read* buf, CUserCmdExtended* move, CUserCmdExtended* from); /////////////////////////////////////////////////////////////////////////////// class VUserCmd : public IDetour @@ -124,6 +128,7 @@ class VUserCmd : public IDetour LogFunAdr("CUserCmd::Reset", CUserCmd__Reset); LogFunAdr("CUserCmd::Copy", CUserCmd__Copy); LogFunAdr("ReadUserCmd", v_ReadUserCmd); + LogFunAdr("ReadUserCmdExtended", v_ReadUserCmdExtended); } virtual void GetFun(void) const { @@ -131,6 +136,7 @@ class VUserCmd : public IDetour g_GameDll.FindPatternSIMD("E8 ?? ?? ?? ?? 48 8B DF 66 83 FE FF").FollowNearCallSelf().GetPtr(CUserCmd__Reset); g_GameDll.FindPatternSIMD("E8 ?? ?? ?? ?? 4C 8B 9B ?? ?? ?? ??").FollowNearCallSelf().GetPtr(CUserCmd__Copy); g_GameDll.FindPatternSIMD("E8 ?? ?? ?? ?? 4C 8B C6 48 81 C6 ?? ?? ?? ??").FollowNearCallSelf().GetPtr(v_ReadUserCmd); + g_GameDll.FindPatternSIMD("E8 ?? ?? ?? ?? 8B 4B ?? 4C 8D 35 ?? ?? ?? ?? 8B 53").FollowNearCallSelf().GetPtr(v_ReadUserCmdExtended); } virtual void GetVar(void) const { } virtual void GetCon(void) const { }