Merge #1082

1082: Fix a bug with name bans r=Learath2 a=heinrich5991

Overlong nick names could be used to circumvent name bans.

CMakeLists.txt

@@ -917,6 +917,8 @@ endif()
 set_glob(ENGINE_SERVER GLOB src/engine/server
   authmanager.cpp
   authmanager.h
+  name_ban.cpp
+  name_ban.h
   register.cpp
   register.h
   server.cpp
@@ -1102,6 +1104,7 @@ if(GTEST_FOUND OR DOWNLOAD_GTEST)
     fs.cpp
     git_revision.cpp
     jobs.cpp
+    name_ban.cpp
     str.cpp
     strip_path_and_extension.cpp
     teehistorian.cpp
@@ -1111,6 +1114,8 @@ if(GTEST_FOUND OR DOWNLOAD_GTEST)
     unix.cpp
   )
   set(TESTS_EXTRA
+    src/engine/server/name_ban.cpp
+    src/engine/server/name_ban.h
     src/game/server/teehistorian.cpp
     src/game/server/teehistorian.h
   )

src/base/system.c

@@ -2701,11 +2701,11 @@ float str_tofloat(const char *str) { return atof(str); }
 
 int str_utf8_isspace(int code)
 {
-	return code > 0x20 && code != 0xA0 && code != 0x034F && code != 0x2800 &&
+	return !(code > 0x20 && code != 0xA0 && code != 0x034F && code != 0x2800 &&
 		(code < 0x2000 || code > 0x200F) && (code < 0x2028 || code > 0x202F) &&
 		(code < 0x205F || code > 0x2064) && (code < 0x206A || code > 0x206F) &&
 		(code < 0xFE00 || code > 0xFE0F) && code != 0xFEFF &&
-		(code < 0xFFF9 || code > 0xFFFC);
+		(code < 0xFFF9 || code > 0xFFFC));
 }
 
 const char *str_utf8_skip_whitespaces(const char *str)
@@ -2719,7 +2719,7 @@ const char *str_utf8_skip_whitespaces(const char *str)
 		code = str_utf8_decode(&str);
 
 		// check if unicode is not empty
-		if(str_utf8_isspace(code))
+		if(!str_utf8_isspace(code))
 		{
 			return str_old;
 		}
@@ -2728,6 +2728,31 @@ const char *str_utf8_skip_whitespaces(const char *str)
 	return str;
 }
 
+void str_utf8_trim_right(char *param)
+{
+	const char *str = param;
+	char *end = 0;
+	while(*str)
+	{
+		char *str_old = (char *)str;
+		int code = str_utf8_decode(&str);
+
+		// check if unicode is not empty
+		if(!str_utf8_isspace(code))
+		{
+			end = 0;
+		}
+		else if(!end)
+		{
+			end = str_old;
+		}
+	}
+	if(end)
+	{
+		*end = 0;
+	}
+}
+
 int str_utf8_isstart(char c)
 {
 	if((c&0xC0) == 0x80) /* 10xxxxxx */

src/base/system.h

@@ -1603,12 +1603,50 @@ int str_utf8_to_skeleton(const char *str, int *buf, int buf_len);
 */
 int str_utf8_comp_confusable(const char *a, const char *b);
 
+/*
+	Function: str_utf8_isspace
+		Checks whether the given Unicode codepoint renders as space.
+
+	Parameters:
+		code - Unicode codepoint to check.
+
+	Returns:
+		0 if the codepoint does not render as space, != 0 if it does.
+*/
 int str_utf8_isspace(int code);
 
 int str_utf8_isstart(char c);
 
+/*
+	Function: str_utf8_skip_whitespaces
+		Skips leading characters that render as spaces.
+
+	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-terminated strings.
+*/
 const char *str_utf8_skip_whitespaces(const char *str);
 
+/*
+	Function: str_utf8_trim_right
+		Removes trailing characters that render as spaces by modifying
+		the string in-place.
+
+	Parameters:
+		str - Pointer to the string.
+
+	Remarks:
+		- The strings are treated as zero-terminated strings.
+		- The string is modified in-place.
+*/
+void str_utf8_trim_right(char *str);
+
 /*
 	Function: str_utf8_rewind
 		Moves a cursor backwards in an utf8 string

src/engine/server/name_ban.cpp

@@ -0,0 +1,24 @@
+#include "name_ban.h"
+
+CNameBan *IsNameBanned(const char *pName, CNameBan *pNameBans, int NumNameBans)
+{
+	char aTrimmed[MAX_NAME_LENGTH];
+	str_copy(aTrimmed, str_utf8_skip_whitespaces(pName), sizeof(aTrimmed));
+	str_utf8_trim_right(aTrimmed);
+
+	int aSkeleton[MAX_NAME_SKELETON_LENGTH];
+	int SkeletonLength = str_utf8_to_skeleton(aTrimmed, aSkeleton, sizeof(aSkeleton) / sizeof(aSkeleton[0]));
+	int aBuffer[MAX_NAME_SKELETON_LENGTH * 2 + 2];
+
+	CNameBan *pResult = 0;
+	for(int i = 0; i < NumNameBans; i++)
+	{
+		CNameBan *pBan = &pNameBans[i];
+		int Distance = str_utf32_dist_buffer(aSkeleton, SkeletonLength, pBan->m_aSkeleton, pBan->m_SkeletonLength, aBuffer, sizeof(aBuffer) / sizeof(aBuffer[0]));
+		if(Distance <= pBan->m_Distance)
+		{
+			pResult = pBan;
+		}
+	}
+	return pResult;
+}

src/engine/server/name_ban.h

@@ -0,0 +1,33 @@
+#ifndef ENGINE_SERVER_NAME_BANS_H
+#define ENGINE_SERVER_NAME_BANS_H
+
+#include <base/system.h>
+#include <engine/shared/protocol.h>
+
+enum
+{
+	MAX_NAME_SKELETON_LENGTH=MAX_NAME_LENGTH*4,
+	MAX_NAMEBAN_REASON_LENGTH=64
+};
+
+class CNameBan
+{
+public:
+	CNameBan() {}
+	CNameBan(const char *pName, int Distance, const char *pReason = "") :
+		m_Distance(Distance)
+	{
+		str_copy(m_aName, pName, sizeof(m_aName));
+		m_SkeletonLength = str_utf8_to_skeleton(m_aName, m_aSkeleton, sizeof(m_aSkeleton) / sizeof(m_aSkeleton[0]));
+		str_copy(m_aReason, pReason, sizeof(m_aReason));
+	}
+	char m_aName[MAX_NAME_LENGTH];
+	char m_aReason[MAX_NAMEBAN_REASON_LENGTH];
+	int m_aSkeleton[MAX_NAME_SKELETON_LENGTH];
+	int m_SkeletonLength;
+	int m_Distance;
+};
+
+CNameBan *IsNameBanned(const char *pName, CNameBan *pNameBans, int NumNameBans);
+
+#endif // ENGINE_SERVER_NAME_BANS_H

src/engine/server/server.cpp

@@ -44,43 +44,6 @@
 	#include <windows.h>
 #endif
 
-static const char *StrLtrim(const char *pStr)
-{
-	while(*pStr)
-	{
-		const char *pStrOld = pStr;
-		int Code = str_utf8_decode(&pStr);
-
-		// check if unicode is not empty
-		if(str_utf8_isspace(Code))
-		{
-			return pStrOld;
-		}
-	}
-	return pStr;
-}
-
-static void StrRtrim(char *pStr)
-{
-	const char *p = pStr;
-	const char *pEnd = 0;
-	while(*p)
-	{
-		const char *pStrOld = p;
-		int Code = str_utf8_decode(&p);
-
-		// check if unicode is not empty
-		if(str_utf8_isspace(Code))
-		{
-			pEnd = 0;
-		}
-		else if(pEnd == 0)
-			pEnd = pStrOld;
-	}
-	if(pEnd != 0)
-		*(const_cast<char *>(pEnd)) = 0;
-}
-
 
 CSnapIDPool::CSnapIDPool()
 {
@@ -343,14 +306,13 @@ CServer::CServer()
 	Init();
 }
 
-
 int CServer::TrySetClientName(int ClientID, const char *pName)
 {
 	char aTrimmedName[64];
 
 	// trim the name
-	str_copy(aTrimmedName, StrLtrim(pName), sizeof(aTrimmedName));
+	str_copy(aTrimmedName, str_utf8_skip_whitespaces(pName), sizeof(aTrimmedName));
-	StrRtrim(aTrimmedName);
+	str_utf8_trim_right(aTrimmedName);
 
 	// check for empty names
 	if(!aTrimmedName[0])
@@ -381,8 +343,6 @@ int CServer::TrySetClientName(int ClientID, const char *pName)
 	return 0;
 }
 
-
-
 void CServer::SetClientName(int ClientID, const char *pName)
 {
 	if(ClientID < 0 || ClientID >= MAX_CLIENTS || m_aClients[ClientID].m_State < CClient::STATE_READY)
@@ -391,20 +351,7 @@ void CServer::SetClientName(int ClientID, const char *pName)
 	if(!pName)
 		return;
 
-	int Skeleton[MAX_NAME_SKELETON_LENGTH];
+	CNameBan *pBanned = IsNameBanned(pName, m_aNameBans.base_ptr(), m_aNameBans.size());
-	int SkeletonLength = str_utf8_to_skeleton(pName, Skeleton, sizeof(Skeleton) / sizeof(Skeleton[0]));
-	int Buffer[MAX_NAME_SKELETON_LENGTH * 2 + 2];
-	CNameBan *pBanned = 0;
-	for(int i = 0; i < m_aNameBans.size(); i++)
-	{
-		CNameBan *pBan = &m_aNameBans[i];
-		int Distance = str_utf32_dist_buffer(Skeleton, SkeletonLength, pBan->m_aSkeleton, pBan->m_SkeletonLength, Buffer, sizeof(Buffer) / sizeof(Buffer[0]));
-		if(Distance <= pBan->m_Distance)
-		{
-			pBanned = pBan;
-		}
-	}
-
 	if(pBanned)
 	{
 		if(m_aClients[ClientID].m_State == CClient::STATE_READY)

src/engine/server/server.h

@@ -22,6 +22,7 @@
 #include <base/tl/array.h>
 
 #include "authmanager.h"
+#include "name_ban.h"
 
 #if defined (CONF_SQL)
 	#include "sql_connector.h"
@@ -98,30 +99,6 @@ class CServer : public IServer
 	UNIXSOCKET m_ConnLoggingSocket;
 #endif
 
-	enum
-	{
-		MAX_NAME_SKELETON_LENGTH=MAX_NAME_LENGTH*4,
-		MAX_REASON_LENGTH=64
-	};
-
-	class CNameBan
-	{
-	public:
-		CNameBan() {}
-		CNameBan(const char *pName, int Distance, const char *pReason) :
-			m_Distance(Distance)
-		{
-			str_copy(m_aName, pName, sizeof(m_aName));
-			m_SkeletonLength = str_utf8_to_skeleton(m_aName, m_aSkeleton, sizeof(m_aSkeleton) / sizeof(m_aSkeleton[0]));
-			str_copy(m_aReason, pReason, sizeof(m_aReason));
-		}
-		char m_aName[MAX_NAME_LENGTH];
-		char m_aReason[MAX_REASON_LENGTH];
-		int m_aSkeleton[MAX_NAME_SKELETON_LENGTH];
-		int m_SkeletonLength;
-		int m_Distance;
-	};
-
 public:
 	class IGameServer *GameServer() { return m_pGameServer; }
 	class IConsole *Console() { return m_pConsole; }

src/game/client/components/chat.cpp

@@ -462,7 +462,7 @@ void CChat::AddLine(int ClientID, int Team, const char *pLine)
 		int Code = str_utf8_decode(&pStr);
 
 		// check if unicode is not empty
-		if(str_utf8_isspace(Code))
+		if(!str_utf8_isspace(Code))
 		{
 			pEnd = 0;
 		}

src/game/server/gamecontext.cpp

@@ -1171,7 +1171,7 @@ void CGameContext::OnMessage(int MsgID, CUnpacker *pUnpacker, int ClientID)
 				int Code = str_utf8_decode(&p);
 
 				// check if unicode is not empty
-				if(str_utf8_isspace(Code))
+				if(!str_utf8_isspace(Code))
 				{
 					pEnd = 0;
 				}

src/test/name_ban.cpp

@@ -0,0 +1,20 @@
+#include <gtest/gtest.h>
+
+#include <engine/server/name_ban.h>
+
+TEST(NameBan, Empty)
+{
+	ASSERT_FALSE(IsNameBanned("", 0, 0));
+	ASSERT_FALSE(IsNameBanned("abc", 0, 0));
+}
+
+TEST(NameBan, Equality)
+{
+	CNameBan Abc0("abc", 0);
+	ASSERT_TRUE(IsNameBanned("abc", &Abc0, 1));
+	ASSERT_TRUE(IsNameBanned("   abc", &Abc0, 1));
+	ASSERT_TRUE(IsNameBanned("abc   ", &Abc0, 1));
+	ASSERT_TRUE(IsNameBanned("abc                   foo", &Abc0, 1)); // Maximum name length.
+	ASSERT_FALSE(IsNameBanned("def", &Abc0, 1));
+	ASSERT_FALSE(IsNameBanned("abcdef", &Abc0, 1));
+}

src/test/str.cpp

@@ -17,3 +17,23 @@ TEST(Str, Dist)
 	EXPECT_EQ(str_utf8_dist("flaw", "lawn"), 2);
 	EXPECT_EQ(str_utf8_dist("saturday", "sunday"), 3);
 }
+
+TEST(Str, Utf8Isspace)
+{
+	EXPECT_TRUE(str_utf8_isspace(' '));
+	EXPECT_FALSE(str_utf8_isspace('a'));
+}
+
+TEST(Str, Utf8SkipWhitespaces)
+{
+	EXPECT_STREQ(str_utf8_skip_whitespaces("abc"), "abc");
+	EXPECT_STREQ(str_utf8_skip_whitespaces("abc   "), "abc   ");
+	EXPECT_STREQ(str_utf8_skip_whitespaces("    abc"), "abc");
+}
+
+TEST(Str, Utf8TrimRight)
+{
+	char A1[] = "abc"; str_utf8_trim_right(A1); EXPECT_STREQ(A1, "abc");
+	char A2[] = "   abc"; str_utf8_trim_right(A2); EXPECT_STREQ(A2, "   abc");
+	char A3[] = "abc   "; str_utf8_trim_right(A3); EXPECT_STREQ(A3, "abc");
+}