From a27c702b7d2189f80c5c441eb44a8a5b6922c538 Mon Sep 17 00:00:00 2001 From: GeckoEidechse <40122905+GeckoEidechse@users.noreply.github.com> Date: Fri, 1 Dec 2023 21:31:20 +0100 Subject: Revert "Move player auth to `CServer::ConnectClient` (#548)" (#610) This reverts commit 17217a39681c7fed35bee95195bdba7eaf508911 (PR #548) which introduced a regression allowing auth to progress further than intended. --- NorthstarDLL/server/auth/serverauthentication.cpp | 89 +++++++++-------------- NorthstarDLL/server/auth/serverauthentication.h | 4 +- 2 files changed, 37 insertions(+), 56 deletions(-) diff --git a/NorthstarDLL/server/auth/serverauthentication.cpp b/NorthstarDLL/server/auth/serverauthentication.cpp index 14e166bd..d5653dcc 100644 --- a/NorthstarDLL/server/auth/serverauthentication.cpp +++ b/NorthstarDLL/server/auth/serverauthentication.cpp @@ -25,9 +25,6 @@ AUTOHOOK_INIT() ServerAuthenticationManager* g_pServerAuthentication; CBaseServer__RejectConnectionType CBaseServer__RejectConnection; -typedef void (*CBaseServer__PushDisconnectReasonType)(void*, int32_t, void*, const char*); -CBaseServer__PushDisconnectReasonType CBaseServer__PushDisconnectReason; - void ServerAuthenticationManager::AddRemotePlayer(std::string token, uint64_t uid, std::string username, std::string pdata) { std::string uidS = std::to_string(uid); @@ -104,7 +101,7 @@ bool ServerAuthenticationManager::IsDuplicateAccount(R2::CBaseClient* pPlayer, c return false; } -bool ServerAuthenticationManager::CheckAuthentication(R2::CBaseClient* pPlayer, uint64_t iUid, const char* pAuthToken) +bool ServerAuthenticationManager::CheckAuthentication(R2::CBaseClient* pPlayer, uint64_t iUid, char* pAuthToken) { std::string sUid = std::to_string(iUid); @@ -129,7 +126,7 @@ bool ServerAuthenticationManager::CheckAuthentication(R2::CBaseClient* pPlayer, return false; } -void ServerAuthenticationManager::AuthenticatePlayer(R2::CBaseClient* pPlayer, uint64_t iUid, const char* pAuthToken) +void ServerAuthenticationManager::AuthenticatePlayer(R2::CBaseClient* pPlayer, uint64_t iUid, char* pAuthToken) { // for bot players, generate a new uid if (pPlayer->m_bFakePlayer) @@ -205,9 +202,14 @@ void ServerAuthenticationManager::WritePersistentData(R2::CBaseClient* pPlayer) // auth hooks +// store these in vars so we can use them in CBaseClient::Connect +// this is fine because ptrs won't decay by the time we use this, just don't use it outside of calls from cbaseclient::connectclient +char* pNextPlayerToken; +uint64_t iNextPlayerUid; + // clang-format off AUTOHOOK(CBaseServer__ConnectClient, engine.dll + 0x114430, -R2::CBaseClient*,, ( +void*,, ( void* self, void* addr, void* a3, @@ -227,71 +229,51 @@ R2::CBaseClient*,, ( uint32_t a17)) // clang-format on { - // try to connect the client to get a client object - R2::CBaseClient* client = - CBaseServer__ConnectClient(self, addr, a3, a4, a5, a6, a7, playerName, serverFilter, a10, a11, a12, a13, a14, uid, a16, a17); - if (!client) - return nullptr; + // auth tokens are sent with serverfilter, can't be accessed from player struct to my knowledge, so have to do this here + pNextPlayerToken = serverFilter; + iNextPlayerUid = uid; + return CBaseServer__ConnectClient(self, addr, a3, a4, a5, a6, a7, playerName, serverFilter, a10, a11, a12, a13, a14, uid, a16, a17); +} + +ConVar* Cvar_ns_allowuserclantags; + +// clang-format off +AUTOHOOK(CBaseClient__Connect, engine.dll + 0x101740, +bool,, (R2::CBaseClient* self, char* pName, void* pNetChannel, char bFakePlayer, void* a5, char pDisconnectReason[256], void* a7)) +// clang-format on +{ const char* pAuthenticationFailure = nullptr; char pVerifiedName[64]; - const char* authToken = serverFilter; - if (!client->m_bFakePlayer) + if (!bFakePlayer) { - if (!g_pServerAuthentication->VerifyPlayerName(authToken, playerName, pVerifiedName)) + if (!g_pServerAuthentication->VerifyPlayerName(pNextPlayerToken, pName, pVerifiedName)) pAuthenticationFailure = "Invalid Name."; - else if (!g_pBanSystem->IsUIDAllowed(uid)) + else if (!g_pBanSystem->IsUIDAllowed(iNextPlayerUid)) pAuthenticationFailure = "Banned From server."; - else if (!g_pServerAuthentication->CheckAuthentication(client, uid, authToken)) + else if (!g_pServerAuthentication->CheckAuthentication(self, iNextPlayerUid, pNextPlayerToken)) pAuthenticationFailure = "Authentication Failed."; } - else - { - spdlog::error("A bot called CBaseServer__ConnectClient, should be impossible!"); - - CBaseServer__PushDisconnectReason(self, (int32_t)((uintptr_t)self + 0xc), addr, "A bot was init in CServer__ConnectClient"); - return nullptr; - } + else // need to copy name for bots still + strncpy_s(pVerifiedName, pName, 63); if (pAuthenticationFailure) { - spdlog::info("{}'s (uid {}) connection was rejected: \"{}\"", playerName, uid, pAuthenticationFailure); + spdlog::info("{}'s (uid {}) connection was rejected: \"{}\"", pName, iNextPlayerUid, pAuthenticationFailure); - CBaseServer__PushDisconnectReason(self, (int32_t)((uintptr_t)self + 0xc), addr, pAuthenticationFailure); - return nullptr; + strncpy_s(pDisconnectReason, 256, pAuthenticationFailure, 255); + return false; } - // write name into the client - strncpy_s(pVerifiedName, client->m_Name, 63); - - // we already know this player's authentication data is legit, actually write it to them now - g_pServerAuthentication->AuthenticatePlayer(client, uid, authToken); - - g_pServerAuthentication->AddPlayer(client, authToken); - g_pServerLimits->AddPlayer(client); -} - -ConVar* Cvar_ns_allowuserclantags; - -// clang-format off -AUTOHOOK(CBaseClient__Connect, engine.dll + 0x101740, -bool,, (R2::CBaseClient* self, char* pName, void* pNetChannel, char bFakePlayer, void* a5, char pDisconnectReason[256], void* a7)) -// clang-format on -{ - // only remains to count bots in player count, - // since bots take player slots and it will give if not counted a false player count on the server browser. - - if (!bFakePlayer) - return CBaseClient__Connect(self, pName, pNetChannel, bFakePlayer, a5, pDisconnectReason, a7); - - // try to actually connect the bot - if (!CBaseClient__Connect(self, pName, pNetChannel, bFakePlayer, a5, pDisconnectReason, a7)) + // try to actually connect the player + if (!CBaseClient__Connect(self, pVerifiedName, pNetChannel, bFakePlayer, a5, pDisconnectReason, a7)) return false; - g_pServerAuthentication->AuthenticatePlayer(self, 0, "0"); + // we already know this player's authentication data is legit, actually write it to them now + g_pServerAuthentication->AuthenticatePlayer(self, iNextPlayerUid, pNextPlayerToken); - g_pServerAuthentication->AddPlayer(self, "0"); + g_pServerAuthentication->AddPlayer(self, pNextPlayerToken); g_pServerLimits->AddPlayer(self); return true; @@ -387,7 +369,6 @@ ON_DLL_LOAD_RELIESON("engine.dll", ServerAuthentication, (ConCommand, ConVar), ( module.Offset(0x101012).Patch("E9 90 00"); CBaseServer__RejectConnection = module.Offset(0x1182E0).RCast(); - CBaseServer__PushDisconnectReason = module.Offset(0x1155D0).RCast(); if (Tier0::CommandLine()->CheckParm("-allowdupeaccounts")) { diff --git a/NorthstarDLL/server/auth/serverauthentication.h b/NorthstarDLL/server/auth/serverauthentication.h index 34680bd6..dd0e13af 100644 --- a/NorthstarDLL/server/auth/serverauthentication.h +++ b/NorthstarDLL/server/auth/serverauthentication.h @@ -48,9 +48,9 @@ class ServerAuthenticationManager bool VerifyPlayerName(const char* pAuthToken, const char* pName, char pOutVerifiedName[64]); bool IsDuplicateAccount(R2::CBaseClient* pPlayer, const char* pUid); - bool CheckAuthentication(R2::CBaseClient* pPlayer, uint64_t iUid, const char* pAuthToken); + bool CheckAuthentication(R2::CBaseClient* pPlayer, uint64_t iUid, char* pAuthToken); - void AuthenticatePlayer(R2::CBaseClient* pPlayer, uint64_t iUid, const char* pAuthToken); + void AuthenticatePlayer(R2::CBaseClient* pPlayer, uint64_t iUid, char* pAuthToken); bool RemovePlayerAuthData(R2::CBaseClient* pPlayer); void WritePersistentData(R2::CBaseClient* pPlayer); }; -- cgit v1.2.3