Guard SetCSteamID When Auth String Is Invalid

[heymykola]

This is a follow-up to PR #2397. While the IClient/netchan guard resolved the initial crash, the server is still crashing during CPlayer::Initialize() at the UpdateAuthIds() call on a non-stock Source 2007 engine fork.

This change simply adds a small defensive guard to the reads performed in that path so they are treated as unset instead of triggering an access violation.

[heymykola]

Crash ID: N66Z-JZGG-6QEI

sourcemod.2.ep2.dll!CPlayer::Initialize(char const *,char const *,edict_t *) [PlayerManager.cpp:2062 + 0x7] 

sourcemod.2.ep2.dll!PlayerManager::OnClientConnect(edict_t *,char const *,char const *,char *,int) [PlayerManager.cpp:518 + 0x10] 

sourcemod.2.ep2.dll!__SourceHook_FHCls_IServerGameClientsClientConnect0::Func(edict_t *,char const *,char const *,char *,int) [PlayerManager.cpp:71 + 0x89]

1.4.8 and 1.5.2 were the last SM versions that worked on Kuma, because they didn’t depend on SteamID plumbing at connect time. My concern right now is if these two lines are merged, I don't yet know if inside UpdateAuthIds(), there are calls that could crash of Kuma's engine returns invalid pointer or string.

[Kenzzer]

I'm going to need clarification on what you meant by

via g_pGameConf, which is not guaranteed to be available on some Source 2007–based engine forks

g_pGameConf is absolutely available by the time the hook on IServerGameClients::ClientConnect. The only reason it wouldn't be is if the file was missing.

Loaded here as sourcemod boots up :

sourcemod/core/logic/GameConfigs.cpp

Lines 1135 to 1139 in 8907569

	void GameConfigManager::OnSourceModStartup(bool late)
	{
	m_gameBinPathManager.Init();
	LoadGameConfigFile("core.games", &g_pGameConf, NULL, 0);

Retrieved here as sourcemod initializes itself :

sourcemod/core/sourcemod.cpp

Lines 313 to 321 in 8907569

	g_pGameConf = logicore.GetCoreGameConfig();
	sCoreProviderImpl.InitializeHooks();
	/* Notify! */
	pBase = SMGlobalClass::head;
	while (pBase)
	{
	pBase->OnSourceModAllInitialized();

Definitively available by the time OnSourceModAllInitialized is invoked and the hook gets setup.

sourcemod/core/PlayerManager.cpp

Lines 165 to 167 in e050532

	void PlayerManager::OnSourceModAllInitialized()
	{
	SH_ADD_HOOK(IServerGameClients, ClientConnect, serverClients, SH_MEMBER(this, &PlayerManager::OnClientConnect), false);

[heymykola]

Gamedata loads correctly, but on the Kuma\War 2 fork an access violation occurs in CPlayer::Initialize() → UpdateAuthIds() during ClientConnect.

This change would guard the g_pGameConf->GetKeyValue() reads on that path. Behavior would be unchanged on standard engines, and it prevents a crash if those reads are hit in an unexpected state on this fork.

This code path did not exist in SourceMod 1.4 and earlier 1.5, where UpdateAuthIds() was not called during ClientConnect. In newer releases, this logic runs earlier during client initialization, which is where the crash occurs.

In addition to the existing guards around IClient access and g_pGameConf->GetKeyValue() reads, would you prefer handling this by deferring the early UpdateAuthIds() call instead? This could follow the same pattern as the IClient netchan guard: default behavior remains unchanged, but forks where the engine state isn’t stable during ClientConnect could opt out of running UpdateAuthIds() at that point via common.games.txt?

[asherkin]

Please add some logging to the local build you used while testing this showing the initialisation order.

Guards like this for our own interfaces generally look like papering over the true issue, which we're not a fan of as they seriously impact future refactoring.

Don't forget to also rebase this branch on main so the build tests can run.

[heymykola]

Guards removed, logging added to each of the 3 lines crash.limetech caught - as well as OnClientConnect, CPlayer:Initialize, and UpdateAuthIds sections using the following:

/* To Be Removed: Debugging Purposes */
if (sm_debug_connect.GetBool())
	logger->LogMessage("");

[Kenzzer]

We're not going to merge a PR that just adds logging. Asherkin meant for you to use that locally to help debugging your issue.

If there really is a bug, then submit the fix. Otherwise this PR's scope as well as your initial fix are both invalid.

[heymykola]

Thank you for your patience, as I am both new to GetHub and local builds. While debugging the player-connect crash, I added logging to show through PlayerManager.cpp - and the logs showed the server crashing inside CPlayer::SetCSteamID() when engine->GetClientSteamID() was called while the auth string was still invalid.

My commit adds a minimal guard to skip the SteamID query until a valid auth string is available. Behavior for normal Steam games is unchanged. I confirmed this fixes server crash on player entry after building and replacing sourcemod.2.ep2.dll.

PlayerManager::OnClientConnect -> enter edict=11310024 name=Unknown addr=203.0.113.42:45181
PlayerManager::OnClientConnect -> client=1 maxrejectlen=128
PlayerManager::OnClientConnect -> PlayersSinceActive=1 already_connected=0
PlayerManager::OnClientConnect -> Initialize begin client=1
CPlayer::Initialize -> enter name=Unknown ip=203.0.113.42:45181 edict=11310024
CPlayer::Initialize -> idx=1 serial=1 ip_no_port=203.0.113.42
CPlayer::Initialize -> iclient=0x1E2584FC
CPlayer::UpdateAuthIds -> enter idx=1 IsAuthorized=0 IsFake=0 SteamIdValid=0 auth=
CPlayer::SetEngineString -> enter idx=1 current=
CPlayer::SetEngineString -> authstr=UNKNOWN current=
CPlayer::SetEngineString -> updated idx=1 new=UNKNOWN
CPlayer::SetCSteamID -> enter idx=1 fake=0 steamIdValid(cur)=0 auth=UNKNOWN