Engine: fix numerous exploitable bugs in voice protocol

- Fixed stack smash in CClient:ProcessVoiceData (oob read on bitbuf).
- Fixed stack smash in CClient:ProcessDurangoVoiceData (oob read on bitbuf).
- Fixed ability to bypass team check on Durango voice packets if forced as reliable from the sender (client).
- Incorporated the following missing checks in the durango version of voice broadcasting:
  - Enforce chat between multiple teams using cvar 'sv_alltalk'.
  - Ability to also disable Durango voice data with cvar 'sv_voiceenable'.
  - Ability to echo voice with Durango voice data using cvar 'sv_voiceEcho'.

src/common/global.cpp

@@ -63,6 +63,8 @@ ConVar* eula_version_accepted              = nullptr;
 
 ConVar* language_cvar                      = nullptr;
 
+ConVar* voice_noxplat                      = nullptr;
+
 //-----------------------------------------------------------------------------
 // SERVER                                                                     |
 #ifndef CLIENT_DLL
@@ -152,7 +154,8 @@ void ConVar_InitShipped(void)
 	eula_version                     = g_pCVar->FindVar("eula_version");
 	eula_version_accepted            = g_pCVar->FindVar("eula_version_accepted");
 
-	language_cvar = g_pCVar->FindVar("language");
+	language_cvar                    = g_pCVar->FindVar("language");
+	voice_noxplat                    = g_pCVar->FindVar("voice_noxplat");
 #ifndef DEDICATED
 	cl_updaterate_mp                 = g_pCVar->FindVar("cl_updaterate_mp");
 	cl_threaded_bone_setup           = g_pCVar->FindVar("cl_threaded_bone_setup");

src/common/global.h

@@ -50,6 +50,8 @@ extern ConVar* eula_version_accepted;
 
 extern ConVar* language_cvar;
 
+extern ConVar* voice_noxplat;
+
 //-------------------------------------------------------------------------
 // SERVER                                                                 |
 #ifndef CLIENT_DLL

src/common/netmessages.h

@@ -372,6 +372,70 @@ public:
 	void* m_DataOut;
 };
 
+struct SVC_DurangoVoiceData : public CNetMessage
+{
+public:
+	SVC_DurangoVoiceData() = default;
+	SVC_DurangoVoiceData(int senderClient, int nBytes, char* data, int unknown, bool useUnreliableStream)
+	{
+		void** pVFTable = reinterpret_cast<void**>(this);
+		*pVFTable = g_pSVC_VoiceData_VFTable;
+
+		m_bReliable = false;
+		m_NetChannel = nullptr;
+
+		m_nFromClient = senderClient;
+		m_nLength = nBytes; // length in bits
+
+		m_unknown = unknown;
+		m_useVoiceStream = useUnreliableStream;
+
+		m_DataOut = data;
+
+		m_nGroup = 2; // must be set to 2 to avoid being copied into replay buffer
+	};
+
+	virtual	~SVC_DurangoVoiceData() {};
+
+	virtual void	SetNetChannel(CNetChan* netchan) { m_NetChannel = netchan; }
+	virtual void	SetReliable(bool state) { m_bReliable = state; };
+
+	virtual bool	Process(void)
+	{
+		return CallVFunc<bool>(NetMessageVtbl::Process, this);
+	};
+	virtual	bool	ReadFromBuffer(bf_read* buffer)
+	{
+		return CallVFunc<bool>(NetMessageVtbl::ReadFromBuffer, this, buffer);
+	}
+	virtual	bool	WriteToBuffer(bf_write* buffer)
+	{
+		return CallVFunc<bool>(NetMessageVtbl::WriteToBuffer, this, buffer);
+	}
+
+	virtual bool	IsReliable(void) const { return m_bReliable; };
+
+	virtual int          GetGroup(void) const { return m_nGroup; };
+	virtual int          GetType(void) const { return NetMessageType::svc_DurangoVoiceData; };
+	virtual const char* GetName(void) const { return "svc_DurangoVoiceData"; };
+	virtual CNetChan* GetNetChannel(void) const { return m_NetChannel; };
+	virtual const char* ToString(void) const
+	{
+		static char szBuf[4096];
+		V_snprintf(szBuf, sizeof(szBuf), "%s: client %i, bytes %i", this->GetName(), m_nFromClient, ((m_nLength + 7) >> 3));
+
+		return szBuf;
+	};
+	virtual size_t       GetSize(void) const { return sizeof(SVC_DurangoVoiceData); };
+
+	int m_nFromClient;
+	int m_nLength;
+	int m_unknown;
+	bool m_useVoiceStream;
+	bf_read m_DataIn;
+	void* m_DataOut;
+};
+
 class SVC_PlaylistOverrides : public CNetMessage
 {
 private:
@@ -385,6 +449,31 @@ private:
 // Client messages:
 ///////////////////////////////////////////////////////////////////////////////////////
 
+class CLC_VoiceData : public CNetMessage
+{
+public:
+	void* unk0;
+	int m_nLength;
+	bf_read m_DataIn;
+	bf_write m_DataOut;
+	int unk1;
+	int unk2;
+};
+
+class CLC_DurangoVoiceData : public CNetMessage
+{
+public:
+	void* unk0;
+	int m_nLength;
+	bf_read m_DataIn;
+	bf_write m_DataOut;
+	bool m_skipXidCheck;
+	bool m_useVoiceStream;
+	int m_xid;
+	int m_unknown;
+};
+
+
 class CLC_ClientTick : public CNetMessage
 {
 public:

src/engine/client/client.cpp

@@ -9,6 +9,7 @@
 //
 ///////////////////////////////////////////////////////////////////////////////////
 #include "core/stdafx.h"
+#include "mathlib/bitvec.h"
 #include "tier1/cvar.h"
 #include "tier1/strtools.h"
 #include "engine/server/server.h"
@@ -524,6 +525,41 @@ bool CClient::VProcessSetConVar(CClient* pClient, NET_SetConVar* pMsg)
 	return true;
 }
 
+//---------------------------------------------------------------------------------
+// Purpose: process voice data
+// Input  : *pClient - (ADJ)
+//			*pMsg - 
+// Output : 
+//---------------------------------------------------------------------------------
+bool CClient::VProcessVoiceData(CClient* pClient, CLC_VoiceData* pMsg)
+{
+	char voiceDataBuffer[4096];
+	const int bitsRead = pMsg->m_DataIn.ReadBitsClamped(voiceDataBuffer, pMsg->m_nLength);
+
+	CClient* const pAdj = AdjustShiftedThisPointer(pClient);
+	SV_BroadcastVoiceData(pAdj, Bits2Bytes(bitsRead), voiceDataBuffer);
+
+	return true;
+}
+
+//---------------------------------------------------------------------------------
+// Purpose: process durango voice data
+// Input  : *pClient - (ADJ)
+//			*pMsg - 
+// Output : 
+//---------------------------------------------------------------------------------
+bool CClient::VProcessDurangoVoiceData(CClient* pClient, CLC_DurangoVoiceData* pMsg)
+{
+	char voiceDataBuffer[4096];
+	const int bitsRead = pMsg->m_DataIn.ReadBitsClamped(voiceDataBuffer, pMsg->m_nLength);
+
+	CClient* const pAdj = AdjustShiftedThisPointer(pClient);
+	SV_BroadcastDurangoVoiceData(pAdj, Bits2Bytes(bitsRead), voiceDataBuffer,
+		pMsg->m_xid, pMsg->m_unknown, pMsg->m_useVoiceStream, pMsg->m_skipXidCheck);
+
+	return true;
+}
+
 //---------------------------------------------------------------------------------
 // Purpose: set UserCmd time buffer
 // Input  : numUserCmdProcessTicksMax - 
@@ -578,5 +614,7 @@ void VClient::Detour(const bool bAttach) const
 
 	DetourSetup(&CClient__ProcessStringCmd, &CClient::VProcessStringCmd, bAttach);
 	DetourSetup(&CClient__ProcessSetConVar, &CClient::VProcessSetConVar, bAttach);
+	DetourSetup(&CClient__ProcessVoiceData, &CClient::VProcessVoiceData, bAttach);
+	DetourSetup(&CClient__ProcessDurangoVoiceData, &CClient::VProcessDurangoVoiceData, bAttach);
 #endif // !CLIENT_DLL
 }

src/engine/client/client.h

@@ -69,6 +69,7 @@ public:
 	inline edict_t GetHandle(void) const { return m_nHandle; }
 	inline int GetUserID(void) const { return m_nUserID; }
 	inline NucleusID_t GetNucleusID(void) const { return m_nNucleusID; }
+	inline int GetXPlatID(void) const { return m_XPlatID; }
 
 	inline SIGNONSTATE GetSignonState(void) const { return m_nSignonState; }
 	inline PERSISTENCE GetPersistenceState(void) const { return m_nPersistenceState; }
@@ -124,6 +125,8 @@ public: // Hook statics:
 
 	static bool VProcessStringCmd(CClient* pClient, NET_StringCmd* pMsg);
 	static bool VProcessSetConVar(CClient* pClient, NET_SetConVar* pMsg);
+	static bool VProcessVoiceData(CClient* pClient, CLC_VoiceData* pMsg);
+	static bool VProcessDurangoVoiceData(CClient* pClient, CLC_DurangoVoiceData* pMsg);
 
 private:
 	// Stub reimplementation to avoid the 'no overrider' compiler errors in the
@@ -208,6 +211,9 @@ private:
 	char pad_5B8[8];
 	PERSISTENCE m_nPersistenceState;
 	char pad_05C0[48];
+	char SnapshotBuffer_AndSomeUnknowns[98344]; // TODO: needs to be reversed further.
+	int m_XPlatID;
+	char pad_30758[196964];
 	ServerDataBlock m_DataBlock;
 	char pad_4A3D8[60];
 	int m_LastMovementTick;
@@ -293,6 +299,8 @@ inline void*(*CClient__SendSnapshot)(CClient* pClient, CClientFrame* pFrame, int
 inline void(*CClient__WriteDataBlock)(CClient* pClient, bf_write& buf);
 inline bool(*CClient__ProcessStringCmd)(CClient* pClient, NET_StringCmd* pMsg);
 inline bool(*CClient__ProcessSetConVar)(CClient* pClient, NET_SetConVar* pMsg);
+inline bool(*CClient__ProcessVoiceData)(CClient* pClient, CLC_VoiceData* pMsg);
+inline bool(*CClient__ProcessDurangoVoiceData)(CClient* pClient, CLC_DurangoVoiceData* pMsg);
 
 ///////////////////////////////////////////////////////////////////////////////
 class VClient : public IDetour
@@ -309,6 +317,8 @@ class VClient : public IDetour
 		LogFunAdr("CClient::WriteDataBlock", CClient__WriteDataBlock);
 		LogFunAdr("CClient::ProcessStringCmd", CClient__ProcessStringCmd);
 		LogFunAdr("CClient::ProcessSetConVar", CClient__ProcessSetConVar);
+		LogFunAdr("CClient::ProcessVoiceData", CClient__ProcessVoiceData);
+		LogFunAdr("CClient::ProcessDurangoVoiceData", CClient__ProcessDurangoVoiceData);
 	}
 	virtual void GetFun(void) const
 	{
@@ -323,6 +333,8 @@ class VClient : public IDetour
 
 		g_GameDll.FindPatternSIMD("48 83 EC 28 48 83 C2 20").GetPtr(CClient__ProcessSetConVar);
 		g_GameDll.FindPatternSIMD("48 8B C4 48 89 58 10 48 89 70 18 57 48 81 EC ?? ?? ?? ?? 0F 29 70 E8 8B F2").GetPtr(CClient__SetSignonState);
+		g_GameDll.FindPatternSIMD("48 89 5C 24 ?? 57 B8 ?? ?? ?? ?? E8 ?? ?? ?? ?? 48 2B E0 44 8B 42 20").GetPtr(CClient__ProcessVoiceData);
+		g_GameDll.FindPatternSIMD("40 53 57 B8 ?? ?? ?? ?? E8 ?? ?? ?? ?? 48 2B E0 44 8B 42 20").GetPtr(CClient__ProcessDurangoVoiceData);
 	}
 	virtual void GetVar(void) const { }
 	virtual void GetCon(void) const { }

src/engine/net_chan.h

@@ -124,10 +124,12 @@ public:
 	int         GetSequenceNr(int flow) const;
 	double      GetTimeConnected(void) const;
 
-	inline float GetTimeoutSeconds(void)          const { return m_Timeout; }
+	inline float GetTimeoutSeconds(void)             const { return m_Timeout; }
-	inline int   GetSocket(void)                  const { return m_Socket; }
+	inline int   GetSocket(void)                     const { return m_Socket; }
-	inline const bf_write& GetStreamVoice(void)   const { return m_StreamVoice; }
+	inline const bf_write& GetStreamVoice(void)      const { return m_StreamVoice; }
-	inline const netadr_t& GetRemoteAddress(void) const { return remote_address; }
+	inline const bf_write& GetStreamReliable(void)   const { return m_StreamReliable; }
+	inline const bf_write& GetStreamUnreliable(void) const { return m_StreamUnreliable; }
+	inline const netadr_t& GetRemoteAddress(void)    const { return remote_address; }
 
 	int         GetNumBitsWritten(const bool bReliable);
 	int         GetNumBitsLeft(const bool bReliable);

src/engine/server/datablock_sender.h

@@ -23,7 +23,6 @@ public:
 
 struct ServerDataBlock
 {
-	char blockBuffer[295312]; // this might be wrong !!!
 	void* userData;
 	char gapC0008[56];
 	ServerDataBlockSender sender;

src/engine/server/sv_main.cpp

@@ -162,15 +162,32 @@ bool SV_ActivateServer()
 	return v_SV_ActivateServer();
 }
 
-void SV_BroadcastVoiceData(CClient* const cl, const int nBytes, char* const data)
+//-----------------------------------------------------------------------------
+// Purpose: returns whether voice data can be broadcasted from the server
+//-----------------------------------------------------------------------------
+bool SV_CanBroadcastVoice()
 {
+	if (IsPartyDedi())
+		return false;
+
 	if (IsTrainingDedi())
-		return;
+		return false;
 
 	if (!sv_voiceenable->GetBool())
-		return;
+		return false;
 
 	if (g_ServerGlobalVariables->m_nMaxClients <= 0)
+		return false;
+
+	return true;
+}
+
+//-----------------------------------------------------------------------------
+// Purpose: relays voice data to other clients
+//-----------------------------------------------------------------------------
+void SV_BroadcastVoiceData(CClient* const cl, const int nBytes, char* const data)
+{
+	if (!SV_CanBroadcastVoice())
 		return;
 
 	SVC_VoiceData voiceData(cl->GetUserID(), nBytes, data);
@@ -194,8 +211,11 @@ void SV_BroadcastVoiceData(CClient* const cl, const int nBytes, char* const data
 		if (pClient->GetTeamNum() != cl->GetTeamNum() && !sv_alltalk->GetBool())
 			continue;
 
-		// there's also supposed to be some xplat checks here
+		//if (voice_noxplat->GetBool() && cl->GetXPlatID() != pClient->GetXPlatID())
-		// but since r5r is only on PC, there's no point in implementing them here
+		//{
+		//	if ((cl->GetXPlatID() -1) > 1 || (pClient->GetXPlatID() -1) > 1)
+		//		continue;
+		//}
 
 		CNetChan* const pNetChan = pClient->GetNetChan();
 
@@ -207,3 +227,69 @@ void SV_BroadcastVoiceData(CClient* const cl, const int nBytes, char* const data
 			pClient->SendNetMsgEx(&voiceData, false, false, true);
 	}
 }
+
+//-----------------------------------------------------------------------------
+// Purpose: relays durango voice data to other clients
+//-----------------------------------------------------------------------------
+void SV_BroadcastDurangoVoiceData(CClient* const cl, const int nBytes, char* const data,
+	const int nXid, const int unknown, const bool useVoiceStream, const bool skipXidCheck)
+{
+	if (!SV_CanBroadcastVoice())
+		return;
+
+	SVC_DurangoVoiceData voiceData(cl->GetUserID(), nBytes, data, unknown, useVoiceStream);
+
+	for (int i = 0; i < g_ServerGlobalVariables->m_nMaxClients; i++)
+	{
+		CClient* const pClient = g_pServer->GetClient(i);
+
+		if (!pClient)
+			continue;
+
+		// is this client fully connected
+		if (pClient->GetSignonState() != SIGNONSTATE::SIGNONSTATE_FULL)
+			continue;
+
+		// is this client the sender
+		if (pClient == cl && !sv_voiceEcho->GetBool())
+			continue;
+
+		if (!skipXidCheck && i != nXid)
+			continue;
+
+		// is this client on the sender's team
+		if (pClient->GetTeamNum() != cl->GetTeamNum() && !sv_alltalk->GetBool())
+		{
+			// NOTE: on Durango packets, the game appears to bypass the team
+			// check if 'useVoiceStream' is false, thus forcing the usage
+			// of the reliable stream. Omitted the check as it appears that
+			// could be exploited to transmit voice to other teams while cvar
+			// 'sv_alltalk' is unset.
+			continue;
+		}
+
+		// NOTE: xplat code checks disabled; CClient::GetXPlatID() seems to be
+		// an enumeration of platforms, but the enum hasn't been reversed yet.
+		//if (voice_noxplat->GetBool() && cl->GetXPlatID() != pClient->GetXPlatID())
+		//{
+		//	if ((cl->GetXPlatID() - 1) > 1 || (pClient->GetXPlatID() - 1) > 1)
+		//		continue;
+		//}
+
+		CNetChan* const pNetChan = pClient->GetNetChan();
+
+		if (!pNetChan)
+			continue;
+
+		// NOTE: the game appears to have the ability to use the unreliable
+		// stream as well, but the condition to hit that code path can never
+		// evaluate to true - appears to be a compile time option that hasn't
+		// been fully optimized away? For now only switch between voice and
+		// reliable streams as that is what the original code does.
+		const bf_write& stream = useVoiceStream ? pNetChan->GetStreamVoice() : pNetChan->GetStreamReliable();
+
+		// if stream has enough space for new data
+		if (stream.GetNumBitsLeft() >= 8 * nBytes + 34)
+			pClient->SendNetMsgEx(&voiceData, false, !useVoiceStream, useVoiceStream);
+	}
+}

src/engine/server/sv_main.h

@@ -66,6 +66,7 @@ void SV_InitGameDLL();
 void SV_ShutdownGameDLL();
 bool SV_ActivateServer();
 void SV_BroadcastVoiceData(CClient* const cl, const int nBytes, char* const data);
+void SV_BroadcastDurangoVoiceData(CClient* const cl, const int nBytes, char* const data, const int nXid, const int unknown, const bool useVoiceStream, const bool skipXidCheck);
 void SV_CheckForBanAndDisconnect(CClient* const pClient, const string& svIPAddr, const NucleusID_t nNucleusID, const string& svPersonaName, const int nPort);
 void SV_CheckClientsForBan(const CBanSystem::BannedList_t* const pBannedVec = nullptr);
 ///////////////////////////////////////////////////////////////////////////////

src/public/tier1/bitbuf.h

@@ -212,6 +212,24 @@ public:
 	int64           ReadSignedVarInt64() { return bitbuf::ZigZagDecode64(ReadVarInt64()); }
 
 	void            ReadBits(void* pOutData, int nBits);
+
+	// Helper 'safe' template function that infers the size of the destination
+	// array. This version of the function should be preferred.
+	// Usage: char databuffer[100];
+	//        ReadBitsClamped( dataBuffer, msg->m_nLength );
+	template <typename T, int N>
+	FORCEINLINE int ReadBitsClamped(T (&pOut)[N], int nBits)
+	{
+		const int outSizeBytes = N * sizeof(T);
+		const int outSizeBits = outSizeBytes * 8;
+
+		if (nBits > outSizeBits)
+			nBits = outSizeBits;
+
+		ReadBits(pOut, nBits);
+		return nBits;
+	}
+
 	bool            ReadBytes(void* pOut, int nBytes);