Client: fix underflow and overflow vulnerability in entitylist

Reported by @dr3murr
CClientEntityList::GetClientNetworkable() and CClientEntityList::GetClientEntity() lacked a proper clamp on the 'entNum' parameter. CClientEntityList::GetClientEntity() only handled entNum == -1 cases prior to this patch.

src/game/client/cliententitylist.cpp

@@ -1,5 +1,52 @@
 #include "cliententitylist.h"
 
+// Note: 'entList' points directly at the vtable member
+// of CClientEntityList; sub classed data is truncated.
+static IClientNetworkable* ClientEntityList_GetClientNetworkable(IClientEntityList* const entList, const int entNum)
+{
+	// entNum is used to index into m_EntPtrArray, which is of size
+	// NUM_ENT_ENTRIES. However, both the lower and upper bounds
+	// checks were missing; check it here.
+	if (entNum < 0 || entNum >= NUM_ENT_ENTRIES)
+	{
+		Assert(0);
+		return nullptr;
+	}
+
+	return v_ClientEntityList_GetClientNetworkable(entList, entNum);
+}
+
+// Note: 'entList' points directly at the vtable member
+// of CClientEntityList; sub classed data is truncated.
+static IClientEntity* ClientEntityList_GetClientEntity(IClientEntityList* const entList, const int entNum)
+{
+	// Numbers < -2 will be used to index into the array as follows:
+	// m_EntPtrArray[ (MAX_EDICTS-2) - entNum ]. However, the code
+	// doesn't have a clamp for underflows; check it here. -1 cases
+	// are ignored here as they already are handled correctly.
+	if (entNum < -(MAX_EDICTS - 2))
+	{
+		Assert(0);
+		return nullptr;
+	}
+
+	// m_EntPtrArray is as large as NUM_ENT_ENTRIES, but there is no
+	// overflow clamp; check it here.
+	if (entNum >= NUM_ENT_ENTRIES)
+	{
+		Assert(0);
+		return nullptr;
+	}
+
+	return v_ClientEntityList_GetClientEntity(entList, entNum);
+}
+
+void VClientEntityList::Detour(const bool bAttach) const
+{
+	DetourSetup(&v_ClientEntityList_GetClientNetworkable, &ClientEntityList_GetClientNetworkable, bAttach);
+	DetourSetup(&v_ClientEntityList_GetClientEntity, &ClientEntityList_GetClientEntity, bAttach);
+}
+
 //-----------------------------------------------------------------------------
 // Purpose: a global list of all the entities in the game. All iteration through
 //          entities is done through this object.

src/game/client/cliententitylist.h

@@ -59,8 +59,10 @@ private:
 };
 COMPILE_TIME_ASSERT(sizeof(CClientEntityList) == 0x3800C0);
 
-inline IClientEntityList* g_pClientEntityList = nullptr;
+inline IClientNetworkable* (*v_ClientEntityList_GetClientNetworkable)(IClientEntityList* const entList, const int entNum);
+inline IClientEntity* (*v_ClientEntityList_GetClientEntity)(IClientEntityList* const entList, const int entNum);
 
+inline IClientEntityList* g_pClientEntityList = nullptr;
 extern CClientEntityList* g_clientEntityList;
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -73,14 +75,18 @@ class VClientEntityList : public IDetour
 	{
 		LogVarAdr("g_clientEntityList", g_clientEntityList);
 	}
-	virtual void GetFun(void) const { }
+	virtual void GetFun(void) const
+	{
+		g_GameDll.FindPatternSIMD("48 63 C2 48 03 C0 48 8B 44 C1").GetPtr(v_ClientEntityList_GetClientNetworkable);
+		g_GameDll.FindPatternSIMD("83 FA ?? 7F ?? B8 ?? ?? ?? ?? 2B C2 48 63 D0 48 C1 E2 ?? 48 8B 8C 0A ?? ?? ?? ?? EB ?? 85 D2 78 ?? 48 63 C2 48 C1 E0 ?? 48 8B 8C 08 ?? ?? ?? ?? 48 85 C9 74 ?? 48 8B 01 48 FF 60 ?? 33 C0 C3 CC 80 FA").GetPtr(v_ClientEntityList_GetClientEntity);
+	}
 	virtual void GetVar(void) const
 	{
 		g_GameDll.FindPatternSIMD("48 8D 0D ?? ?? ?? ?? 48 8D 05 ?? ?? ?? ?? 44 89 0D").
 			ResolveRelativeAddressSelf(3, 7).ResolveRelativeAddressSelf(3, 7).GetPtr(g_clientEntityList);
 	}
 	virtual void GetCon(void) const { }
-	virtual void Detour(const bool bAttach) const { };
+	virtual void Detour(const bool bAttach) const;
 };
 ///////////////////////////////////////////////////////////////////////////////