From ce163a323b0b606b92a9aa973c259555545c2047 Mon Sep 17 00:00:00 2001
From: Alik Aslanyan <alik.aslanyan@picsart.com>
Date: Fri, 6 Apr 2018 20:11:38 +0400
Subject: [PATCH] Fixes for new exploit involving " in info strings. (#596)

* Refactor SV_CheckForDuplicateNames

* Refactor and fix Info_IsValid
---
 rehlds/engine/info.cpp    | 77 +++++++++++++++++++--------------------
 rehlds/engine/sv_main.cpp | 18 ++++-----
 2 files changed, 46 insertions(+), 49 deletions(-)

diff --git a/rehlds/engine/info.cpp b/rehlds/engine/info.cpp
index 7d5f19f..8a60be6 100644
--- a/rehlds/engine/info.cpp
+++ b/rehlds/engine/info.cpp
@@ -541,11 +541,6 @@ void Info_Print(const char *s)
 
 qboolean Info_IsValid(const char *s)
 {
-	char key[MAX_KV_LEN];
-	char value[MAX_KV_LEN];
-	char *c;
-	int nCount;
-
 	while (*s)
 	{
 		if (*s == '\\')
@@ -553,46 +548,48 @@ qboolean Info_IsValid(const char *s)
 			s++;	// skip the slash
 		}
 
-		// Copy a key
-		nCount = 0;
-		c = key;
-		while (*s != '\\')
+		// Returns character count
+		// -1 - error
+		//  0 - string size is zero
+		enum class AllowNull {
+			Yes,
+			No,
+		};
+		auto validate = [&s](AllowNull allowNull) -> int
 		{
-			if (!*s)
-			{
-				return FALSE;		// key should end with a \, not a NULL
-			}
-			if (nCount >= MAX_KV_LEN)
-			{
-				return FALSE;		// key length should be less then MAX_KV_LEN
-			}
-			*c++ = *s++;
-			nCount++;
-		}
-		*c = 0;
-		s++;	// skip the slash
+			int nCount = 0;
 
-		// Copy a value
-		nCount = 0;
-		c = value;
-		while (*s != '\\')
-		{
-			if (!*s)
+			for(; *s != '\\'; nCount++, s++)
 			{
-				break;				// allow value to be ended with NULL
-			}
-			if (nCount >= MAX_KV_LEN)
-			{
-				return FALSE;		// value length should be less then MAX_KV_LEN
-			}
-			*c++ = *s++;
-			nCount++;
-		}
-		*c = 0;
+				if (!*s)
+				{
+					return (allowNull == AllowNull::Yes) ? nCount : -1;
+				}
 
-		if (value[0] == 0)
+				if (nCount >= MAX_KV_LEN)
+				{
+					return -1;		// string length should be less then MAX_KV_LEN
+				}
+
+#ifdef REHLDS_FIXES
+				if (*s == '\"')
+				{
+					return -1; // string should not contain "
+				}
+#endif
+			}
+			return nCount;
+		};
+
+		if (validate(AllowNull::No) == -1)
 		{
-			return FALSE;	// empty values are not valid
+			return FALSE;
+		}
+		s++; // Skip slash
+
+		if (validate(AllowNull::Yes) <= 0)
+		{
+			return FALSE;
 		}
 
 		if (!*s)
diff --git a/rehlds/engine/sv_main.cpp b/rehlds/engine/sv_main.cpp
index 3f6176f..21d7de9 100644
--- a/rehlds/engine/sv_main.cpp
+++ b/rehlds/engine/sv_main.cpp
@@ -1977,29 +1977,29 @@ int SV_CheckForDuplicateSteamID(client_t *client)
 
 int SV_CheckForDuplicateNames(char *userinfo, qboolean bIsReconnecting, int nExcludeSlot)
 {
-	const char *val;
-	int i;
-	client_t *client;
 	int dupc = 0;
-	char rawname[MAX_NAME];
-	char newname[MAX_NAME];
 	int changed = FALSE;
 
-	val = Info_ValueForKey(userinfo, "name");
+	const char *val = Info_ValueForKey(userinfo, "name");
+
+	char rawname[MAX_NAME];
 	Q_strncpy(rawname, val, MAX_NAME - 1);
 
 	while (true)
 	{
-		for (i = 0, client = g_psvs.clients; i < g_psvs.maxclients; i++, client++)
+		int clientId = 0;
+		client_t *client = &g_psvs.clients[0];
+		for (; clientId < g_psvs.maxclients; clientId++, client++)
 		{
-			if (client->connected && !(i == nExcludeSlot && bIsReconnecting) && !Q_stricmp(client->name, val))
+			if (client->connected && !(clientId == nExcludeSlot && bIsReconnecting) && !Q_stricmp(client->name, val))
 				break;
 		}
 
 		// no duplicates for current name
-		if (i == g_psvs.maxclients)
+		if (clientId == g_psvs.maxclients)
 			return changed;
 
+		char newname[MAX_NAME];
 		Q_snprintf(newname, sizeof(newname), "(%d)%-0.*s", ++dupc, 28, rawname);
 #ifdef REHLDS_FIXES
 		// Fix possibly incorrectly cut UTF8 chars