[Licq-devel] Null Pointer Crash Fixes

Florian Doersch <[email protected]>
Newsgroups gmane.network.licq.devel
Organization privat
Message-ID <[email protected]> 
Hi,

I found some crashes caused by missing NULL pointer checks.
I fixed most of them and added additional checks.

The common crash was while receiving a pager message from a non existant 
user (mostly spambots) here:

---
ICQUser *u = gUserManager.FetchUser(nUin, LOCK_R);
bool bIgnore = u->IgnoreList();
gUserManager.DropUser(u);
---

u was NULL very often and I think you can see what happens then ;-)

I provided a patch for that.
(I hope googlegroups accept the attached patch ;-) )

See Ya
FloSoft
licq-1.3.4-null-pointer-fixes.patch (text/plain, 6.7 KB) 
diff -urN licq-1.3.4/src/icqd-srv.cpp licq-1.3.4-fixed/src/icqd-srv.cpp
--- licq-1.3.4/src/icqd-srv.cpp	2006-10-15 14:10:58.000000000 +0200
+++ licq-1.3.4-fixed/src/icqd-srv.cpp	2007-08-28 11:59:20.000000000 +0200
@@ -144,7 +144,7 @@
   {
     // If they aren't a current server user and not in the ingore list,
     // let's import them!
-    if (pUser->GetSID() == 0 && !pUser->IgnoreList())
+    if (pUser && pUser->GetSID() == 0 && !pUser->IgnoreList())
     {
       UserStringList::const_iterator p = std::find(doneUsers.begin(), doneUsers.end(),
         pUser->IdString());
@@ -177,7 +177,7 @@
   UserStringList visibleUsers, invisibleUsers, ignoredUsers;
   FOR_EACH_PROTO_USER_START(LICQ_PPID, LOCK_R)
   {
-    if (pUser->IgnoreList() && pUser->GetSID() == 0)
+    if (pUser && pUser->IgnoreList() && pUser->GetSID() == 0)
     {
       ignoredUsers.push_back(strdup(pUser->IdString()));
       pthread_mutex_lock(&mutex_modifyserverusers);
@@ -186,7 +186,7 @@
     }
     else
     {
-      if (pUser->InvisibleList() && pUser->GetInvisibleSID() == 0)
+      if (pUser && pUser->InvisibleList() && pUser->GetInvisibleSID() == 0)
       {
         invisibleUsers.push_back(strdup(pUser->IdString()));
         pthread_mutex_lock(&mutex_modifyserverusers);
@@ -194,7 +194,7 @@
         pthread_mutex_unlock(&mutex_modifyserverusers);
       }
 
-      if (pUser->VisibleList() && pUser->GetVisibleSID() == 0)
+      if (pUser && pUser->VisibleList() && pUser->GetVisibleSID() == 0)
       {
         visibleUsers.push_back(strdup(pUser->IdString()));
         pthread_mutex_lock(&mutex_modifyserverusers);
@@ -1219,6 +1219,7 @@
   UserStringList users;
   FOR_EACH_PROTO_USER_START(LICQ_PPID, LOCK_W)
   {
+  	assert(pUser);
     n++;
     users.push_back(strdup(pUser->IdString()));
     if (n == m_nMaxUsersPerPacket)
@@ -1266,7 +1267,7 @@
   UserStringList users;
   FOR_EACH_PROTO_USER_START(LICQ_PPID, LOCK_R)
   {
-    if (pUser->GetInGroup(GROUPS_SYSTEM, GROUP_VISIBLE_LIST) )
+    if (pUser && pUser->GetInGroup(GROUPS_SYSTEM, GROUP_VISIBLE_LIST) )
       users.push_back(strdup(pUser->IdString()));
   }
   FOR_EACH_PROTO_USER_END
@@ -1282,7 +1283,7 @@
   UserStringList users;
   FOR_EACH_PROTO_USER_START(LICQ_PPID, LOCK_R)
   {
-    if (pUser->GetInGroup(GROUPS_SYSTEM, GROUP_INVISIBLE_LIST) )
+    if (pUser && pUser->GetInGroup(GROUPS_SYSTEM, GROUP_INVISIBLE_LIST) )
       users.push_back(strdup(pUser->IdString()));
   }
   FOR_EACH_PROTO_USER_END
@@ -1580,6 +1581,7 @@
   // Delete all the users in groups
   FOR_EACH_PROTO_USER_START(LICQ_PPID, LOCK_W)
   {
+  	assert(pUser);
     n++;
     users.push_back(strdup(pUser->IdString()));
     if (n == m_nMaxUsersPerPacket)
@@ -1610,7 +1612,7 @@
     
   FOR_EACH_PROTO_USER_START(LICQ_PPID, LOCK_W)
   {
-    if (pUser->GetInvisibleSID())
+    if (pUser && pUser->GetInvisibleSID())
     {
       n++;
       users.push_back(strdup(pUser->IdString()));
@@ -1643,7 +1645,7 @@
   
   FOR_EACH_PROTO_USER_START(LICQ_PPID, LOCK_W)
   {
-    if (pUser->GetVisibleSID())
+    if (pUser && pUser->GetVisibleSID())
     {
       n++;
       users.push_back(strdup(pUser->IdString()));
@@ -2008,7 +2010,7 @@
   // online field
   FOR_EACH_PROTO_USER_START(LICQ_PPID, LOCK_W)
   {
-    if (!pUser->StatusOffline())
+    if (pUser && !pUser->StatusOffline())
       ChangeUserStatus(pUser, ICQ_STATUS_OFFLINE);
   }
   FOR_EACH_PROTO_USER_END
@@ -2182,6 +2184,7 @@
   
   FOR_EACH_USER_START(LOCK_R)
   {
+  	assert(pUser);
     ParseDigits(szParsedNumber1, pUser->GetCellularNumber(), 15);
     ParseDigits(szParsedNumber2, szCellular, 15);
     if (!strcmp(szParsedNumber1, szParsedNumber2))
@@ -4919,9 +4922,14 @@
 	  case ICQ_CMDxSUB_EMAILxPAGER:
 	  {
             ICQUser *u = gUserManager.FetchUser(nUin, LOCK_R);
-            bool bIgnore = u->IgnoreList();
-            gUserManager.DropUser(u);
-
+            bool bIgnore = true;
+            
+            if(u)
+            {
+            	bIgnore = u->IgnoreList();
+            	gUserManager.DropUser(u);
+			}
+			
             if (bIgnore)
             {
               delete eEvent; // Processing stops here, needs to be deleted
diff -urN licq-1.3.4/src/icqd-udp.cpp licq-1.3.4-fixed/src/icqd-udp.cpp
--- licq-1.3.4/src/icqd-udp.cpp	2006-10-15 14:10:58.000000000 +0200
+++ licq-1.3.4-fixed/src/icqd-udp.cpp	2007-08-28 11:44:37.000000000 +0200
@@ -302,7 +302,7 @@
   UinList uins;
   FOR_EACH_USER_START(LOCK_R)
   {
-    if (pUser->GetInGroup(GROUPS_SYSTEM, GROUP_VISIBLE_LIST) )
+    if (pUser && pUser->GetInGroup(GROUPS_SYSTEM, GROUP_VISIBLE_LIST) )
       uins.push_back(pUser->Uin());
   }
   FOR_EACH_USER_END
@@ -320,7 +320,7 @@
   UinList uins;
   FOR_EACH_USER_START(LOCK_R)
   {
-    if (pUser->GetInGroup(GROUPS_SYSTEM, GROUP_INVISIBLE_LIST) )
+    if (pUser && pUser->GetInGroup(GROUPS_SYSTEM, GROUP_INVISIBLE_LIST) )
       uins.push_back(pUser->Uin());
   }
   FOR_EACH_USER_END
diff -urN licq-1.3.4/src/icqpacket.cpp licq-1.3.4-fixed/src/icqpacket.cpp
--- licq-1.3.4/src/icqpacket.cpp	2006-10-15 14:10:58.000000000 +0200
+++ licq-1.3.4-fixed/src/icqpacket.cpp	2007-08-28 11:45:08.000000000 +0200
@@ -2581,7 +2581,7 @@
       GroupIDList *pID = gUserManager.LockGroupIDList(LOCK_R);
       for (unsigned short j = 1; j < pID->size() + 1; j++)
       {
-        if (u->GetInGroup(GROUPS_USER, j))
+        if (u && u->GetInGroup(GROUPS_USER, j))
         {
           m_nGSID = (*pID)[j-1];
           if (m_nGSID)
@@ -2741,7 +2741,7 @@
         // Use the first group that the user is in as the server stored group
         for (unsigned short i = 1; i < pID->size() + 1; i++)
         {
-          if (u->GetInGroup(GROUPS_USER, i))
+          if (u && u->GetInGroup(GROUPS_USER, i))
           {
             m_nGSID = (*pID)[i-1];
             if (m_nGSID)
diff -urN licq-1.3.4/src/user.cpp licq-1.3.4-fixed/src/user.cpp
--- licq-1.3.4/src/user.cpp	2006-10-15 14:10:58.000000000 +0200
+++ licq-1.3.4-fixed/src/user.cpp	2007-08-28 11:47:32.000000000 +0200
@@ -20,6 +20,7 @@
 #include <string.h>
 #include <errno.h>
 #include <vector>
+#include <assert.h>
 
 #ifdef HAVE_INET_ATON
 #include <arpa/inet.h>
@@ -867,7 +868,7 @@
   {
     return;
   }
-
+  
   GroupList *g = LockGroupList(LOCK_R);	
 
   // Must be called when there are no locks on GroupID and Group lists
@@ -886,6 +887,7 @@
   unsigned short j;
   FOR_EACH_USER_START(LOCK_W)
   {
+    assert(pUser);
     for (j = n; j < g->size() + 1; j++)
       pUser->SetInGroup(GROUPS_USER, j, pUser->GetInGroup(GROUPS_USER, j + 1));
   }
@@ -930,6 +932,8 @@
   bool bInG1;
   FOR_EACH_USER_START(LOCK_W)
   {
+    assert(pUser);
+    
     bInG1 = pUser->GetInGroup(GROUPS_USER, g1);
     pUser->SetInGroup(GROUPS_USER, g1, pUser->GetInGroup(GROUPS_USER, g2));
     pUser->SetInGroup(GROUPS_USER, g2, bInG1);
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.