[Fixx] Wipe Bugg

05/13/2012 11:59 Razzer'#1
Hey,
habe einen SQL-Injection Bug gefunden , dieser ist in diesen Funktionen:
PHP Code:
LPTSTR CAr::ReadString( LPTSTR lpsz ) 
Einfach meine Funktion durch eure Ersetzten.
PHP Code:
LPTSTR CAr::ReadString( LPTSTR lpsz )
{
    int nLen;
    *this >> nLen;
    Read( lpsz, sizeof(TCHAR) * nLen );
    for( int i=0;i<nLen;i++ )
#ifdef __CLIENT
        if( lpsz[i] == '\x60' )
            lpsz[i] = '\x27';
#else // __CLIENT
        if( lpsz[i] == '\x27' )
            lpsz[i] = '\x60';
#endif // __CLIENT
    lpsz[nLen] = '\0';
    return lpsz;
}

LPTSTR CAr::ReadString( LPTSTR lpsz, int nBufSize )
{
    int nLen;
    *this >> nLen;

    int nReadable    = nBufSize - 1;
    if( nLen > nReadable || nLen < 0 )
    {
        memset( (void*)lpsz, 0, nBufSize );
        m_lpBufCur    = m_lpBufMax;
        return lpsz;
    }
    else
    {
        Read( lpsz, sizeof(TCHAR) * nLen );
        for( int i=0;i<nLen;i++ )
#ifdef __CLIENT
            if( lpsz[i] == '\x60' )
                lpsz[i] = '\x27';
#else // __CLIENT
            if( lpsz[i] == '\x27' )
                lpsz[i] = '\x60';
#endif // __CLIENT

        lpsz[nLen] = '\0';
        return lpsz;
    }
} 
Dieser Fixx ist nur Clientsided.
Jetzt sollte es nicht mehr möglich sein , durch den Client die DB zu Wipen .
Bzw. durch irgendwelche SQL-Injections Verfahren versuchen die DB zu schrotten .

Wo kann man bei dem Client SQL-INJECTION anwenden?
A:Ganz einfach , wo man seine ACCDATEN + PW eingibt.

Lg
Razzer'
05/13/2012 12:01 Ultimate™#2
mehr infos dazu ?
ich würde nichts einbauen wo keine richtige information bei ist wäre mir zu doof
05/13/2012 12:02 Razzer'#3
Habe was drunter geschrieben.


Lg
Razzer
05/13/2012 12:42 Krustenkäse#4
Außerdem ist fraglich, wie nötig dieser Fix ist...
Was er macht, ist nämlich einfach die Anführungszeichen('), die gesendet werden in escapezeichen (\) umzuwandeln...
ist eig ziemlich unnötig und wenn man chattet stört es sogar
unnötig ist es deswegen, weil die logindaten erstmal an den certifier gesendet werden, wo sie dann in einer funktion verarbeitet werden, wo unter anderem das hier drin steht:
Code:
    if( pszAccount[0] == '\0' || StringFind( pszAccount, '\'' ) >= 0 || StringFind( pszPwd, '\'' ) >= 0 )
    {
        DestroyPlayer( dpid );
        return;
    }
von daher würde die Injection erst gar nicht zum DB-Server kommen^^
05/13/2012 13:00 Xefiroid#5
wenn du sachen wie z.b. ';delete character_01_tbl;-- eingibst kommt da dieses fenster welches dir sagt das das password/account zu lang ist.
05/13/2012 13:23 paddelx3#6
Quote:
Originally Posted by Xefiroid View Post
wenn du sachen wie z.b. ';delete character_01_tbl;-- eingibst kommt da dieses fenster welches dir sagt das das password/account zu lang ist.
es gibt noch unzählige andere SQL Injectionen. ;)
Ansonsten eig nen netter Release, doch habe immer noch meine zweifel, dass es diese Lücke auch wirklich gibt. ;O

mfg
05/14/2012 23:29 Caseツ#7
Netter Release, Der wos gebrauchen kann soll es benutzten !

Werde es zwar nicht benutzten

hast dir aber ein Thanks für die müe vedient

MFG

TheBlackWater
05/16/2012 09:43 xMootie#8
Here's a less obtrusive version.

Code:
LPTSTR CAr::ReadString( LPTSTR lpsz )
{
	int nLen;
	*this >> nLen;
	Read( lpsz, sizeof(TCHAR) * nLen );
#if defined(__CLIENT) || defined(__DBSERVER)
	for( int i=0;i<nLen;i++ )
#ifdef __CLIENT
		if( lpsz[i] == '\x60' )
			lpsz[i] = '\x27';
#else // __CLIENT
#ifdef __DBSERVER
		if( lpsz[i] == '\x27' )
			lpsz[i] = '\x60';
#endif // __DBSERVER
#endif // __CLIENT
#endif // defined(__CLIENT) || defined(__DBSERVER)
	lpsz[nLen] = '\0';
	return lpsz;
}

LPTSTR CAr::ReadString( LPTSTR lpsz, int nBufSize )
{
	int nLen;
	*this >> nLen;

	int nReadable	= nBufSize - 1;
	if( nLen > nReadable || nLen < 0 )
	{
		memset( (void*)lpsz, 0, nBufSize );
		m_lpBufCur	= m_lpBufMax;
		return lpsz;
	}
	else
	{
		Read( lpsz, sizeof(TCHAR) * nLen );
#if defined(__CLIENT) || defined(__DBSERVER)
		for( int i=0;i<nLen;i++ )
#ifdef __CLIENT
			if( lpsz[i] == '\x60' )
				lpsz[i] = '\x27';
#else // __CLIENT
#ifdef __DBSERVER
			if( lpsz[i] == '\x27' )
				lpsz[i] = '\x60';
#endif // __DBSERVER
#endif // __CLIENT
#endif // defined(__CLIENT) || defined(__DBSERVER)

		lpsz[nLen] = '\0';
		return lpsz;
	}
}
This originally was my fix that I told Insanity about(after being threatened with DDoS). It's not very good for performance, but it is secure and a decent quick fix. The long, but efficient, fix is to convert each input variable to hexadecimal format to escape the single quote entirely.

Here's an example.

Standard method of variable formatting:
Quote:
SELECT [password] FROM [ACCOUNT_TBL] WHERE account = '';shutdown;--'
Hexadecimal formatting:
Quote:
SELECT [password] FROM [ACCOUNT_TBL] WHERE account = 0x273b73687574646f776e3b2d2d
Try it yourself, it works very nicely and is useful for data that may contain single quotes such as mail etc.