From 15013ecd173c7bdbd404c13fbbf5a594a9b04878 Mon Sep 17 00:00:00 2001
From: w00t <w00t@e03df62e-2008-0410-955e-edbf42e46eb7>
Date: Tue, 28 Oct 2008 23:47:33 +0000
Subject: [PATCH] Fix a low risk race condition which could result in crashes,
 spotted by Szymek - big thanks! :)

git-svn-id: http://svn.inspircd.org/repository/branches/1_1_stable@10747 e03df62e-2008-0410-955e-edbf42e46eb7
---
 include/users.h          |  5 +++--
 src/modules/m_cgiirc.cpp | 39 +++++++++++++++++++++++++++++++++------
 src/users.cpp            | 24 +++++++++++++++++++-----
 3 files changed, 55 insertions(+), 13 deletions(-)

diff --git a/include/users.h b/include/users.h
index 81ff0f5b8..697e12216 100644
--- a/include/users.h
+++ b/include/users.h
@@ -784,9 +784,10 @@ class CoreExport userrec : public connection
 	 */
 	void Oper(const std::string &opertype);
 
-	/** Call this method to find the matching <connect> for a user, and to check them against it.
+	/** Call this method to check a user against a connect class.
+	 * @param a The connect class to match them against.
 	 */
-	void CheckClass();
+	void CheckClass(ConnectClass *a);
 
 	/** Use this method to fully connect a user.
 	 * This will send the message of the day, check G/K/E lines, etc.
diff --git a/src/modules/m_cgiirc.cpp b/src/modules/m_cgiirc.cpp
index cc06035a2..927acd6d4 100644
--- a/src/modules/m_cgiirc.cpp
+++ b/src/modules/m_cgiirc.cpp
@@ -351,8 +351,18 @@ class ModuleCgiIRC : public Module
 			user->Shrink("cgiirc_webirc_ip");
 			ServerInstance->AddLocalClone(user);
 			ServerInstance->AddGlobalClone(user);
-			user->CheckClass();
 			Recheck(user);
+
+			ConnectClass *i = user->GetClass();
+			if (!i)
+			{
+				userrec::QuitUser(ServerInstance, user, "Access denied by configuration");
+				return;
+			}
+			else
+			{
+				user->CheckClass(i);
+			}
 		}
 	}
 
@@ -379,7 +389,6 @@ class ModuleCgiIRC : public Module
 #endif
 			ServerInstance->AddLocalClone(user);
 			ServerInstance->AddGlobalClone(user);
-			user->CheckClass();
 
 			if (valid)
 			{
@@ -409,6 +418,17 @@ class ModuleCgiIRC : public Module
 			/*if(NotifyOpers)
 				ServerInstance->WriteOpers("*** Connecting user %s detected as using CGI:IRC (%s), changing real host to %s from PASS", user->nick, user->host, user->password);*/
 
+			ConnectClass *i = user->GetClass();
+			if (!i)
+			{
+				userrec::QuitUser(ServerInstance, user, "Access denied by configuration");
+				return false;
+			}
+			else
+			{
+				user->CheckClass(i);
+			}
+
 			return true;
 		}
 		
@@ -441,7 +461,6 @@ class ModuleCgiIRC : public Module
 		user->SetSockAddr(user->GetProtocolFamily(), newip, user->GetPort());
 		ServerInstance->AddLocalClone(user);
 		ServerInstance->AddGlobalClone(user);
-		user->CheckClass();
 		try
 		{
 			strlcpy(user->host, newip, 16);
@@ -462,9 +481,17 @@ class ModuleCgiIRC : public Module
 			if(NotifyOpers)
 				 ServerInstance->WriteOpers("*** Connecting user %s detected as using CGI:IRC (%s), but i could not resolve their hostname!", user->nick, user->host);
 		}
-		/*strlcpy(user->host, newip, 16);
-		strlcpy(user->dhost, newip, 16);
-		strlcpy(user->ident, "~cgiirc", 8);*/
+
+		ConnectClass *i = user->GetClass();
+		if (!i)
+		{
+			userrec::QuitUser(ServerInstance, user, "Access denied by configuration");
+			return true;
+		}
+		else
+		{
+			user->CheckClass(i);
+		}
 
 		return true;
 	}
diff --git a/src/users.cpp b/src/users.cpp
index 420400671..301ec48c6 100644
--- a/src/users.cpp
+++ b/src/users.cpp
@@ -930,7 +930,7 @@ void userrec::AddClient(InspIRCd* Instance, int socket, int port, bool iscached,
 	 * Check connect class settings and initialise settings into userrec.
 	 * This will be done again after DNS resolution. -- w00t
 	 */
-	New->CheckClass();
+	New->CheckClass(i);
 
 	Instance->local_users.push_back(New);
 
@@ -1009,10 +1009,8 @@ unsigned long userrec::LocalCloneCount()
 /*
  * Check class restrictions
  */
-void userrec::CheckClass()
+void userrec::CheckClass(ConnectClass *a)
 {
-	ConnectClass* a = this->GetClass();
-
 	if ((!a) || (a->GetType() == CC_DENY))
 	{
 		userrec::QuitUser(ServerInstance, this, "Unauthorised connection");
@@ -1045,13 +1043,29 @@ void userrec::FullConnect()
 	ServerInstance->stats->statsConnects++;
 	this->idle_lastmsg = ServerInstance->Time();
 
+
+	ConnectClass *a = this->GetClass();
+	if (!a)
+	{
+		/*
+		 * This can happen:
+		 *  - user connects
+		 *  - /rehash, removing <connect> tag they matched
+		 *  - dns completes (putting us here) ...but they no longer have a connect tag
+		 * Catch it here. Thanks Szymek!
+		 *  -- w00t
+		 */
+		userrec::QuitUser(ServerInstance, this, "Invalid password");
+		return;
+	}
+	
 	/*
 	 * You may be thinking "wtf, we checked this in userrec::AddClient!" - and yes, we did, BUT.
 	 * At the time AddClient is called, we don't have a resolved host, by here we probably do - which
 	 * may put the user into a totally seperate class with different restrictions! so we *must* check again.
 	 * Don't remove this! -- w00t
 	 */
-	this->CheckClass();
+	this->CheckClass(a);
 	
 	/* Check the password, if one is required by the user's connect class.
 	 * This CANNOT be in CheckClass(), because that is called prior to PASS as well!