From 897eaa0761e1acaf7c9181f5a18d6b34ce6e2c76 Mon Sep 17 00:00:00 2001 From: Barnaby <22575741+barnabwhy@users.noreply.github.com> Date: Fri, 11 Mar 2022 03:23:24 +0000 Subject: 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 --- .../scriptservertoclientstringcommand.cpp | 7 +++--- NorthstarDedicatedTest/serverauthentication.cpp | 28 ++++++++++++++++++++++ NorthstarDedicatedTest/serverauthentication.h | 2 ++ 3 files changed, 34 insertions(+), 3 deletions(-) (limited to 'NorthstarDedicatedTest') 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 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); -- cgit v1.2.3