Fix a crash bug with auth modules

If a module like imapauth needs some time to process a login, it's possible that
the client already disconnected by the time the lookup finished. This would then
cause a stale pointer in CAuthBase to be dereferenced.

Fix this remotely exploitable crash bug by adding a new function
CAuthBase::Invalidate(). After this was called, the CAuthBase instance doesn't
do anything at all anymore, especially not dereferencing the (possibly stale)
m_pSock pointer.

This also makes sure that one can only call AcceptLogin() or RefuseLogin() once.

Thanks to Sm0ke0ut for providing backtraces and reporting this bug.


git-svn-id: https://znc.svn.sourceforge.net/svnroot/znc/trunk@1669 726aef4b-f618-498e-8847-2d620e286838

Client.cpp

@@ -49,7 +49,7 @@
 CClient::~CClient() {
 	if (!m_spAuth.IsNull()) {
 		CClientAuth* pAuth = (CClientAuth*) &(*m_spAuth);
-		pAuth->SetClient(NULL);
+		pAuth->Invalidate();
 	}
 	if (m_pUser != NULL) {
 		m_pUser->AddBytesRead(GetBytesRead());
@@ -641,9 +641,23 @@ CString CAuthBase::GetRemoteIP() const {
 	return "";
 }
 
+void CAuthBase::Invalidate() {
+	m_pSock = NULL;
+}
+
+void CAuthBase::AcceptLogin(CUser& User) {
+	if (m_pSock) {
+		AcceptedLogin(User);
+		Invalidate();
+	}
+}
+
 void CAuthBase::RefuseLogin(const CString& sReason) {
 	CUser* pUser = CZNC::Get().GetUser(GetUsername());
 
+	if (!m_pSock)
+		return;
+
 	// If the username is valid, notify that user that someone tried to
 	// login. Use sReason because there are other reasons than "wrong
 	// password" for a login to be rejected (e.g. fail2ban).
@@ -656,6 +670,7 @@ void CAuthBase::RefuseLogin(const CString& sReason) {
 	CZNC::Get().GetModules().OnFailedLogin(GetUsername(), GetRemoteIP());
 #endif
 	RefusedLogin(sReason);
+	Invalidate();
 }
 
 void CClient::RefuseLogin(const CString& sReason) {

Client.h

@@ -35,7 +35,7 @@ public:
 		m_pSock = pSock;
 	}
 
-	void AcceptLogin(CUser& User) { AcceptedLogin(User); }
+	void AcceptLogin(CUser& User);
 	void RefuseLogin(const CString& sReason);
 
 	const CString& GetUsername() const { return m_sUsername; }
@@ -43,6 +43,10 @@ public:
 	Csock *GetSocket() const { return m_pSock; }
 	CString GetRemoteIP() const;
 
+	// Invalidate this CAuthBase instance which means it will no longer use
+	// m_pSock and AcceptLogin() or RefusedLogin() will have no effect.
+	virtual void Invalidate();
+
 protected:
 	virtual void AcceptedLogin(CUser& User) = 0;
 	virtual void RefusedLogin(const CString& sReason) = 0;
@@ -59,7 +63,7 @@ public:
 	CClientAuth(CClient* pClient, const CString& sUsername, const CString& sPassword);
 	virtual ~CClientAuth() {}
 
-	void SetClient(CClient* pClient) { m_pClient = pClient; }
+	void Invalidate() { m_pClient = NULL; CAuthBase::Invalidate(); }
 	void AcceptedLogin(CUser& User);
 	void RefusedLogin(const CString& sReason);
 private:

modules/webadmin.cpp

@@ -25,7 +25,7 @@ public:
 
 	virtual ~CWebAdminAuth() {}
 
-	void SetWebAdminSock(CWebAdminSock* pWebAdminSock) { m_pWebAdminSock = pWebAdminSock; }
+	void Invalidate() { m_pWebAdminSock = NULL; CAuthBase::Invalidate(); }
 	void AcceptedLogin(CUser& User);
 	void RefusedLogin(const CString& sReason);
 private:
@@ -359,7 +359,7 @@ CWebAdminSock::CWebAdminSock(CWebAdminMod* pModule, const CString& sHostname, un
 CWebAdminSock::~CWebAdminSock() {
 	if (!m_spAuth.IsNull()) {
 		CWebAdminAuth* pAuth = (CWebAdminAuth*) &(*m_spAuth);
-		pAuth->SetWebAdminSock(NULL);
+		pAuth->Invalidate();
 	}
 }