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

Page 3 of 6

12/18/2015 03:12 VegaS ♆#31

Quote:

Originally Posted by deco016

Lol this is my code from [Only registered and activated users can see links. Click Here To Register...] ...

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

[Only registered and activated users can see links. Click Here To Register...]

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

Thanks anyway for that Fixx.

12/18/2015 03:35 SandMann016#32

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

Code:

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

Code:

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))

12/18/2015 04:11 deco016#33

Quote:
Originally Posted by SandMann016
/libraries/libsql/AsyncSQL.cpp:135:old
Code:
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
Code:
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

12/18/2015 06:17 xworldx#34

Hello, please diff for 2089m

12/18/2015 12:04 Ken�#35

Quote:

Originally Posted by [SGA]Vegas

I would prefer a solution like this:

Code:

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

Code:

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

Code:

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

Code:

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

12/18/2015 12:40 matalaj#36

Please upload .mix/binary

12/18/2015 18:38 skytech63#37

Thanks

TR FIX: [Only registered and activated users can see links. Click Here To Register...]

12/18/2015 21:36 I�m Raylee#38

Thank�s for the fix! :)

Best regards
Raylee

12/19/2015 00:15 oceanusPT#39

its possible make onde diff for 34k ??

12/19/2015 04:01 �~Dennis~�#40

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 ?

12/19/2015 10:34 R0bo7#41

Quote:

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 [Only registered and activated users can see links. Click Here To Register...] rein.

12/19/2015 10:41 naosou#42

Quote:

Originally Posted by oceanusPT

its possible make onde diff for 34k ??

Code:

This difference file is created by IdaPro

game
002EB6F5: 01 00

Code:

This difference file is created by IdaPro

db
00082F15: 01 00

12/19/2015 11:31 Bercol#43

is it possible to make a dif for 67k?

12/19/2015 11:35 iMer#44

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)

12/19/2015 11:43 miguelmig#45

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

Page 3 of 6

Last »