[FIX][C++] SQL Injection in Messenger and Guild

[VegaS ♆]

Originally Posted by deco016

Lol this is my code from ...

I said I like it I did not say it's mine, you misunderstand?



I said I like your method and I referred to the two functions for Fixx Query / DirectQuery

Thanks anyway for that Fixx.

[SandMann016]

/libraries/libsql/AsyncSQL.cpp:135:old

if (!mysql_real_connect(&m_hDB, m_stHost.c_str(), m_stUser.c_str(), m_stPassword.c_str(), m_stDB.c_str(), m_iPort, NULL, CLIENT_MULTI_STATEMENTS))

/libraries/libsql/AsyncSQL.cpp:135:new

if (!mysql_real_connect(&m_hDB, m_stHost.c_str(), m_stUser.c_str(), m_stPassword.c_str(), m_stDB.c_str(), m_iPort, NULL, NULL))

[deco016]

Originally Posted by SandMann016

/libraries/libsql/AsyncSQL.cpp:135ld

if (!mysql_real_connect(&m_hDB, m_stHost.c_str(), m_stUser.c_str(), m_stPassword.c_str(), m_stDB.c_str(), m_iPort, NULL, CLIENT_MULTI_STATEMENTS))

/libraries/libsql/AsyncSQL.cpp:135:new

if (!mysql_real_connect(&m_hDB, m_stHost.c_str(), m_stUser.c_str(), m_stPassword.c_str(), m_stDB.c_str(), m_iPort, NULL, NULL))

My solution to this problem does exactly the same as yours

[xworldx]

Hello, please diff for 2089m

[Ken™]

Originally Posted by [SGA]Vegas

I would prefer a solution like this:

SQLMsg * DBManager::DirectQuery(const char * c_pszFormat, ...)
{
    char szQuery[4096];
    va_list args;
    va_start(args, c_pszFormat);
    vsnprintf(szQuery, sizeof(szQuery), c_pszFormat, args);
    va_end(args);
    std::string sQuery(szQuery);
    return m_sql_direct.DirectQuery(sQuery.substr(0, sQuery.find_first_of(";") == -1 ? sQuery.length() : sQuery.find_first_of(";")).c_str());
}

and

void DBManager::Query(const char * c_pszFormat, ...)
{
    char szQuery[4096];
    va_list args;

    va_start(args, c_pszFormat);
    vsnprintf(szQuery, sizeof(szQuery), c_pszFormat, args);
    va_end(args);
    std::string sQuery(szQuery);

    m_sql.AsyncQuery(sQuery.substr(0,sQuery.find_first_of(";")==-1?sQuery.length(): sQuery.find_first_of(";")).c_str());
}

At first, you don't have to use DBManager::Instance().DirectQuery. You just need to put a small condition in the function. Here is my function with normal and ban query.

With ban query

void MessengerManager::RemoveFromList(MessengerManager::keyA account, MessengerManager::keyA companion)
{
	if (companion.empty())
		return;

	// Second fix
	if (m_Relation[account].find(companion) == m_Relation[account].end() || m_InverseRelation[companion].find(account) == m_InverseRelation[companion].end())
	{
		LPCHARACTER ch = CHARACTER_MANAGER::Instance().FindPC(account.c_str());
		if (ch)
		{
			sys_err("MessengerManager::RemoveFromList: %s tries to use messenger sql injection", ch->GetName());
			DBManager::Instance().DirectQuery("UPDATE account.account SET status = 'BAN' WHERE id = %u", ch->GetAID());
			if (ch->GetDesc())
				ch->GetDesc()->DelayedDisconnect(3);
		}
		else
			sys_err("MessengerManager::RemoveFromList: Omg! The ghost tried to use this function!");
		return;
	}

	sys_log(1, "MessengerManager::RemoveFromList: Remove %s %s", account.c_str(), companion.c_str());
	DBManager::instance().Query("DELETE FROM messenger_list%s WHERE account='%s' AND companion = '%s'", get_table_postfix(), account.c_str(), companion.c_str());
	__RemoveFromList(account, companion);
	TPacketGGMessenger p2ppck;
	p2ppck.bHeader = HEADER_GG_MESSENGER_REMOVE;
	strlcpy(p2ppck.szAccount, account.c_str(), sizeof(p2ppck.szAccount));
	strlcpy(p2ppck.szCompanion, companion.c_str(), sizeof(p2ppck.szCompanion));;
	P2P_MANAGER::instance().Send(&p2ppck, sizeof(TPacketGGMessenger));
}

With normal

void MessengerManager::RemoveFromList(MessengerManager::keyA account, MessengerManager::keyA companion)
{
	if (companion.empty())
		return;

	// Second fix
	if (m_Relation[account].find(companion) == m_Relation[account].end() || m_InverseRelation[companion].find(account) == m_InverseRelation[companion].end())
	{
		LPCHARACTER ch = CHARACTER_MANAGER::Instance().FindPC(account.c_str());
		if (ch)
		{
			sys_err("MessengerManager::RemoveFromList: %s tries to use messenger sql injection", ch->GetName());

			if (ch->GetDesc())
				ch->GetDesc()->DelayedDisconnect(3);
		}
		else
			sys_err("MessengerManager::RemoveFromList: Omg! The ghost tried to use this function!");
		return;
	}

	sys_log(1, "MessengerManager::RemoveFromList: Remove %s %s", account.c_str(), companion.c_str());
	DBManager::instance().Query("DELETE FROM messenger_list%s WHERE account='%s' AND companion = '%s'", get_table_postfix(), account.c_str(), companion.c_str());
	__RemoveFromList(account, companion);
	TPacketGGMessenger p2ppck;
	p2ppck.bHeader = HEADER_GG_MESSENGER_REMOVE;
	strlcpy(p2ppck.szAccount, account.c_str(), sizeof(p2ppck.szAccount));
	strlcpy(p2ppck.szCompanion, companion.c_str(), sizeof(p2ppck.szCompanion));;
	P2P_MANAGER::instance().Send(&p2ppck, sizeof(TPacketGGMessenger));
}

About the older game versions.

You can make a so file for this or you can use a diff but If you want to ban who tries to use this SQL injection, you will need to a so file.

Kind Regards ~ Ken

[matalaj]

Please upload .mix/binary

[skytech63]

Thanks

TR FIX:

[I´m Raylee]

Thank´s for the fix!

Best regards
Raylee

[oceanusPT]

its possible make onde diff for 34k ??

[°~Dennis~°]

Ich habe da ein Problem mit der Funktion CreateGuild.
Wenn der User eine Gilde erstellt und einen bereits vorhanden namen verwendet erstellt er die Gilde und der User ist der Admin der alten breits vorhanden Gilde

Das ist die Funktion:

PHP Code:

DWORD CGuildManager::CreateGuild(TGuildCreateParameter& gcp)
{
    if (!gcp.master)
        return 0;

    if (!check_name(gcp.name))
    {
        gcp.master->ChatPacket(CHAT_TYPE_INFO, LC_TEXT("187"));
        return 0;
    }
    static char __escape_name[GUILD_NAME_MAX_LEN * 2 + 1];
    DBManager::instance().EscapeString(__escape_name, sizeof(__escape_name), static_cast<const char *>(gcp.name), sizeof(gcp.name));

    std::auto_ptr<SQLMsg> pmsg(DBManager::instance().DirectQuery("SELECT COUNT(*) FROM guild%s WHERE name = '%s'",
        get_table_postfix(), __escape_name));

    if (pmsg->Get()->uiNumRows > 0)
    {
        MYSQL_ROW row = mysql_fetch_row(pmsg->Get()->pSQLResult);

        if (!(row[0] && row[0][0] == '0'))
        {
            gcp.master->ChatPacket(CHAT_TYPE_INFO, LC_TEXT("188"));
            return 0;
        }
    }
    else
    {
        gcp.master->ChatPacket(CHAT_TYPE_INFO, LC_TEXT("189"));
        return 0;
    }

    CGuild * pg = M2_NEW CGuild(gcp);
    m_mapGuild.insert(std::make_pair(pg->GetID(), pg));
    return pg->GetID();
} 


Jemand eine Lösung vielleicht ?

[R0bo7]

Originally Posted by °~Dennis~°

Ich habe da ein Problem mit der Funktion CreateGuild.
Wenn der User eine Gilde erstellt und einen bereits vorhanden namen verwendet erstellt er die Gilde und der User ist der Admin der alten breits vorhanden Gilde

Das ist die Funktion:

PHP Code:

DWORD CGuildManager::CreateGuild(TGuildCreateParameter& gcp)
{
    if (!gcp.master)
        return 0;

    if (!check_name(gcp.name))
    {
        gcp.master->ChatPacket(CHAT_TYPE_INFO, LC_TEXT("187"));
        return 0;
    }
    static char __escape_name[GUILD_NAME_MAX_LEN * 2 + 1];
    DBManager::instance().EscapeString(__escape_name, sizeof(__escape_name), static_cast<const char *>(gcp.name), sizeof(gcp.name));

    std::auto_ptr<SQLMsg> pmsg(DBManager::instance().DirectQuery("SELECT COUNT(*) FROM guild%s WHERE name = '%s'",
        get_table_postfix(), __escape_name));

    if (pmsg->Get()->uiNumRows > 0)
    {
        MYSQL_ROW row = mysql_fetch_row(pmsg->Get()->pSQLResult);

        if (!(row[0] && row[0][0] == '0'))
        {
            gcp.master->ChatPacket(CHAT_TYPE_INFO, LC_TEXT("188"));
            return 0;
        }
    }
    else
    {
        gcp.master->ChatPacket(CHAT_TYPE_INFO, LC_TEXT("189"));
        return 0;
    }

    CGuild * pg = M2_NEW CGuild(gcp);
    m_mapGuild.insert(std::make_pair(pg->GetID(), pg));
    return pg->GetID();
} 


Jemand eine Lösung vielleicht ?

Gehört rein.

[naosou]

Originally Posted by oceanusPT

its possible make onde diff for 34k ??

This difference file is created by IdaPro

game
002EB6F5: 01 00

This difference file is created by IdaPro

db
00082F15: 01 00

[Bercol]

is it possible to make a dif for 67k?

[iMer]

Dif to kill the function for 2089M (to allow for a safe transition period)
THIS WILL DISABLE DELETING FRIENDS

game_2089M
0010F5C3: 31 90
0010F5C4: C0 90
0010F5C5: 8B 90
0010F5C6: 03 90
0010F5C7: 8B 90
0010F5C8: 50 90
0010F5C9: F4 90
0010F5CA: 85 90
0010F5CB: D2 90
0010F5CC: 75 90
0010F5CD: 22 90


(why are people even using such an old version still? ugh)

[miguelmig]

I don't see why you're making fixes for the CGuildManager::CreateGuild, there's already
the
!check_name(gcp.name)

which prevents any special character that isn't a digit or alphanumerical