FIX: Unexpected behavior with mp_forcerespawn (#653)

* Unexpected behavior with mp_forcerespawn leading to a possible exploit

Basically there is an exploit (or bug?) where depending on mp_forcerespawn if his value is higher to 0 and lower than 1, when you respawn you have a brief window to change your team, if you succesfully manage to change your team between the mp_forcerespawn value and WITHOUT closing the change appearance menu, you won't die due to "m_fNextSuicideTime", that will trigger the change team but without actually changing your skin model, you will keep the enemy one but the team change will success.

Thanks https://github.com/metita for helping me with this

Co-Authored-By: metita <33007491+metita@users.noreply.github.com>

* Revert "Unexpected behavior with mp_forcerespawn leading to a possible exploit"

This reverts commit 73d1c1670645a0798b94055562baff2484cc2cd9.

* fix: nullify `m_fNextSuicideTime` before call `ClientKill()`

* ClientKill: refactoring

* add forgotten if-statement

* remove macros

Co-authored-by: metita <33007491+metita@users.noreply.github.com>
Co-authored-by: Sergey Shorokhov <wopox1337@ya.ru>

regamedll/dlls/API/CSPlayer.cpp

@@ -112,8 +112,10 @@ EXT_FUNC bool CCSPlayer::JoinTeam(TeamName team)
 
 	if (pPlayer->pev->deadflag == DEAD_NO)
 	{
-		ClientKill(pPlayer->edict());
+		if (pPlayer->Kill())
-		pPlayer->pev->frags++;
+		{
+			pPlayer->pev->frags++;
+		}
 	}
 
 	MESSAGE_BEGIN(MSG_ALL, gmsgScoreInfo);

regamedll/dlls/bot/cs_bot_manager.cpp

@@ -410,11 +410,7 @@ void CCSBotManager::ServerCommand(const char *pcmd)
 			{
 				if (killThemAll || FStrEq(name, msg))
 				{
-#ifdef REGAMEDLL_FIXES
+					pPlayer->Kill();
-					ClientKill(pPlayer->edict());
-#else
-					pPlayer->TakeDamage(pPlayer->pev, pPlayer->pev, 9999.9f, DMG_CRUSH);
-#endif
 				}
 			}
 		}

regamedll/dlls/client.cpp

@@ -375,29 +375,13 @@ void EXT_FUNC ClientKill(edict_t *pEntity)
 	entvars_t *pev = &pEntity->v;
 	CBasePlayer *pPlayer = CBasePlayer::Instance(pev);
 
-	if (pPlayer->GetObserverMode() != OBS_NONE)
-		return;
-
-	if (pPlayer->m_iJoiningState != JOINED)
-		return;
-
 	// prevent suiciding too often
 	if (pPlayer->m_fNextSuicideTime > gpGlobals->time)
 		return;
 
-	pPlayer->m_LastHitGroup = HITGROUP_GENERIC;
-
 	// don't let them suicide for 1 second after suiciding
 	pPlayer->m_fNextSuicideTime = gpGlobals->time + 1.0f;
-
+	pPlayer->Kill();
-	// have the player kill themself
-	pEntity->v.health = 0;
-	pPlayer->Killed(pev, GIB_NEVER);
-
-	if (CSGameRules()->m_pVIP == pPlayer)
-	{
-		CSGameRules()->m_iConsecutiveVIP = 10;
-	}
 }
 
 LINK_HOOK_VOID_CHAIN(ShowMenu, (CBasePlayer *pPlayer, int bitsValidSlots, int nDisplayTime, BOOL fNeedMore, char *pszText), pPlayer, bitsValidSlots, nDisplayTime, fNeedMore, pszText)
@@ -1856,10 +1840,11 @@ BOOL EXT_FUNC __API_HOOK(HandleMenu_ChooseTeam)(CBasePlayer *pPlayer, int slot)
 		{
 			if (pPlayer->m_iTeam != UNASSIGNED && pPlayer->pev->deadflag == DEAD_NO)
 			{
-				ClientKill(pPlayer->edict());
+				if (pPlayer->Kill())
-
+				{
-				// add 1 to frags to balance out the 1 subtracted for killing yourself
+					// add 1 to frags to balance out the 1 subtracted for killing yourself
-				pPlayer->pev->frags++;
+					pPlayer->pev->frags++;
+				}
 			}
 
 			pPlayer->RemoveAllItems(TRUE);
@@ -2087,10 +2072,10 @@ BOOL EXT_FUNC __API_HOOK(HandleMenu_ChooseTeam)(CBasePlayer *pPlayer, int slot)
 		pPlayer->m_iMenu = Menu_ChooseAppearance;
 
 		// Show the appropriate Choose Appearance menu
-		// This must come before ClientKill() for CheckWinConditions() to function properly
+		// This must come before pPlayer->Kill() for CheckWinConditions() to function properly
 		if (pPlayer->pev->deadflag == DEAD_NO)
 		{
-			ClientKill(pPlayer->edict());
+			pPlayer->Kill();
 		}
 	}
 

regamedll/dlls/player.cpp

@@ -3862,9 +3862,7 @@ void EXT_FUNC CBasePlayer::__API_HOOK(RoundRespawn)()
 
 #ifdef REGAMEDLL_FIXES
 	if (m_bPunishedForTK && pev->health > 0)
-	{
+		Kill();
-		ClientKill(ENT(pev));
-	}
 #endif
 
 }
@@ -10313,3 +10311,23 @@ void EXT_FUNC CBasePlayer::__API_HOOK(DropIdlePlayer)(const char *reason)
 	SERVER_COMMAND(UTIL_VarArgs("kick \"%s\"\n", STRING(pev->netname)));
 #endif // #ifdef REGAMEDLL_FIXES
 }
+
+bool CBasePlayer::Kill()
+{
+	if (GetObserverMode() != OBS_NONE)
+		return false;
+	
+	if (m_iJoiningState != JOINED)
+		return false;
+	
+	m_LastHitGroup = HITGROUP_GENERIC;
+
+	// have the player kill himself
+	pev->health = 0.0f;
+	Killed(pev, GIB_NEVER);
+
+	if (CSGameRules()->m_pVIP == this)
+		CSGameRules()->m_iConsecutiveVIP = 10;
+	
+	return true;
+}

regamedll/dlls/player.h

@@ -642,6 +642,7 @@ public:
 	void RemoveSpawnProtection();
 	void UseEmpty();
 	void DropIdlePlayer(const char *reason);
+	bool Kill();
 
 	// templates
 	template<typename T = CBasePlayerItem, typename Functor>