[CS:GO] Safest Connection Hook? Experiencing Data Loss Bug

aexi0n · **Author**

Currently my server saves/loads player information from a MySQL database. I have taken all the precautions to make sure the strings have been escaped, that I'm only passing the client user id, etc, so that data is secure and consistent. However, recently I've had my system flawed because users are occasionally getting their levels reset. I've looked through the code, and two things stand out to me as a possiblity:

1. I'm currently using the public OnClientPostAdminCheck(client) to retrieve the players data. The reset seems to happen when a player connects, but never fully makes it into the server.

2. I'm also updating the players data in the DB everytime the disconnect. It could be related to this aswell. In both occurences, I check for if(!IsFakeClient(client) && client != 0), so I'm not sure what could be causing this bug to occur?

Any advice is appreciated, and I look forward your guy's input.

Here is the code if that will help:

Code:

public OnClientPostAdminCheck(client)
{
	if(!IsFakeClient(client) && client != 0)
	{
		GetPlayerData(client);
		
		decl String:name[MAX_NAME_LENGTH];
		GetClientName(client, name, sizeof(name));
		
		g_timers[client][Enabled] = false;
		SetLevelTag(client);
		
		PrintToChatAll("Player \x01\x04%s \x01\x07has joined the server!", name);
	}
}

and

Code:

public OnClientDisconnect(client)
{
	PrintToChatAll("Player \x01\x04%N \x01\x07has disconnected.", client);

        UpdatePlayer(client);
}

Neuro Toxin

OnClientPostAdminCheck - is called once the player's steamid is fully validated by steam servers. If a player cant be validated they will be kicked with a message, No Steam Login.

Your may want to add a check to see if a player is authorized (IsClientAuthorized) before allowing any level code that is outside of OnClientPostAdminCheck.

Can you show your routines for UpdatePlayer, GetPlayerData and SetLevelTag? I'm assuming your not using asynchronous SQL calls which you should also avoid.

Drixevel

If you're needing to save/load data that involves SteamIDs then use 'OnClientAuthorized'.
If you're needing to save/load data revolving clients and their administration, use 'OnClientPostAdminCheck'.

Otherwise you should use 'OnClientPutInServer' for most of the work for clients you want to use. You could technically use this for everything but it depends on what you need to do.

Also, to note: You don't need to check if the client is 0 if you're already checking if they're a fake client.

Impact123 · *Last edited by Impact123; 12-29-2014 at 19:11.*

Quote:

Originally Posted by r3dw3r3w0lf

You don't need to check if the client is 0 if you're already checking if they're a fake client.

Yes you do.

As you can see, the order of the checks should be switched, because IsFakeClient would throw an error if an index below 1 is passed.
I'd usually skip the 0 check in this special case because the OnClientPostAdminCheck forward shouldn't be triggered with a client index below 1 at all.

It could help if you can post more code, especially what you're doing in your GetPlayerData, SetlevelTag and UpdatePlayer functions.

Neuro Toxin · *Last edited by Neuro Toxin; 12-29-2014 at 19:27.*

This could be your problem.

Your code in OnClientPostAdminCheck could be erroring out, causing sourcemod to not complete the OnClientPostAdminCheck chain.

Leaving the client in question as not authorised as far as sourcemod is concerned.

Do your error logs show any problems?

They are located at addons/sourcemod/logs/error_[date].log

Edit: From what I can tell, using 'OnClientPutInServer' is a better option also as you dont need to override admin flags or groups.

Drixevel · *Last edited by Drixevel; 12-29-2014 at 19:35.*

Quote:

Originally Posted by Impact123

Yes you do.

As you can see, the order of the checks should be switched, because IsFakeClient would throw an error if an index below 1 is passed.
I'd usually skip the 0 check in this special case because the OnClientPostAdminCheck forward shouldn't be triggered with a client index below 1 at all.

It could help if you can post more code, especially what you're doing in your GetPlayerData, SetlevelTag and UpdatePlayer functions.

That's kind of what I meant.. I mostly only do checks if they're console for commands.

friagram

Use onclientauthorized since you will be using steamid for the primary keys. Post admin is slow and often delayed. Client 0 never fires authorized iirc, along with other client callbacks, but bots will.

aexi0n

Sorry for the lack of other code, didn't even think about posting the other segments.
I'll change it to OnClientAuthorized(), and see if I can spot any level resets. Originally I think I had it on that awhile back, but that was when I was using keyvalues and .txts, and it created some unwanted bugs.

Code:

CreateDB(&Handle:DbHNDL)
{
	SQL_TConnect(SQL_OnConnect, "default");	
}
//Connection Callback
public SQL_OnConnect(Handle:owner, Handle:hndl, const String:Error[], any:data) 
{ 
	if (hndl == INVALID_HANDLE) 
	{ 
		//It didn't work, so we log the error 
		PrintToServer("SQL ERROR: %s", Error);
	} 
	else 
	{ 
		db = hndl;
		PrintToServer("[SQL] Connection Successful!");
		SQL_CreateTables();
	} 
}
//Create Database Tables
SQL_CreateTables()
{
	decl String:Query[255];
	Format(Query, sizeof(Query), "CREATE TABLE IF NOT EXISTS playerdata (steamid VARCHAR(64), name TEXT, level TINYINT, XP INT);");
	SQL_TQuery(db, SQL_OnCreatedTable, Query);
}
//Create Tables Callback
public SQL_OnCreatedTable(Handle:owner, Handle:hndl, const String:Error[], any:data) 
{ 
	if (hndl == INVALID_HANDLE) 
	{ 
		PrintToServer("SQL ERROR: %s", Error); 
	}
} 


//New Player - Add To Database
public CreatePlayer(client)
{
	new userid = GetClientUserId(client);
	
	decl String:steamid[32], String:name[(MAX_NAME_LENGTH + 1) * 2], String:Query[255], String:nameBuffer[MAX_NAME_LENGTH];
	
	GetClientAuthString(client, steamid, sizeof(steamid));
	GetClientName(client, nameBuffer, sizeof(nameBuffer));
	
	SQL_EscapeString(db, nameBuffer, name, sizeof(name)); 
	
	Format(Query, sizeof(Query), "INSERT INTO playerdata (steamid, name, level, XP) VALUES ('%s', '%s', '1', '0')", steamid, name);
		
	SQL_TQuery(db, SQL_ErrorCallback, Query, userid);
		
	GetPlayerData(client);
}
//Existing Player - Update Database
public UpdatePlayer(client)
{
	new userid = GetClientUserId(client);
	
	decl String:steamid[32], String:Query[255];
	
	GetClientAuthString(client, steamid, sizeof(steamid));
	
	Format(Query, sizeof(Query), "UPDATE playerdata SET level='%i', XP='%i' WHERE (steamid='%s')", Level[client], XP[client], steamid);
		
	SQL_TQuery(db, SQL_ErrorCallback, Query, userid);
		
	GetPlayerData(client);
}
//General Error Callback
public SQL_ErrorCallback(Handle:owner, Handle:hndl, const String:Error[], any:data)
{
	new client = GetClientOfUserId(data);

	if(hndl == INVALID_HANDLE)
	{
		PrintToServer("SQL ERROR: %s", Error);
	}

	if (client == 0)
	{
		return;
	}
}


//Retrieve Player Data
public GetPlayerData(client)
{	
	new userid = GetClientUserId(client);
	decl String:Query[255], String:steamid[32];
	GetClientAuthString(client, steamid, sizeof(steamid));
	
	Format(Query, sizeof(Query), "SELECT level, XP FROM playerdata WHERE (steamid='%s')", steamid);
	
	SQL_TQuery(db, SQL_GetPlayerData, Query, userid);

}
//Retrieve Player Data Callback
public SQL_GetPlayerData(Handle:owner, Handle:hndl, const String:Error[], any:data)
{
	new client = GetClientOfUserId(data);

	if(hndl == INVALID_HANDLE)
	{
		PrintToServer("SQL ERROR: %s", Error);
	}
	
	if (client == 0)
	{
		return;
	}

	if(!SQL_FetchRow(hndl))
	{
		CreatePlayer(client);
	}
	else
	{
		Level[client] = SQL_FetchInt(hndl, 0);
		XP[client] = SQL_FetchInt(hndl, 1);
		new L = Level[client];
		XP_Needed[client] = ((L * L + L + 3) * 4);
	}	
}

Neuro Toxin

1. You have a memory leak. You need to close the query handles.

Code:

public SQL_ErrorCallback(Handle:owner, Handle:hndl, const String:Error[], any:data)
{
    new client = GetClientOfUserId(data);
 
    if(hndl == INVALID_HANDLE)
    {
        PrintToServer("SQL ERROR: %s", Error);
    }
 
    if (client == 0)
    {
        return;
    }
}

change this to:

Code:

public SQL_ErrorCallback(Handle:owner, Handle:hndl, const String:Error[], any:data)
{
    new client = GetClientOfUserId(data);
 
    if(hndl == INVALID_HANDLE)
    {
        PrintToServer("SQL ERROR: %s", Error);
        return;
    }
 
    CloseHandle(hndl);
    return;
}

2. This also leaks in SQL_OnCreatedTable and SQL_GetPlayerData

------------------------------------

Please also check error logs.

Im confident that an error is occuring that is breaking the call chain.

Neuro Toxin

Also,

I just noticed, you pass the clients userid into the steamid fields in your database.

You need to pass the steamid as the userid is not consistant.