[1.298, 1.310/1.351/2.0] Friend report overflow

tarafından **Hello EMO** Çarş. Şub. 23, 2011 8:28 am

[quote name='twostars' timestamp='1297199843' post='2717']
The CUser::FriendReport() overflow is probably one of the most stupid exploits MGame ever caused. Really, it's all just ignorance - typically, when one creates a buffer, they create it large enough to hold the most possible data that will ever be going into it. But no, MGame creates one so far underneath that (less than HALF - 256 bytes) it's just plain stupid.

Here's the function that's problematic. I've somewhat updated it to what it looks like in 1.298 (the problem, anyway).

Kod:: void CUser::FriendReport(char *pBuf) { int index = 0; short usercount = 0, idlen = 0; int send_index = 0; char send_buff[256]; // ARRAY BEING OVERFLOWED memset( send_buff, NULL, 256); char userid[MAX_ID_SIZE+1]; memset( userid, NULL, MAX_ID_SIZE+1 ); CUser* pUser = NULL; usercount = GetShort( pBuf, index ); if( usercount >= 25 || usercount < 0) return; SetByte( send_buff, WIZ_FRIEND_REPORT, send_index ); SetShort(send_buff, usercount, send_index); for (int k = 0 ; k < usercount ; k++) { idlen = GetShort( pBuf, index ); if (idlen > MAX_ID_SIZE || idlen < 0) { SetShort(send_buff, strlen(userid), send_index); SetString( send_buff, userid, strlen(userid), send_index ); SetShort(send_buff, -1, send_index); SetByte( send_buff, 0, send_index); continue; } GetString( userid, pBuf, idlen, index ); pUser = m_pMain->GetUserPtr(userid, 0x02); SetShort(send_buff, idlen, send_index); SetString( send_buff, userid, idlen, send_index ); if (!pUser) { SetShort(send_buff, -1, send_index); SetByte(send_buff, 0, send_index); } else { SetShort(send_buff, pUser->m_Sid, send_index); if (pUser->m_sPartyIndex >= 0) { SetByte(send_buff, 3, send_index); } else { SetByte(send_buff, 1, send_index); } } } Send(send_buff, send_index); }

So we can glean from the above code a couple of notable things:

The total number of players retrieved and placed back in that packet (with statuses) can be no more than 25.
The buffer needs to be approximately: 1 (opcode) + 2 (user count; from memory this isn't used anymore, but it doesn't hurt to have slightly extra) + (25 [player limit] * (2 [string size prefix] + MAX_ID_LEN [20] + 1 [status byte])) bytes in length. As there's 25 users total, that's 578 bytes. MGame's is 256 - clearly, it's not big enough.

There's really two different solutions here (at least, to solve the crash exploit) of which I both explained in a previous thread (in the old Snoxd) when encouraging you guys to figure out a patch for yourself. However, I am a little bit disappointed you chose the latter - the latter being to simply decrease the player limit. That limits functionality! The other method really isn't so hard, so here it is - done for you:

This solution means we need to increase the array! To do that, we need to adjust the stack space allocated for that function and fix up all required references that follow.

First things first - 578 in hexadecimal is 242h. Currently, they allocate:

Kod:: 004B4B50 81EC 30010000 SUB ESP,130

30h of that is for the other variables they require in that method, so 100h (256!) is what they have assigned for our send_buff array. So, what we'll want to do is make it 272h (that is, 30h [other variables] + 242h [increased send_buff array]). However, since the stack pointer needs to be aligned, we will increase that ever-so-slightly to 280h. Wink