Fix crash when services (or a misbehaving remote server) introduces a server with duplicate SID

git-svn-id: http://svn.inspircd.org/repository/trunk/inspircd@11171 e03df62e-2008-0410-955e-edbf42e46eb7

4 files changed, 35 insertions, 54 deletions

diff --git a/src/modules/m_spanningtree/server.cpp b/src/modules/m_spanningtree/server.cpp
index eebfa97cb..e7aef1451 100644
--- a/src/modules/m_spanningtree/server.cpp
+++ b/src/modules/m_spanningtree/server.cpp
@@ -40,8 +40,8 @@ bool TreeSocket::RemoteServer(const std::string &prefix, std::deque<std::string>
 	}
 
 	std::string servername = params[0];
-	std::string password = params[1];
-	// hopcount is not used for a remote server, we calculate this ourselves
+	// password is not used for a remote server
+	// hopcount is not used (ever)
 	std::string sid = params[3];
 	std::string description = params[4];
 	TreeServer* ParentOfThis = Utils->FindServer(prefix);
@@ -59,22 +59,23 @@ bool TreeSocket::RemoteServer(const std::string &prefix, std::deque<std::string>
 	TreeServer* CheckDupe = Utils->FindServer(servername);
 	if (CheckDupe)
 	{
-		this->SendError("Server "+CheckDupe->GetName()+" already exists!");
+		this->SendError("Server "+servername+" already exists!");
 		this->ServerInstance->SNO->WriteToSnoMask('L', "Server \2"+CheckDupe->GetName()+"\2 being introduced from \2" + ParentOfThis->GetName() + "\2 denied, already exists. Closing link with " + ParentOfThis->GetName());
 		return false;
 	}
-
-	Link* lnk = Utils->FindLink(servername);
-
-	TreeServer *Node = new TreeServer(this->Utils, this->ServerInstance, servername, description, sid, ParentOfThis,NULL, lnk ? lnk->Hidden : false);
-
-	if (Node->DuplicateID())
+	CheckDupe = Utils->FindServer(sid);
+	if (CheckDupe)
 	{
-		this->SendError("Server ID "+servername+" already exists on the network! You may want to specify the server ID for the server manually with <server:id> so they do not conflict.");
+		this->SendError("Server ID "+sid+" already exists! You may want to specify the server ID for the server manually with <server:id> so they do not conflict.");
 		this->ServerInstance->SNO->WriteToSnoMask('L', "Server \2"+servername+"\2 being introduced from \2" + ParentOfThis->GetName() + "\2 denied, server ID already exists on the network. Closing link with " + ParentOfThis->GetName());
 		return false;
 	}
 
+
+	Link* lnk = Utils->FindLink(servername);
+
+	TreeServer *Node = new TreeServer(this->Utils, this->ServerInstance, servername, description, sid, ParentOfThis,NULL, lnk ? lnk->Hidden : false);
+
 	ParentOfThis->AddChild(Node);
 	params[4] = ":" + params[4];
 	Utils->DoOneToAllButSender(prefix,"SERVER",params,prefix);
@@ -141,6 +142,13 @@ bool TreeSocket::Outbound_Reply_Server(std::deque<std::string> &params)
 			this->ServerInstance->SNO->WriteToSnoMask('l',"Server connection from \2"+sname+"\2 denied, already exists on server "+CheckDupe->GetParent()->GetName());
 			return false;
 		}
+		CheckDupe = Utils->FindServer(sid);
+		if (CheckDupe)
+		{
+			this->SendError("Server ID "+sid+" already exists on the network! You may want to specify the server ID for the server manually with <server:id> so they do not conflict.");
+			this->ServerInstance->SNO->WriteToSnoMask('l',"Server \2"+assign(servername)+"\2 being introduced denied, server ID already exists on the network. Closing link.");
+			return false;
+		}
 
 		/*
 		 * They're in WAIT_AUTH_2 (having accepted our credentials).
@@ -156,13 +164,6 @@ bool TreeSocket::Outbound_Reply_Server(std::deque<std::string> &params)
 
 		TreeServer *Node = new TreeServer(this->Utils, this->ServerInstance, sname, description, sid, Utils->TreeRoot, this, x->Hidden);
 
-		if (Node->DuplicateID())
-		{
-			this->SendError("Server ID "+sid+" already exists on the network! You may want to specify the server ID for the server manually with <server:id> so they do not conflict.");
-			this->ServerInstance->SNO->WriteToSnoMask('l',"Server \2"+assign(servername)+"\2 being introduced denied, server ID already exists on the network. Closing link.");
-			return false;
-		}
-
 		Utils->TreeRoot->AddChild(Node);
 		params[4] = ":" + params[4];
 
@@ -232,18 +233,6 @@ bool TreeSocket::Inbound_Server(std::deque<std::string> &params)
 			continue;
 		}
 
-		/* Check for fully initialized instances of the server by id */
-		ServerInstance->Logs->Log("m_spanningtree",DEBUG,"Looking for dupe SID %s", sid.c_str());
-		TreeServer* CheckDupeSID = Utils->FindServerID(sid);
-
-		if (CheckDupeSID)
-		{
-			this->SendError("Server ID "+CheckDupeSID->GetID()+" already exists on server "+CheckDupeSID->GetName()+"! You may want to specify the server ID for the server manually with <server:id> so they do not conflict.");
-			this->ServerInstance->SNO->WriteToSnoMask('l',"Server connection from \2"+sname+"\2 denied, server ID '"+CheckDupeSID->GetID()+
-					"' already exists on server "+CheckDupeSID->GetName());
-			return false;
-		}
-
 		/* Now check for fully initialized ServerInstances of the server by name */
 		TreeServer* CheckDupe = Utils->FindServer(sname);
 		if (CheckDupe)
@@ -253,6 +242,19 @@ bool TreeSocket::Inbound_Server(std::deque<std::string> &params)
 			return false;
 		}
 
+		/* Check for fully initialized instances of the server by id */
+		ServerInstance->Logs->Log("m_spanningtree",DEBUG,"Looking for dupe SID %s", sid.c_str());
+		CheckDupe = Utils->FindServerID(sid);
+
+		if (CheckDupe)
+		{
+			this->SendError("Server ID "+CheckDupe->GetID()+" already exists on server "+CheckDupe->GetName()+"! You may want to specify the server ID for the server manually with <server:id> so they do not conflict.");
+			this->ServerInstance->SNO->WriteToSnoMask('l',"Server connection from \2"+sname+"\2 denied, server ID '"+CheckDupe->GetID()+
+					"' already exists on server "+CheckDupe->GetName());
+			return false;
+		}
+
+
 		this->ServerInstance->SNO->WriteToSnoMask('l',"Verified incoming server connection from \002"+sname+"\002["+(x->HiddenFromStats ? "<hidden>" : this->GetIP())+"] ("+description+")");
 		if (this->Hook)
 		{
diff --git a/src/modules/m_spanningtree/treeserver.cpp b/src/modules/m_spanningtree/treeserver.cpp
index 49d345383..6c3e0f87d 100644
--- a/src/modules/m_spanningtree/treeserver.cpp
+++ b/src/modules/m_spanningtree/treeserver.cpp
@@ -32,7 +32,7 @@ TreeServer::TreeServer(SpanningTreeUtilities* Util, InspIRCd* Instance, const st
 	VersionString.clear();
 	ServerUserCount = ServerOperCount = 0;
 	StartBurst = rtt = 0;
-	Warned = Hidden = DupError = false;
+	Warned = Hidden = false;
 	VersionString = ServerInstance->GetVersionString();
 	SetID(id);
 }
@@ -52,7 +52,7 @@ TreeServer::TreeServer(SpanningTreeUtilities* Util, InspIRCd* Instance, std::str
 	Route = NULL;
 	Socket = NULL; /* Fix by brain */
 	StartBurst = rtt = 0;
-	Warned = Hidden = DupError = false;
+	Warned = Hidden = false;
 	AddHashEntry();
 	SetID(id);
 }
@@ -69,7 +69,7 @@ TreeServer::TreeServer(SpanningTreeUtilities* Util, InspIRCd* Instance, std::str
 	ServerUserCount = ServerOperCount = 0;
 	this->SetNextPingTime(ServerInstance->Time() + Utils->PingFreq);
 	this->SetPingFlag();
-	Warned = DupError = false;
+	Warned = false;
 	StartBurst = rtt = 0;
 
 	timeval t;
@@ -156,16 +156,7 @@ void TreeServer::SetID(const std::string &id)
 {
 	ServerInstance->Logs->Log("m_spanningtree",DEBUG, "Setting SID to " + id);
 	sid = id;
-	server_hash::iterator iter = Utils->sidlist.find(sid);
-	if (iter == Utils->sidlist.end())
-		Utils->sidlist[sid] = this;
-	else
-		DupError = true;
-}
-
-bool TreeServer::DuplicateID()
-{
-	return DupError;
+	Utils->sidlist[sid] = this;
 }
 
 int TreeServer::QuitUsers(const std::string &reason)
diff --git a/src/modules/m_spanningtree/treeserver.h b/src/modules/m_spanningtree/treeserver.h
index fea17b7d2..07e586c62 100644
--- a/src/modules/m_spanningtree/treeserver.h
+++ b/src/modules/m_spanningtree/treeserver.h
@@ -44,7 +44,6 @@ class TreeServer : public classbase
 	bool LastPingWasGood;			/* True if the server responded to the last PING with a PONG */
 	SpanningTreeUtilities* Utils;		/* Utility class */
 	std::string sid;			/* Server ID */
-	bool DupError;				/* True if the server ID is duplicated (!) */
 
 	/** Set server ID
 	 * @param id Server ID
@@ -194,10 +193,6 @@ class TreeServer : public classbase
 	 */
 	std::string& GetID();
 
-	/** True on duplicate server ID (server not usable)
-	 */
-	bool DuplicateID();
-
 	/** Marks a server as having finished bursting and performs appropriate actions.
 	 */
 	void FinishBurst();
diff --git a/src/modules/m_spanningtree/treesocket2.cpp b/src/modules/m_spanningtree/treesocket2.cpp
index c006344ba..0fa8dd31a 100644
--- a/src/modules/m_spanningtree/treesocket2.cpp
+++ b/src/modules/m_spanningtree/treesocket2.cpp
@@ -180,13 +180,6 @@ bool TreeSocket::ProcessLine(std::string &line)
 
 				Node = new TreeServer(this->Utils, this->ServerInstance, InboundServerName, InboundDescription, InboundSID, Utils->TreeRoot, this, lnk ? lnk->Hidden : false);
 
-				if (Node->DuplicateID())
-				{
-					this->SendError("Server ID "+InboundSID+" already exists on the network!");
-					this->ServerInstance->SNO->WriteToSnoMask('l',"Server \2"+InboundServerName+"\2 being introduced from \2" + prefix + "\2 denied, server ID already exists on the network. Closing link.");
-					return false;
-				}
-
 				Utils->TreeRoot->AddChild(Node);
 				params.clear();
 				params.push_back(InboundServerName);