made unpacking of net strings more strict - prevents possible malformed output

8 files changed, 91 insertions, 26 deletions

diff --git a/datasrc/datatypes.py b/datasrc/datatypes.py
index e4459e48..4e293ab1 100644
--- a/datasrc/datatypes.py
+++ b/datasrc/datatypes.py
@@ -294,6 +294,14 @@ class NetString(NetVariable):
 	def emit_pack(self):
 		return ["pPacker->AddString(%s, -1);" % self.name]
 
+class NetStringStrict(NetVariable):
+	def emit_declaration(self):
+		return ["const char *%s;"%self.name]
+	def emit_unpack(self):
+		return ["pMsg->%s = pUnpacker->GetString(CUnpacker::SANITIZE_CC|CUnpacker::SKIP_START_WHITESPACES);" % self.name]
+	def emit_pack(self):
+		return ["pPacker->AddString(%s, -1);" % self.name]
+
 class NetIntAny(NetVariable):
 	def emit_declaration(self):
 		return ["int %s;"%self.name]
diff --git a/datasrc/network.py b/datasrc/network.py
index e31c0b3d..e01dba63 100644
--- a/datasrc/network.py
+++ b/datasrc/network.py
@@ -237,13 +237,13 @@ Messages = [
 	]),
 	
 	NetMessage("Sv_VoteOption", [
-		NetString("m_pCommand"),
+		NetStringStrict("m_pCommand"),
 	]),
 
 	NetMessage("Sv_VoteSet", [
 		NetIntRange("m_Timeout", 0, 60),
-		NetString("m_pDescription"),
-		NetString("m_pCommand"),
+		NetStringStrict("m_pDescription"),
+		NetStringStrict("m_pCommand"),
 	]),
 
 	NetMessage("Sv_VoteStatus", [
@@ -264,16 +264,16 @@ Messages = [
 	]),
 	
 	NetMessage("Cl_StartInfo", [
-		NetString("m_pName"),
-		NetString("m_pSkin"),
+		NetStringStrict("m_pName"),
+		NetStringStrict("m_pSkin"),
 		NetBool("m_UseCustomColor"),
 		NetIntAny("m_ColorBody"),
 		NetIntAny("m_ColorFeet"),
 	]),	
 
 	NetMessage("Cl_ChangeInfo", [
-		NetString("m_pName"),
-		NetString("m_pSkin"),
+		NetStringStrict("m_pName"),
+		NetStringStrict("m_pSkin"),
 		NetBool("m_UseCustomColor"),
 		NetIntAny("m_ColorBody"),
 		NetIntAny("m_ColorFeet"),
@@ -290,7 +290,7 @@ Messages = [
 	]),
 	
 	NetMessage("Cl_CallVote", [
-		NetString("m_Type"),
-		NetString("m_Value"),
+		NetStringStrict("m_Type"),
+		NetStringStrict("m_Value"),
 	]),
 ]
diff --git a/src/base/system.c b/src/base/system.c
index 457b761a..84f732e7 100644
--- a/src/base/system.c
+++ b/src/base/system.c
@@ -1092,6 +1092,18 @@ void str_sanitize_strong(char *str_in)
 	}
 }
 
+/* makes sure that the string only contains the characters between 32 and 255 */
+void str_sanitize_cc(char *str_in)
+{
+	unsigned char *str = (unsigned char *)str_in;
+	while(*str)
+	{
+		if(*str < 32)
+			*str = ' ';
+		str++;
+	}
+}
+
 /* makes sure that the string only contains the characters between 32 and 255 + \r\n\t */
 void str_sanitize(char *str_in)
 {
@@ -1104,6 +1116,13 @@ void str_sanitize(char *str_in)
 	}
 }
 
+char *str_skip_whitespaces(char *str)
+{
+	while(*str && (*str == ' ' || *str == '\t' || *str == '\n' || *str == '\r'))
+		str++;
+	return str;
+}
+
 /* case */
 int str_comp_nocase(const char *a, const char *b)
 {
diff --git a/src/base/system.h b/src/base/system.h
index 0850a08b..c49fbdad 100644
--- a/src/base/system.h
+++ b/src/base/system.h
@@ -732,9 +732,21 @@ void str_format(char *buffer, int buffer_size, const char *format, ...);
 void str_sanitize_strong(char *str);
 
 /*
+	Function: str_sanitize_cc
+		Replaces all characters below 32 with whitespace.
+	
+	Parameters:
+		str - String to sanitize.
+
+	Remarks:
+		- The strings are treated as zero-termineted strings.
+*/
+void str_sanitize_cc(char *str);
+
+/*
 	Function: str_sanitize
-		Replaces all characters below 32 and above 127 with whitespace with
-		exception to \r, \n and \r.
+		Replaces all characters below 32 with whitespace with
+		exception to \t, \n and \r.
 	
 	Parameters:
 		str - String to sanitize.
@@ -745,6 +757,22 @@ void str_sanitize_strong(char *str);
 void str_sanitize(char *str);
 
 /*
+	Function: str_skip_whitespaces
+		Skips leading whitespace characters(' ', '\t', '\n', '\r').
+	
+	Parameters:
+		str - Pointer to the string.
+
+	Returns:
+		Pointer to the first non-whitespace character found
+		within the string.
+
+	Remarks:
+		- The strings are treated as zero-termineted strings.
+*/
+char *str_skip_whitespaces(char *str);
+
+/*
 	Function: str_comp_nocase
 		Compares to strings case insensitive.
 	
diff --git a/src/engine/client/client.cpp b/src/engine/client/client.cpp
index 78bed263..d2872408 100644
--- a/src/engine/client/client.cpp
+++ b/src/engine/client/client.cpp
@@ -870,10 +870,10 @@ void CClient::ProcessPacket(CNetChunk *pPacket)
 				Up.Reset((unsigned char*)pPacket->m_pData+sizeof(SERVERBROWSE_INFO), pPacket->m_DataSize-sizeof(SERVERBROWSE_INFO));
 				if(PacketType >= 2)
 					Token = str_toint(Up.GetString());
-				str_copy(Info.m_aVersion, Up.GetString(), sizeof(Info.m_aVersion));
-				str_copy(Info.m_aName, Up.GetString(), sizeof(Info.m_aName));
-				str_copy(Info.m_aMap, Up.GetString(), sizeof(Info.m_aMap));
-				str_copy(Info.m_aGameType, Up.GetString(), sizeof(Info.m_aGameType));
+				str_copy(Info.m_aVersion, Up.GetString(CUnpacker::SANITIZE_CC|CUnpacker::SKIP_START_WHITESPACES), sizeof(Info.m_aVersion));
+				str_copy(Info.m_aName, Up.GetString(CUnpacker::SANITIZE_CC|CUnpacker::SKIP_START_WHITESPACES), sizeof(Info.m_aName));
+				str_copy(Info.m_aMap, Up.GetString(CUnpacker::SANITIZE_CC|CUnpacker::SKIP_START_WHITESPACES), sizeof(Info.m_aMap));
+				str_copy(Info.m_aGameType, Up.GetString(CUnpacker::SANITIZE_CC|CUnpacker::SKIP_START_WHITESPACES), sizeof(Info.m_aGameType));
 				Info.m_Flags = str_toint(Up.GetString());
 				Info.m_Progression = str_toint(Up.GetString());
 				Info.m_NumPlayers = str_toint(Up.GetString());
@@ -889,7 +889,7 @@ void CClient::ProcessPacket(CNetChunk *pPacket)
 
 				for(int i = 0; i < Info.m_NumPlayers; i++)
 				{
-					str_copy(Info.m_aPlayers[i].m_aName, Up.GetString(), sizeof(Info.m_aPlayers[i].m_aName));
+					str_copy(Info.m_aPlayers[i].m_aName, Up.GetString(CUnpacker::SANITIZE_CC|CUnpacker::SKIP_START_WHITESPACES), sizeof(Info.m_aPlayers[i].m_aName));
 					Info.m_aPlayers[i].m_Score = str_toint(Up.GetString());
 				}
 
@@ -933,7 +933,7 @@ void CClient::ProcessPacket(CNetChunk *pPacket)
 			// system message
 			if(Msg == NETMSG_MAP_CHANGE)
 			{
-				const char *pMap = Unpacker.GetString();
+				const char *pMap = Unpacker.GetString(CUnpacker::SANITIZE_CC|CUnpacker::SKIP_START_WHITESPACES);
 				int MapCrc = Unpacker.GetInt();
 				const char *pError = 0;
 
diff --git a/src/engine/server/server.cpp b/src/engine/server/server.cpp
index a849504d..95c708a1 100644
--- a/src/engine/server/server.cpp
+++ b/src/engine/server/server.cpp
@@ -613,7 +613,7 @@ void CServer::ProcessClientPacket(CNetChunk *pPacket)
 		{
 			char aVersion[64];
 			const char *pPassword;
-			str_copy(aVersion, Unpacker.GetString(), 64);
+			str_copy(aVersion, Unpacker.GetString(CUnpacker::SANITIZE_CC), 64);
 			if(str_comp(aVersion, GameServer()->NetVersion()) != 0)
 			{
 				// OH FUCK! wrong version, drop him
@@ -623,9 +623,9 @@ void CServer::ProcessClientPacket(CNetChunk *pPacket)
 				return;
 			}
 			
-			str_copy(m_aClients[ClientId].m_aName, Unpacker.GetString(), MAX_NAME_LENGTH);
-			str_copy(m_aClients[ClientId].m_aClan, Unpacker.GetString(), MAX_CLANNAME_LENGTH);
-			pPassword = Unpacker.GetString();
+			str_copy(m_aClients[ClientId].m_aName, Unpacker.GetString(CUnpacker::SANITIZE_CC|CUnpacker::SKIP_START_WHITESPACES), MAX_NAME_LENGTH);
+			str_copy(m_aClients[ClientId].m_aClan, Unpacker.GetString(CUnpacker::SANITIZE_CC|CUnpacker::SKIP_START_WHITESPACES), MAX_CLANNAME_LENGTH);
+			pPassword = Unpacker.GetString(CUnpacker::SANITIZE_CC);
 			
 			if(g_Config.m_Password[0] != 0 && str_comp(g_Config.m_Password, pPassword) != 0)
 			{
@@ -762,7 +762,7 @@ void CServer::ProcessClientPacket(CNetChunk *pPacket)
 			{
 				const char *pPw;
 				Unpacker.GetString(); // login name, not used
-				pPw = Unpacker.GetString();
+				pPw = Unpacker.GetString(CUnpacker::SANITIZE_CC);
 				
 				if(Unpacker.Error() == 0)
 				{
diff --git a/src/engine/shared/packer.cpp b/src/engine/shared/packer.cpp
index 3e1d8dd6..53a74754 100644
--- a/src/engine/shared/packer.cpp
+++ b/src/engine/shared/packer.cpp
@@ -114,7 +114,7 @@ int CUnpacker::GetInt()
 	return i;
 }
 
-const char *CUnpacker::GetString()
+const char *CUnpacker::GetString(int SanitizeType)
 {
 	if(m_Error || m_pCurrent >= m_pEnd)
 		return "";
@@ -132,8 +132,11 @@ const char *CUnpacker::GetString()
 	m_pCurrent++;
 	
 	// sanitize all strings
-	str_sanitize(pPtr);
-	return pPtr;
+	if(SanitizeType&SANITIZE)
+		str_sanitize(pPtr);
+	else if(SanitizeType&SANITIZE_CC)
+		str_sanitize_cc(pPtr);
+	return SanitizeType&SKIP_START_WHITESPACES ? str_skip_whitespaces(pPtr) : pPtr;
 }
 
 const unsigned char *CUnpacker::GetRaw(int Size)
diff --git a/src/engine/shared/packer.h b/src/engine/shared/packer.h
index 7a98501a..02619919 100644
--- a/src/engine/shared/packer.h
+++ b/src/engine/shared/packer.h
@@ -32,9 +32,16 @@ class CUnpacker
 	const unsigned char *m_pEnd;
 	int m_Error;
 public:
+	enum
+	{
+		SANITIZE=1,
+		SANITIZE_CC=2,
+		SKIP_START_WHITESPACES=4
+	};
+
 	void Reset(const void *pData, int Size);
 	int GetInt();
-	const char *GetString();
+	const char *GetString(int SanitizeType = SANITIZE);
 	const unsigned char *GetRaw(int Size);
 	bool Error() const { return m_Error; }
 };