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
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);
