Make server-to-server protocol more robust

Now ngIRCd catches more errors on the server-to-server (S2S) protocol that could crash the daemon before. This hasn't been a real problem because the IRC S2S protocol is "trusted" by design, but the behavior is much better now. Thanks to wowaname on #ngircd for pointing this out!
ngircd · Sep 3, 2015 · eb9929e · eb9929e
1 parent c2e4c30
commit eb9929e
Show file tree

Hide file tree

Showing 6 changed files with 46 additions and 16 deletions.
diff --git a/src/ngircd/irc-login.c b/src/ngircd/irc-login.c
@@ -1,6 +1,6 @@
 /*
  * ngIRCd -- The Next Generation IRC Daemon
- * Copyright (c)2001-2014 Alexander Barton (alex@barton.de) and Contributors.
+ * Copyright (c)2001-2015 Alexander Barton (alex@barton.de) and Contributors.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -196,6 +196,7 @@ IRC_NICK( CLIENT *Client, REQUEST *Req )
 
 		/* Search "target" client */
 		if (Client_Type(Client) == CLIENT_SERVER) {
+			_IRC_REQUIRE_PREFIX_OR_RETURN_(Client, Req)
 			target = Client_Search(Req->prefix);
 			if (!target)
 				return IRC_WriteErrClient(Client,
@@ -379,6 +380,8 @@ IRC_SVSNICK(CLIENT *Client, REQUEST *Req)
 	assert(Client != NULL);
 	assert(Req != NULL);
 
+	_IRC_REQUIRE_PREFIX_OR_RETURN_(Client, Req)
+
 	/* Search the originator */
 	from = Client_Search(Req->prefix);
 	if (!from)
@@ -481,6 +484,7 @@ IRC_USER(CLIENT * Client, REQUEST * Req)
 		   Client_Type(Client) == CLIENT_SERVICE) {
 		/* Server/service updating an user */
 		_IRC_ARGC_EQ_OR_RETURN_(Client, Req, 4)
+		_IRC_REQUIRE_PREFIX_OR_RETURN_(Client, Req)
 
 		c = Client_Search(Req->prefix);
 		if (!c)
@@ -653,6 +657,8 @@ IRC_QUIT( CLIENT *Client, REQUEST *Req )
 
 	if (Client_Type(Client) == CLIENT_SERVER) {
 		/* Server */
+		_IRC_REQUIRE_PREFIX_OR_RETURN_(Client, Req)
+
 		target = Client_Search(Req->prefix);
 		if (!target) {
 			Log(LOG_WARNING,
@@ -741,9 +747,10 @@ IRC_PING(CLIENT *Client, REQUEST *Req)
 
 		if (target != Client_ThisServer()) {
 			/* Ok, we have to forward the PING */
-			if (Client_Type(Client) == CLIENT_SERVER)
+			if (Client_Type(Client) == CLIENT_SERVER) {
+				_IRC_REQUIRE_PREFIX_OR_RETURN_(Client, Req)
 				from = Client_Search(Req->prefix);
-			else
+			} else
 				from = Client;
 			if (!from)
 				return IRC_WriteErrClient(Client,
@@ -815,6 +822,8 @@ IRC_PONG(CLIENT *Client, REQUEST *Req)
 
 	/* Forward? */
 	if (Req->argc == 2 && Client_Type(Client) == CLIENT_SERVER) {
+		_IRC_REQUIRE_PREFIX_OR_RETURN_(Client, Req)
+
 		target = Client_Search(Req->argv[0]);
 		if (!target)
 			return IRC_WriteErrClient(Client, ERR_NOSUCHSERVER_MSG,

diff --git a/src/ngircd/irc-macros.h b/src/ngircd/irc-macros.h
@@ -1,6 +1,6 @@
 /*
  * ngIRCd -- The Next Generation IRC Daemon
- * Copyright (c)2001-2013 Alexander Barton (alex@barton.de) and Contributors.
+ * Copyright (c)2001-2015 Alexander Barton (alex@barton.de) and Contributors.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -65,6 +65,18 @@ if (Req->argc < Min || Req->argc > Max) { \
 				  Client_ID(Client), Req->command); \
 }
 
+/**
+ * Make sure that the command has a prefix.
+ *
+ * If there is no prefix, send an error to the client and return from
+ * the function.
+ */
+#define _IRC_REQUIRE_PREFIX_OR_RETURN_(Client, Req) \
+if (!Req->prefix) { \
+	return IRC_WriteErrClient(Client, ERR_NEEDMOREPARAMS_MSG, \
+				  Client_ID(Client), Req->command); \
+}
+
 /**
  * Get sender of an IRC command.
  *
@@ -73,13 +85,17 @@ if (Req->argc < Min || Req->argc > Max) { \
  * send an error to the client and return from the function.
  */
 #define _IRC_GET_SENDER_OR_RETURN_(Sender, Req, Client) \
-	if (Client_Type(Client) == CLIENT_SERVER) \
+	if (Client_Type(Client) == CLIENT_SERVER) { \
+		if (!Req->prefix) \
+			return IRC_WriteErrClient(Client, ERR_NEEDMOREPARAMS_MSG, \
+						  Client_ID(Client), Req->command); \
 		Sender = Client_Search(Req->prefix); \
-	else \
+	} else \
 		Sender = Client; \
 	if (!Sender) \
 		return IRC_WriteErrClient(Client, ERR_NOSUCHNICK_MSG, \
-					  Client_ID(Client), Req->prefix);
+					  Client_ID(Client), \
+					  Req->prefix ? Req->prefix : "(none)");
 
 /**
  * Get target of an IRC command and make sure that it is a server.

diff --git a/src/ngircd/irc-metadata.c b/src/ngircd/irc-metadata.c
@@ -1,6 +1,6 @@
 /*
  * ngIRCd -- The Next Generation IRC Daemon
- * Copyright (c)2001-2014 Alexander Barton (alex@barton.de) and Contributors.
+ * Copyright (c)2001-2015 Alexander Barton (alex@barton.de) and Contributors.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -24,6 +24,7 @@
 
 #include "conn-func.h"
 #include "channel.h"
+#include "irc-macros.h"
 #include "irc-write.h"
 #include "log.h"
 #include "messages.h"
@@ -47,6 +48,8 @@ IRC_METADATA(CLIENT *Client, REQUEST *Req)
 	assert(Client != NULL);
 	assert(Req != NULL);
 
+	_IRC_REQUIRE_PREFIX_OR_RETURN_(Client, Req)
+
 	prefix = Client_Search(Req->prefix);
 	if (!prefix)
 		return IRC_WriteErrClient(Client, ERR_NOSUCHNICK_MSG,

diff --git a/src/ngircd/irc-oper.c b/src/ngircd/irc-oper.c
@@ -1,6 +1,6 @@
 /*
  * ngIRCd -- The Next Generation IRC Daemon
- * Copyright (c)2001-2014 Alexander Barton (alex@barton.de) and Contributors.
+ * Copyright (c)2001-2015 Alexander Barton (alex@barton.de) and Contributors.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -30,6 +30,7 @@
 #include "class.h"
 #include "parse.h"
 #include "irc.h"
+#include "irc-macros.h"
 #include "irc-write.h"
 #include "lists.h"
 #include "log.h"
@@ -358,6 +359,7 @@ IRC_WALLOPS( CLIENT *Client, REQUEST *Req )
 		from = Client;
 		break;
 	case CLIENT_SERVER:
+		_IRC_REQUIRE_PREFIX_OR_RETURN_(Client, Req)
 		from = Client_Search(Req->prefix);
 		break;
 	default:

diff --git a/src/ngircd/irc.c b/src/ngircd/irc.c
@@ -1,6 +1,6 @@
 /*
  * ngIRCd -- The Next Generation IRC Daemon
- * Copyright (c)2001-2014 Alexander Barton (alex@barton.de) and Contributors.
+ * Copyright (c)2001-2015 Alexander Barton (alex@barton.de) and Contributors.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -526,7 +526,7 @@ Send_Message(CLIENT * Client, REQUEST * Req, int ForceType, bool SendErrors)
 					  Client_ID(Client), Req->command);
 	}
 
-	if (Client_Type(Client) == CLIENT_SERVER)
+	if (Client_Type(Client) == CLIENT_SERVER && Req->prefix)
 		from = Client_Search(Req->prefix);
 	else
 		from = Client;

diff --git a/src/ngircd/parse.c b/src/ngircd/parse.c
@@ -1,6 +1,6 @@
 /*
  * ngIRCd -- The Next Generation IRC Daemon
- * Copyright (c)2001-2014 Alexander Barton (alex@barton.de) and Contributors.
+ * Copyright (c)2001-2015 Alexander Barton (alex@barton.de) and Contributors.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -461,10 +461,10 @@ Handle_Numeric(CLIENT *client, REQUEST *Req)
 	}
 
 	/* Determine source */
-	if (! Req->prefix[0]) {
-		/* Oops, no prefix!? */
-		Log(LOG_WARNING, "Got status code %s from \"%s\" without prefix!?",
-						Req->command, Client_ID(client));
+	if (!Req->prefix) {
+		Log(LOG_WARNING,
+		    "Got status code %s from \"%s\" without prefix!?",
+		    Req->command, Client_ID(client));
 		return true;
 	}