From 619bbd161c3dddbfdea10c9320da013639c4552d Mon Sep 17 00:00:00 2001 From: Kawe Mazidjatari <48657826+Mauler125@users.noreply.github.com> Date: Tue, 30 May 2023 23:01:46 +0200 Subject: [PATCH] Fix stringcmd replay exploit This commit fixes an engine bug where netmessages are getting copied into the replay buffer, while these messages should never be replayed. The engine performs an internal check on 'CNetMessage::m_nGroup', and if its NOT 2, the message is getting copied into the replay buffer. All messages returning false in 'ShouldReplayMessage' are not getting copied into the replay buffer anymore. This exploit has been used in the past to route clients that were watching a replay to an arbitrary server, which essentially forms an info leak as the client attempts to connect to the arbitrary server on its own. The exploit also allows for some form of remote code execution, depending on if the client was launched in developer mode or not. --- r5dev/common/netmessages.cpp | 28 ++++++++++++++++++++++++++++ r5dev/common/netmessages.h | 9 +++++++++ r5dev/engine/client/client.cpp | 26 +++++++++++++++++++++++--- r5dev/engine/client/client.h | 15 +++++++++------ r5dev/engine/server/sv_main.cpp | 2 +- 5 files changed, 70 insertions(+), 10 deletions(-) diff --git a/r5dev/common/netmessages.cpp b/r5dev/common/netmessages.cpp index c18e6da2..f89c3313 100644 --- a/r5dev/common/netmessages.cpp +++ b/r5dev/common/netmessages.cpp @@ -86,6 +86,34 @@ bool Base_CmdKeyValues::WriteToBufferImpl(Base_CmdKeyValues* thisptr, bf_write* return Base_CmdKeyValues_WriteToBuffer(thisptr, buffer); } +/////////////////////////////////////////////////////////////////////////////////// +// determine whether or not the message should be copied into the replay buffer, +// regardless of the 'CNetMessage::m_Group' type. +/////////////////////////////////////////////////////////////////////////////////// +bool ShouldReplayMessage(CNetMessage* msg) +{ + switch (msg->GetType()) + { + // String commands can be abused in a way they get executed + // on the client that is watching a replay. This happens as + // the server copies the message into the replay buffer from + // the client that initially submitted it. Its group type is + // 'None', so call this to determine whether or not to set + // the group type to 'NoReplay'. This exploit has been used + // to connect clients to an arbitrary server during replay. + case NetMessageType::net_StringCmd: + // Print and user messages sometimes make their way to the + // client that is watching a replay, while it should only + // be broadcasted to the target client. This happens for the + // same reason as the 'net_StringCmd' above. + case NetMessageType::svc_Print: + case NetMessageType::svc_UserMessage: + return false; + default: + return true; + } +} + void V_NetMessages::Attach() const { #if !defined(DEDICATED) diff --git a/r5dev/common/netmessages.h b/r5dev/common/netmessages.h index 27fe464f..0a44a9d3 100644 --- a/r5dev/common/netmessages.h +++ b/r5dev/common/netmessages.h @@ -144,6 +144,14 @@ enum NetMessageType clc_GamepadMsg = 65, }; +//------------------------------------------------------------------------- +// Enumeration of netmessage groups +//------------------------------------------------------------------------- +enum NetMessageGroup +{ + NoReplay = 2, +}; + /////////////////////////////////////////////////////////////////////////////////////// class CNetMessage : public INetMessage { @@ -370,6 +378,7 @@ public: bf_write m_DataOut; }; +bool ShouldReplayMessage(CNetMessage* msg); /////////////////////////////////////////////////////////////////////////////// class V_NetMessages : public IDetour diff --git a/r5dev/engine/client/client.cpp b/r5dev/engine/client/client.cpp index 1fa308ae..25a706f2 100644 --- a/r5dev/engine/client/client.cpp +++ b/r5dev/engine/client/client.cpp @@ -332,15 +332,22 @@ void CClient::VActivatePlayer(CClient* pClient) } //--------------------------------------------------------------------------------- -// Purpose: send a net message +// Purpose: send a net message with replay. +// set 'CNetMessage::m_nGroup' to 'NoReplay' to disable replay. // Input : *pMsg - // bLocal - // bForceReliable - // bVoice - //--------------------------------------------------------------------------------- -bool CClient::SendNetMsg(CNetMessage* pMsg, char bLocal, bool bForceReliable, bool bVoice) +bool CClient::SendNetMsgEx(CNetMessage* pMsg, char bLocal, bool bForceReliable, bool bVoice) { - return v_CClient_SendNetMsg(this, pMsg, bLocal, bForceReliable, bVoice); + if (!ShouldReplayMessage(pMsg)) + { + // Don't copy the message into the replay buffer. + pMsg->m_nGroup = NetMessageGroup::NoReplay; + } + + return v_CClient_SendNetMsgEx(this, pMsg, bLocal, bForceReliable, bVoice); } //--------------------------------------------------------------------------------- @@ -406,5 +413,18 @@ bool CClient::VProcessStringCmd(CClient* pClient, NET_StringCmd* pMsg) return v_CClient_ProcessStringCmd(pClient, pMsg); } +//--------------------------------------------------------------------------------- +// 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); +} + /////////////////////////////////////////////////////////////////////////////// CClient* g_pClient = nullptr; \ No newline at end of file diff --git a/r5dev/engine/client/client.h b/r5dev/engine/client/client.h index 0b29f431..b0f09884 100644 --- a/r5dev/engine/client/client.h +++ b/r5dev/engine/client/client.h @@ -87,7 +87,7 @@ public: bool IsFakeClient(void) const; bool IsHumanPlayer(void) const; - bool SendNetMsg(CNetMessage* pMsg, char bLocal, bool bForceReliable, bool bVoice); + bool SendNetMsgEx(CNetMessage* pMsg, char bLocal, bool bForceReliable, bool bVoice); bool Connect(const char* szName, void* pNetChannel, bool bFakePlayer, void* a5, char* szMessage, int nMessageSize); void Disconnect(const Reputation_t nRepLvl, const char* szReason, ...); static bool VConnect(CClient* pClient, const char* szName, void* pNetChannel, bool bFakePlayer, void* a5, char* szMessage, int nMessageSize); @@ -98,6 +98,7 @@ public: // Hook statics: 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); private: int m_nUserID; //0x0010 @@ -165,8 +166,8 @@ inline auto v_CClient_ProcessStringCmd = p_CClient_ProcessStringCmd.RCast(); -inline CMemory p_CClient_SendNetMsg; -inline auto v_CClient_SendNetMsg = p_CClient_SendNetMsg.RCast(); +inline CMemory p_CClient_SendNetMsgEx; +inline auto v_CClient_SendNetMsgEx = p_CClient_SendNetMsgEx.RCast(); inline CMemory p_CClient_SendSnapshot; inline auto v_CClient_SendSnapshot = p_CClient_SendSnapshot.RCast(); @@ -182,7 +183,7 @@ class VClient : public IDetour LogFunAdr("CClient::ActivatePlayer", p_CClient_ActivatePlayer.GetPtr()); LogFunAdr("CClient::ProcessStringCmd", p_CClient_ProcessStringCmd.GetPtr()); LogFunAdr("CClient::SetSignonState", p_CClient_SetSignonState.GetPtr()); - LogFunAdr("CClient::SendNetMsg", p_CClient_SendNetMsg.GetPtr()); + LogFunAdr("CClient::SendNetMsgEx", p_CClient_SendNetMsgEx.GetPtr()); LogFunAdr("CClient::SendSnapshot", p_CClient_SendSnapshot.GetPtr()); LogVarAdr("g_Client[128]", reinterpret_cast(g_pClient)); } @@ -203,7 +204,7 @@ class VClient : public IDetour #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_SendNetMsg = g_GameDll.FindPatternSIMD("40 53 55 56 57 41 56 48 83 EC 40 48 8B 05 ?? ?? ?? ??"); + 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 ??"); #endif // !GAMEDLL_S0 || !GAMEDLL_S1 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"); @@ -214,7 +215,7 @@ class VClient : public IDetour v_CClient_ActivatePlayer = p_CClient_ActivatePlayer.RCast(); v_CClient_ProcessStringCmd = p_CClient_ProcessStringCmd.RCast(); v_CClient_SetSignonState = p_CClient_SetSignonState.RCast(); - v_CClient_SendNetMsg = p_CClient_SendNetMsg.RCast(); + v_CClient_SendNetMsgEx = p_CClient_SendNetMsgEx.RCast(); v_CClient_SendSnapshot = p_CClient_SendSnapshot.RCast(); } virtual void GetVar(void) const @@ -229,6 +230,7 @@ class VClient : public IDetour 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); } virtual void Detach(void) const @@ -237,6 +239,7 @@ class VClient : public IDetour 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); } }; diff --git a/r5dev/engine/server/sv_main.cpp b/r5dev/engine/server/sv_main.cpp index ba3fe0d4..ba7f4e97 100644 --- a/r5dev/engine/server/sv_main.cpp +++ b/r5dev/engine/server/sv_main.cpp @@ -184,7 +184,7 @@ void SV_BroadcastVoiceData(CClient* cl, int nBytes, char* data) // if voice stream has enough space for new data if (pNetChan->GetStreamVoice().GetNumBitsLeft() >= 8 * nBytes + 96) - pClient->SendNetMsg(&voiceData, false, false, true); + pClient->SendNetMsgEx(&voiceData, false, false, true); } }