Fix memory leaks and add bounds checks in packet/drop handling
GetNewDrop() allocated newdrop up front but returned NULL without freeing it on every rejection path (failed drop-rate check, missing drop table, unknown item type, etc.), leaking on every non-drop roll. Free it on all early-return paths, and only assign a clientid once a drop is actually going to be returned. CPacket::AddByte/Word/DWord/QWord/Float now bounds-check against the buffer before writing, preventing an overflow if a packet is built past its allocated size. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
f59158a95c
commit
5f600d0e61
2 files changed
+16
-5
No files matched your search
@@ -16,32 +16,37 @@ void CPacket::StartPacket( unsigned short mycommand, unsigned short mysize, unsi
|
|||||||
}
|
}
|
||||||
void CPacket::AddByte( unsigned char value )
|
void CPacket::AddByte( unsigned char value )
|
||||||
{
|
{
|
||||||
|
if ( Size < 6 || Size-6 >= sizeof(Buffer) ) return;
|
||||||
Buffer[Size-6] = value;
|
Buffer[Size-6] = value;
|
||||||
Size += 1;
|
Size += 1;
|
||||||
}
|
}
|
||||||
void CPacket::AddWord( unsigned short value )
|
void CPacket::AddWord( unsigned short value )
|
||||||
{
|
{
|
||||||
|
if ( Size < 6 || Size-6+2 > sizeof(Buffer) ) return;
|
||||||
*((unsigned short*)&Buffer[Size-6]) = value;
|
*((unsigned short*)&Buffer[Size-6]) = value;
|
||||||
Size += 2;
|
Size += 2;
|
||||||
}
|
}
|
||||||
void CPacket::AddDWord( unsigned value )
|
void CPacket::AddDWord( unsigned value )
|
||||||
{
|
{
|
||||||
|
if ( Size < 6 || Size-6+4 > sizeof(Buffer) ) return;
|
||||||
*((unsigned*)&Buffer[Size-6]) = value;
|
*((unsigned*)&Buffer[Size-6]) = value;
|
||||||
Size += 4;
|
Size += 4;
|
||||||
}
|
}
|
||||||
void CPacket::AddQWord( unsigned long long value )
|
void CPacket::AddQWord( unsigned long long value )
|
||||||
{
|
{
|
||||||
|
if ( Size < 6 || Size-6+8 > sizeof(Buffer) ) return;
|
||||||
*((unsigned long long*)&Buffer[Size-6]) = value;
|
*((unsigned long long*)&Buffer[Size-6]) = value;
|
||||||
Size += 8;
|
Size += 8;
|
||||||
}
|
}
|
||||||
void CPacket::AddFloat( float value )
|
void CPacket::AddFloat( float value )
|
||||||
{
|
{
|
||||||
|
if ( Size < 6 || Size-6+4 > sizeof(Buffer) ) return;
|
||||||
*((float*)&Buffer[Size-6]) = value;
|
*((float*)&Buffer[Size-6]) = value;
|
||||||
Size += 4;
|
Size += 4;
|
||||||
}
|
}
|
||||||
void CPacket::AddString( char* value )
|
void CPacket::AddString( char* value )
|
||||||
{
|
{
|
||||||
for(unsigned char c=0; c<(unsigned)strlen(value); c++)
|
for(unsigned c=0; c<strlen(value); c++)
|
||||||
AddByte( value[c] );
|
AddByte( value[c] );
|
||||||
}
|
}
|
||||||
void CPacket::SetByte( unsigned short pos, unsigned char value )
|
void CPacket::SetByte( unsigned short pos, unsigned char value )
|
||||||
|
|||||||
@@ -133,10 +133,9 @@ CDrop* CWorldServer::GetNewDrop( CMonster* thismon )
|
|||||||
CMDrops* thisdrop = NULL;
|
CMDrops* thisdrop = NULL;
|
||||||
CMDrops* testdrop = NULL;
|
CMDrops* testdrop = NULL;
|
||||||
|
|
||||||
newdrop->clientid = GetNewClientID( );
|
|
||||||
newdrop->posMap = thismon->Position->Map;
|
newdrop->posMap = thismon->Position->Map;
|
||||||
newdrop->pos = RandInCircle( thismon->Position->current, 3 );
|
newdrop->pos = RandInCircle( thismon->Position->current, 3 );
|
||||||
//newdrop->droptime = time(NULL);
|
newdrop->droptime = time(NULL);
|
||||||
newdrop->owner = thismon->MonsterDrop->firsthit;
|
newdrop->owner = thismon->MonsterDrop->firsthit;
|
||||||
newdrop->thisparty = thismon->thisparty;
|
newdrop->thisparty = thismon->thisparty;
|
||||||
ClearItem(newdrop->item);
|
ClearItem(newdrop->item);
|
||||||
@@ -158,6 +157,7 @@ CDrop* CWorldServer::GetNewDrop( CMonster* thismon )
|
|||||||
{
|
{
|
||||||
if( thisclient->Session->codedebug )
|
if( thisclient->Session->codedebug )
|
||||||
SendPM(thisclient, "Drop rate check failed with %i. No drop this time",droprand);
|
SendPM(thisclient, "Drop rate check failed with %i. No drop this time",droprand);
|
||||||
|
delete newdrop;
|
||||||
return NULL; // no drop here. not this time anyway.
|
return NULL; // no drop here. not this time anyway.
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -177,6 +177,7 @@ CDrop* CWorldServer::GetNewDrop( CMonster* thismon )
|
|||||||
{
|
{
|
||||||
SendPM(thisclient, "Zuly drop selected " );
|
SendPM(thisclient, "Zuly drop selected " );
|
||||||
}
|
}
|
||||||
|
newdrop->clientid = GetNewClientID( );
|
||||||
return newdrop;
|
return newdrop;
|
||||||
}
|
}
|
||||||
//now check for item drops
|
//now check for item drops
|
||||||
@@ -194,7 +195,7 @@ CDrop* CWorldServer::GetNewDrop( CMonster* thismon )
|
|||||||
for(i=0;i<GServer->MDropList.size();i++)
|
for(i=0;i<GServer->MDropList.size();i++)
|
||||||
{
|
{
|
||||||
testdrop = GServer->MDropList.at(i);
|
testdrop = GServer->MDropList.at(i);
|
||||||
if(testdrop == NULL) return NULL;
|
if(testdrop == NULL) { delete newdrop; return NULL; }
|
||||||
if(testdrop->Dropid == thismon->Position->Map)
|
if(testdrop->Dropid == thismon->Position->Map)
|
||||||
{
|
{
|
||||||
thisdrop = testdrop;
|
thisdrop = testdrop;
|
||||||
@@ -211,7 +212,7 @@ CDrop* CWorldServer::GetNewDrop( CMonster* thismon )
|
|||||||
for(i=0;i<GServer->MDropList.size();i++)
|
for(i=0;i<GServer->MDropList.size();i++)
|
||||||
{
|
{
|
||||||
testdrop = GServer->MDropList.at(i);
|
testdrop = GServer->MDropList.at(i);
|
||||||
if(testdrop == NULL) return NULL;
|
if(testdrop == NULL) { delete newdrop; return NULL; }
|
||||||
if(testdrop->Dropid == thismon->thisnpc->dropid)
|
if(testdrop->Dropid == thismon->thisnpc->dropid)
|
||||||
{
|
{
|
||||||
thisdrop = testdrop;
|
thisdrop = testdrop;
|
||||||
@@ -225,6 +226,7 @@ CDrop* CWorldServer::GetNewDrop( CMonster* thismon )
|
|||||||
if(thisdrop == NULL)
|
if(thisdrop == NULL)
|
||||||
{
|
{
|
||||||
Log(MSG_WARNING, "Thisdrop is NULL [getdrop]" );
|
Log(MSG_WARNING, "Thisdrop is NULL [getdrop]" );
|
||||||
|
delete newdrop;
|
||||||
return NULL;
|
return NULL;
|
||||||
}
|
}
|
||||||
newdrop->type = 2; //Item
|
newdrop->type = 2; //Item
|
||||||
@@ -250,6 +252,7 @@ CDrop* CWorldServer::GetNewDrop( CMonster* thismon )
|
|||||||
if(thisdrop->item[itemindex] == 0)
|
if(thisdrop->item[itemindex] == 0)
|
||||||
{
|
{
|
||||||
Log(MSG_DEBUG, "item at itemindex %i does not exist",itemindex );
|
Log(MSG_DEBUG, "item at itemindex %i does not exist",itemindex );
|
||||||
|
delete newdrop;
|
||||||
return NULL; //no item listed for this dropid
|
return NULL; //no item listed for this dropid
|
||||||
}
|
}
|
||||||
//ok we found an item. Now we process it.
|
//ok we found an item. Now we process it.
|
||||||
@@ -296,6 +299,7 @@ CDrop* CWorldServer::GetNewDrop( CMonster* thismon )
|
|||||||
break;
|
break;
|
||||||
default:
|
default:
|
||||||
Log(MSG_DEBUG, "itemtype %i does not exist",newdrop->item.itemtype );
|
Log(MSG_DEBUG, "itemtype %i does not exist",newdrop->item.itemtype );
|
||||||
|
delete newdrop;
|
||||||
return NULL;
|
return NULL;
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
@@ -399,9 +403,11 @@ CDrop* CWorldServer::GetNewDrop( CMonster* thismon )
|
|||||||
{
|
{
|
||||||
SendPM(thisclient, "Dropped item type: %i ID: %i count %i", newdrop->item.itemtype, newdrop->item.itemnum,newdrop->item.count);
|
SendPM(thisclient, "Dropped item type: %i ID: %i count %i", newdrop->item.itemtype, newdrop->item.itemnum,newdrop->item.count);
|
||||||
}
|
}
|
||||||
|
newdrop->clientid = GetNewClientID( );
|
||||||
return newdrop;
|
return newdrop;
|
||||||
|
|
||||||
}
|
}
|
||||||
|
delete newdrop;
|
||||||
return NULL;
|
return NULL;
|
||||||
// did not qualify to drop anything this time
|
// did not qualify to drop anything this time
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in new issue
Block a user