Fix a user-after-free bug

When a user is deleted we didn't properly clean up after it. Active DCC
connections (CDCCBounce and CDCCSock) where left laying around and at some later
point of time they used their CUser* pointer which now pointed to invalid data.

This bug is similar to the one from r1557.

Thanks to cnu, our beloved master of destruction, for finding yet another way to
make ZNC break and for testing the patch.


git-svn-id: https://znc.svn.sourceforge.net/svnroot/znc/trunk@1575 726aef4b-f618-498e-8847-2d620e286838
This commit is contained in:
psychon
2009-07-22 16:54:52 +00:00
parent 6c1014f200
commit f8178e9bc8
5 changed files with 65 additions and 44 deletions
+9 -4
View File
@@ -32,6 +32,8 @@ CDCCBounce::CDCCBounce(CUser* pUser, unsigned long uLongIP, unsigned short uPort
if (bIsChat) {
EnableReadLine();
}
m_pUser->AddDCCBounce(this);
}
CDCCBounce::CDCCBounce(const CString& sHostname, unsigned short uPort, CUser* pUser,
@@ -50,6 +52,8 @@ CDCCBounce::CDCCBounce(const CString& sHostname, unsigned short uPort, CUser* pU
if (bIsChat) {
EnableReadLine();
}
m_pUser->AddDCCBounce(this);
}
CDCCBounce::~CDCCBounce() {
@@ -57,10 +61,11 @@ CDCCBounce::~CDCCBounce() {
m_pPeer->Shutdown();
m_pPeer = NULL;
}
if (m_pUser) {
m_pUser->AddBytesRead(GetBytesRead());
m_pUser->AddBytesWritten(GetBytesWritten());
}
m_pUser->AddBytesRead(GetBytesRead());
m_pUser->AddBytesWritten(GetBytesWritten());
m_pUser->DelDCCBounce(this);
}
void CDCCBounce::ReadLine(const CString& sData) {
+38 -4
View File
@@ -10,15 +10,49 @@
#include "User.h"
#include "Utils.h"
CDCCSock::CDCCSock(CUser* pUser, const CString& sRemoteNick, const CString& sLocalFile, const CString& sModuleName,
unsigned long uFileSize, CFile* pFile) : CZNCSock() {
m_sRemoteNick = sRemoteNick;
m_uFileSize = uFileSize;
m_uRemotePort = 0;
m_uBytesSoFar = 0;
m_pUser = pUser;
m_pFile = pFile;
m_sLocalFile = sLocalFile;
m_sModuleName = sModuleName;
m_bSend = true;
m_bNoDelFile = false;
m_pUser->AddDCCSock(this);
}
CDCCSock::CDCCSock(CUser* pUser, const CString& sRemoteNick, const CString& sRemoteIP, unsigned short uRemotePort,
const CString& sLocalFile, unsigned long uFileSize, const CString& sModuleName) : CZNCSock() {
m_sRemoteNick = sRemoteNick;
m_sRemoteIP = sRemoteIP;
m_uRemotePort = uRemotePort;
m_uFileSize = uFileSize;
m_uBytesSoFar = 0;
m_pUser = pUser;
m_pFile = NULL;
m_sLocalFile = sLocalFile;
m_sModuleName = sModuleName;
m_bSend = false;
m_bNoDelFile = false;
m_pUser->AddDCCSock(this);
}
CDCCSock::~CDCCSock() {
if ((m_pFile) && (!m_bNoDelFile)) {
m_pFile->Close();
delete m_pFile;
}
if (m_pUser) {
m_pUser->AddBytesRead(GetBytesRead());
m_pUser->AddBytesWritten(GetBytesWritten());
}
m_pUser->AddBytesRead(GetBytesRead());
m_pUser->AddBytesWritten(GetBytesWritten());
m_pUser->DelDCCSock(this);
}
void CDCCSock::ReadData(const char* data, int len) {
+2 -36
View File
@@ -17,42 +17,8 @@ class CUser;
class CDCCSock : public CZNCSock {
public:
CDCCSock(CUser* pUser, const CString& sRemoteNick, const CString& sLocalFile, const CString& sModuleName, unsigned long uFileSize = 0, CFile* pFile = NULL) : CZNCSock() {
m_sRemoteNick = sRemoteNick;
m_uFileSize = uFileSize;
m_uRemotePort = 0;
m_uBytesSoFar = 0;
m_pUser = pUser;
m_pFile = pFile;
m_sLocalFile = sLocalFile;
m_sModuleName = sModuleName;
m_bSend = true;
m_bNoDelFile = false;
}
CDCCSock(CUser* pUser, const CString& sRemoteNick, const CString& sRemoteIP, unsigned short uRemotePort, const CString& sLocalFile, unsigned long uFileSize, const CString& sModuleName) : CZNCSock() {
m_sRemoteNick = sRemoteNick;
m_sRemoteIP = sRemoteIP;
m_uRemotePort = uRemotePort;
m_uFileSize = uFileSize;
m_uBytesSoFar = 0;
m_pUser = pUser;
m_pFile = NULL;
m_sLocalFile = sLocalFile;
m_sModuleName = sModuleName;
m_bSend = false;
m_bNoDelFile = false;
}
/* CDCCSock(CUser* pUser, const CString& sHostname, unsigned short uPort, int iTimeout = 60) : Csock(sHostname, uPort, iTimeout) {
m_uRemotePort = 0;
m_uBytesSoFar = 0;
m_uFileSize = 0;
m_pFile = NULL;
m_pUser = pUser;
m_bNoDelFile = false;
}
*/
CDCCSock(CUser* pUser, const CString& sRemoteNick, const CString& sLocalFile, const CString& sModuleName, unsigned long uFileSize = 0, CFile* pFile = NULL);
CDCCSock(CUser* pUser, const CString& sRemoteNick, const CString& sRemoteIP, unsigned short uRemotePort, const CString& sLocalFile, unsigned long uFileSize, const CString& sModuleName);
virtual ~CDCCSock();
virtual void ReadData(const char* data, int len);
+7
View File
@@ -69,6 +69,13 @@ CUser::~CUser() {
delete m_vChans[b];
}
// This will cause an endless loop if the destructor doesn't remove the
// socket from this list / if the socket doesn't exist any more.
while (!m_sDCCBounces.empty())
CZNC::Get().GetManager().DelSockByAddr((CZNCSock*) *m_sDCCBounces.begin());
while (!m_sDCCSocks.empty())
CZNC::Get().GetManager().DelSockByAddr((CZNCSock*) *m_sDCCSocks.begin());
CZNC::Get().GetManager().DelCronByAddr(m_pJoinTimer);
CZNC::Get().GetManager().DelCronByAddr(m_pMiscTimer);
}
+9
View File
@@ -27,6 +27,8 @@ class CIRCSock;
class CJoinTimer;
class CMiscTimer;
class CServer;
class CDCCBounce;
class CDCCSock;
class CUser {
public:
@@ -97,6 +99,11 @@ public:
void IRCDisconnected();
void CheckIRCConnect();
void AddDCCBounce(CDCCBounce* p) { m_sDCCBounces.insert(p); }
void DelDCCBounce(CDCCBounce* p) { m_sDCCBounces.erase(p); }
void AddDCCSock(CDCCSock* p) { m_sDCCSocks.insert(p); }
void DelDCCSock(CDCCSock* p) { m_sDCCSocks.erase(p); }
CString ExpandString(const CString& sStr) const;
CString& ExpandString(const CString& sStr, CString& sRet) const;
@@ -246,6 +253,8 @@ protected:
vector<CServer*> m_vServers;
vector<CChan*> m_vChans;
vector<CClient*> m_vClients;
set<CDCCBounce*> m_sDCCBounces;
set<CDCCSock*> m_sDCCSocks;
set<CString> m_ssAllowedHosts;
unsigned int m_uServerIdx;
unsigned int m_uBufferCount;