From 5f600d0e61e1de6bd0ab5c84964db1697b807803 Mon Sep 17 00:00:00 2001
From: Thomas <claude@zneb.to>
Date: Thu, 9 Jul 2026 06:35:18 +0200
Subject: [PATCH] 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>
---
 Src/Common/globals.cpp               |  7 ++++++-
 Src/World Server/ServerFunctions.cpp | 14 ++++++++++----
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/Src/Common/globals.cpp b/Src/Common/globals.cpp
index 0f449cb..7b8993b 100644
--- a/Src/Common/globals.cpp
+++ b/Src/Common/globals.cpp
@@ -16,32 +16,37 @@ void CPacket::StartPacket( unsigned short mycommand, unsigned short mysize, unsi
 }
 void CPacket::AddByte( unsigned char value )
 {
+	if ( Size < 6 || Size-6 >= sizeof(Buffer) ) return;
 	Buffer[Size-6] = value;
 	Size += 1;
 }
 void CPacket::AddWord( unsigned short value )
 {
+	if ( Size < 6 || Size-6+2 > sizeof(Buffer) ) return;
 	*((unsigned short*)&Buffer[Size-6]) = value;
 	Size += 2;
 }
 void CPacket::AddDWord( unsigned value )
 {
+	if ( Size < 6 || Size-6+4 > sizeof(Buffer) ) return;
 	*((unsigned*)&Buffer[Size-6]) = value;
 	Size += 4;
 }
 void CPacket::AddQWord( unsigned long long value )
 {
+	if ( Size < 6 || Size-6+8 > sizeof(Buffer) ) return;
 	*((unsigned long long*)&Buffer[Size-6]) = value;
 	Size += 8;
 }
 void CPacket::AddFloat( float value )
 {
+	if ( Size < 6 || Size-6+4 > sizeof(Buffer) ) return;
 	*((float*)&Buffer[Size-6]) = value;
 	Size += 4;
 }
 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] );
 }
 void CPacket::SetByte( unsigned short pos, unsigned char value )
diff --git a/Src/World Server/ServerFunctions.cpp b/Src/World Server/ServerFunctions.cpp
index f00fd38..02d09a2 100644
--- a/Src/World Server/ServerFunctions.cpp	
+++ b/Src/World Server/ServerFunctions.cpp	
@@ -133,10 +133,9 @@ CDrop* CWorldServer::GetNewDrop( CMonster* thismon )
     CMDrops* thisdrop = NULL;
     CMDrops* testdrop = NULL;
 
-    newdrop->clientid = GetNewClientID( );
     newdrop->posMap = thismon->Position->Map;
     newdrop->pos = RandInCircle( thismon->Position->current, 3 );
-    //newdrop->droptime = time(NULL);
+    newdrop->droptime = time(NULL);
     newdrop->owner = thismon->MonsterDrop->firsthit;
     newdrop->thisparty = thismon->thisparty;
     ClearItem(newdrop->item);
@@ -158,6 +157,7 @@ CDrop* CWorldServer::GetNewDrop( CMonster* thismon )
     {
         if( thisclient->Session->codedebug )
             SendPM(thisclient, "Drop rate check failed with %i. No drop this time",droprand);
+        delete newdrop;
         return NULL; // no drop here. not this time anyway.
     }
 
@@ -177,6 +177,7 @@ CDrop* CWorldServer::GetNewDrop( CMonster* thismon )
         {
            SendPM(thisclient, "Zuly drop selected " );
         }
+        newdrop->clientid = GetNewClientID( );
         return  newdrop;
     }
     //now check for item drops
@@ -194,7 +195,7 @@ CDrop* CWorldServer::GetNewDrop( CMonster* thismon )
             for(i=0;i<GServer->MDropList.size();i++)
             {
                 testdrop =  GServer->MDropList.at(i);
-                if(testdrop == NULL) return NULL;
+                if(testdrop == NULL) { delete newdrop; return NULL; }
                 if(testdrop->Dropid == thismon->Position->Map)
                 {
                     thisdrop = testdrop;
@@ -211,7 +212,7 @@ CDrop* CWorldServer::GetNewDrop( CMonster* thismon )
             for(i=0;i<GServer->MDropList.size();i++)
             {
                 testdrop =  GServer->MDropList.at(i);
-                if(testdrop == NULL) return NULL;
+                if(testdrop == NULL) { delete newdrop; return NULL; }
                 if(testdrop->Dropid == thismon->thisnpc->dropid)
                 {
                     thisdrop = testdrop;
@@ -225,6 +226,7 @@ CDrop* CWorldServer::GetNewDrop( CMonster* thismon )
         if(thisdrop == NULL)
         {
             Log(MSG_WARNING, "Thisdrop is NULL [getdrop]" );
+            delete newdrop;
             return NULL;
         }
         newdrop->type = 2; //Item
@@ -250,6 +252,7 @@ CDrop* CWorldServer::GetNewDrop( CMonster* thismon )
         if(thisdrop->item[itemindex] == 0)
         {
             Log(MSG_DEBUG, "item at itemindex %i does not exist",itemindex );
+            delete newdrop;
             return NULL; //no item listed for this dropid
         }
         //ok we found an item. Now we process it.
@@ -296,6 +299,7 @@ CDrop* CWorldServer::GetNewDrop( CMonster* thismon )
                 break;
             default:
                 Log(MSG_DEBUG, "itemtype %i does not exist",newdrop->item.itemtype );
+                delete newdrop;
                 return NULL;
                 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);
         }
+        newdrop->clientid = GetNewClientID( );
         return newdrop;
 
     }
+    delete newdrop;
     return NULL;
     // did not qualify to drop anything this time
 }