From b0e59f12940301c9c6b647276da292f875eeb576 Mon Sep 17 00:00:00 2001
From: psychon <psychon@726aef4b-f618-498e-8847-2d620e286838>
Date: Sat, 28 Nov 2009 18:53:20 +0000
Subject: [PATCH] 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           | 17 ++++++++++++++++-
 Client.h             |  8 ++++++--
 modules/webadmin.cpp |  4 ++--
 3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/Client.cpp b/Client.cpp
index e697fda6..e901a2f8 100644
--- a/Client.cpp
+++ b/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) {
diff --git a/Client.h b/Client.h
index 7e5c46f6..9e71e3ba 100644
--- a/Client.h
+++ b/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:
diff --git a/modules/webadmin.cpp b/modules/webadmin.cpp
index fb2ee907..2bd1f4ca 100644
--- a/modules/webadmin.cpp
+++ b/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();
 	}
 }