Add check for player name correctness (#86)

* Add player auth failure when username is faked

* Fix the action error?

* Split name check into different function

allows unique error message

* Oops

* Fix allow_insecure and kick message

* Put it back cause i'm bad at programmig

* format

* Fix duplicated if statement maybe? Will need testing

* Fail open + change to authData.username

* Change name instead of kicking

* Format

* Remove unecessary borked name check

* fail open in VerifyPlayerName if missing authData

* Fix convar stuff

* limit name length when copying from master server

* 63 < 64

* please bob for the love of god this is like the third time i've had to do this

3 files changed, 34 insertions, 3 deletions

diff --git a/NorthstarDedicatedTest/scriptservertoclientstringcommand.cpp b/NorthstarDedicatedTest/scriptservertoclientstringcommand.cpp
index 58d92ec6..7bad2773 100644
--- a/NorthstarDedicatedTest/scriptservertoclientstringcommand.cpp
+++ b/NorthstarDedicatedTest/scriptservertoclientstringcommand.cpp
@@ -7,14 +7,15 @@
 
 void ConCommand_ns_script_servertoclientstringcommand(const CCommand& arg)
 {
-	if (g_ClientSquirrelManager->sqvm && g_ClientSquirrelManager->setupfunc("NSClientCodeCallback_RecievedServerToClientStringCommand") != SQRESULT_ERROR)
+	if (g_ClientSquirrelManager->sqvm &&
+		g_ClientSquirrelManager->setupfunc("NSClientCodeCallback_RecievedServerToClientStringCommand") != SQRESULT_ERROR)
 	{
 		g_ClientSquirrelManager->pusharg(arg.ArgS());
 		g_ClientSquirrelManager->call(1); // todo: doesn't throw or log errors from within this, probably not great behaviour
 	}
 }
 
-void InitialiseScriptServerToClientStringCommands(HMODULE baseAddress) 
+void InitialiseScriptServerToClientStringCommands(HMODULE baseAddress)
 {
 	if (IsDedicated())
 		return;
@@ -22,4 +23,4 @@ void InitialiseScriptServerToClientStringCommands(HMODULE baseAddress)
 	RegisterConCommand(
 		"ns_script_servertoclientstringcommand", ConCommand_ns_script_servertoclientstringcommand, "",
 		FCVAR_CLIENTDLL | FCVAR_SERVER_CAN_EXECUTE);
-}
\ No newline at end of file
+}
diff --git a/NorthstarDedicatedTest/serverauthentication.cpp b/NorthstarDedicatedTest/serverauthentication.cpp
index 44ee97f3..84a2bbeb 100644
--- a/NorthstarDedicatedTest/serverauthentication.cpp
+++ b/NorthstarDedicatedTest/serverauthentication.cpp
@@ -113,6 +113,9 @@ void ServerAuthenticationManager::StartPlayerAuthServer()
 					strncpy(newAuthData.uid, request.get_param_value("id").c_str(), sizeof(newAuthData.uid));
 					newAuthData.uid[sizeof(newAuthData.uid) - 1] = 0;
 
+					strncpy(newAuthData.username, request.get_param_value("username").c_str(), sizeof(newAuthData.username));
+					newAuthData.username[sizeof(newAuthData.username) - 1] = 0;
+
 					newAuthData.pdataSize = request.body.size();
 					newAuthData.pdata = new char[newAuthData.pdataSize];
 					memcpy(newAuthData.pdata, request.body.c_str(), newAuthData.pdataSize);
@@ -141,6 +144,27 @@ void ServerAuthenticationManager::StopPlayerAuthServer()
 	m_playerAuthServer.stop();
 }
 
+char* ServerAuthenticationManager::VerifyPlayerName(void* player, char* authToken, char* name)
+{
+	std::lock_guard<std::mutex> guard(m_authDataMutex);
+
+	if (!m_authData.empty() && m_authData.count(std::string(authToken)))
+	{
+		AuthData authData = m_authData[authToken];
+
+		bool nameAccepted = (!*authData.username || !strcmp(name, authData.username));
+
+		if (!nameAccepted && g_MasterServerManager->m_bRequireClientAuth && !CVar_ns_auth_allow_insecure->GetInt())
+		{
+			// limit name length to 64 characters just in case something changes, this technically shouldn't be needed given the master
+			// server gets usernames from origin but we have it just in case
+			strncpy(name, authData.username, 64);
+			name[63] = 0;
+		}
+	}
+	return name;
+}
+
 bool ServerAuthenticationManager::AuthenticatePlayer(void* player, int64_t uid, char* authToken)
 {
 	std::string strUid = std::to_string(uid);
@@ -151,6 +175,7 @@ bool ServerAuthenticationManager::AuthenticatePlayer(void* player, int64_t uid,
 	{
 		// use stored auth data
 		AuthData authData = m_authData[authToken];
+
 		if (!strcmp(strUid.c_str(), authData.uid)) // connecting client's uid is the same as auth's uid
 		{
 			authFail = false;
@@ -297,6 +322,9 @@ void* CBaseServer__ConnectClientHook(
 
 bool CBaseClient__ConnectHook(void* self, char* name, __int64 netchan_ptr_arg, char b_fake_player_arg, __int64 a5, char* Buffer, void* a7)
 {
+	// try changing name before all else
+	name = g_ServerAuthenticationManager->VerifyPlayerName(self, nextPlayerToken, name);
+
 	// try to auth player, dc if it fails
 	// we connect irregardless of auth, because returning bad from this function can fuck client state p bad
 	bool ret = CBaseClient__Connect(self, name, netchan_ptr_arg, b_fake_player_arg, a5, Buffer, a7);
diff --git a/NorthstarDedicatedTest/serverauthentication.h b/NorthstarDedicatedTest/serverauthentication.h
index 8ff5099e..06834d30 100644
--- a/NorthstarDedicatedTest/serverauthentication.h
+++ b/NorthstarDedicatedTest/serverauthentication.h
@@ -7,6 +7,7 @@
 struct AuthData
 {
 	char uid[33];
+	char username[64];
 
 	// pdata
 	char* pdata;
@@ -94,6 +95,7 @@ class ServerAuthenticationManager
 	void StartPlayerAuthServer();
 	void StopPlayerAuthServer();
 	bool AuthenticatePlayer(void* player, int64_t uid, char* authToken);
+	char* VerifyPlayerName(void* player, char* authToken, char* name);
 	bool RemovePlayerAuthData(void* player);
 	void WritePersistentData(void* player);
 	bool CheckPlayerChatRatelimit(void* player);