ngIRCd Release 20.3

There have been code paths that ignored the return code of Handle_Write()
when sending "notice auth" messages to new clients connecting to the
server. But because Handle_Write() would have closed the client connection
again if an error occurred, this would have resulted in new errors and
assert()'s later on that could have crashed the server (denial of service).

Only setups having the configuration option "NoticeAuth" enabled are
affected, which is not the default.

CVE-2013-5580.

(cherry picked from commit 309122017e)

ChangeLog

@@ -9,6 +9,43 @@
                                -- ChangeLog --
 
 
+ngIRCd 20.3 (2013-08-23)
+
+  - Security: Fix a denial of service bug (server crash) which could happen
+    when the configuration option "NoticeAuth" is enabled (which is NOT the
+    default) and ngIRCd failed to send the "notice auth" messages to new
+    clients connecting to the server (CVE-2013-5580).
+
+ngIRCd 20.2 (2013-02-15)
+
+  - Security: Fix a denial of service bug in the function handling KICK
+    commands that could be used by arbitrary users to to crash the daemon
+    (CVE-2013-1747).
+  - WHO command: Use the currently "displayed hostname" (which can be cloaked!)
+    for hostname matching, not the real one. In other words: don't display all
+    the cloaked users on a specific real hostname!
+  - configure: The header file "netinet/in_systm.h" already is optional in
+    ngIRCd, so don't require it in the configure script. Now ngIRCd can be
+    built on Minix 3 again :-)
+  - Return better "Connection not registered as server link" errors: Now ngIRCd
+    returns a more specific error message for numeric ERR_NOTREGISTERED(451)
+    when a regular user tries to use a command that isn't allowed for users but
+    for servers.
+  - Don't report ERR_NEEDMOREPARAMS(461) when a MDOE command with more modes
+    than nicknames is handled, as well as for channel limit and key changes
+    without specifying the limit or key parameters.
+    This is how a lot (all?) other IRC servers behave, including ircd2.11,
+    InspIRCd, and ircd-seven. And because of clients (tested with Textual and
+    mIRC) sending bogus MODE commands like "MODE -ooo nick", end-users got the
+    expected result as well as correct but misleading error messages ...
+  - Correctly detect when SSL subsystem must be initialized and take
+    outgoing connections (server links!) into account, too.
+  - autogen.sh: Enforce serial test harness on GNU automake >=1.13. The
+    new parallel test harness which is enabled by default starting with
+    automake 1.13 isn't compatible with our test suite.
+    And don't use "egrep -o", insetead use "sed", because it isn't portable
+    and not available on OpenBSD, for example.
+
 ngIRCd 20.1 (2013-01-02)
 
   - Allow ERROR command on server and service links only, ignore them and

NEWS

@@ -9,6 +9,21 @@
                                   -- NEWS --
 
 
+ngIRCd 20.3 (2013-08-23)
+
+  - This release is a bugfix release only, without new features.
+  - Security: Fix a denial of service bug (server crash) which could happen
+    when the configuration option "NoticeAuth" is enabled (which is NOT the
+    default) and ngIRCd failed to send the "notice auth" messages to new
+    clients connecting to the server (CVE-2013-5580).
+
+ngIRCd 20.2 (2013-02-15)
+
+  - This release is a bugfix release only, without new features.
+  - Security: Fix a denial of service bug in the function handling KICK
+    commands that could be used by arbitrary users to to crash the daemon
+    (CVE-2013-1747).
+
 ngIRCd 20.1 (2013-01-02)
 
   - This release is a bugfix release only, without new features.

autogen.sh

@@ -153,7 +153,7 @@ echo "Searching for required tools ..."
 [ -z "$AUTOMAKE" ] && Notfound automake
 [ -z "$AUTOCONF" ] && Notfound autoconf
 
-AM_VERSION=`$AUTOMAKE --version|head -n 1|egrep -o "([1-9]\.[0-9]+(\.[0-9]+)*)"`
+AM_VERSION=`$AUTOMAKE --version | head -n 1 | sed -e 's/.* //g'`
 ifs=$IFS; IFS="."; set $AM_VERSION; IFS=$ifs
 AM_MAJOR="$1"; AM_MINOR="$2"; AM_PATCHLEVEL="$3"
 
@@ -172,11 +172,21 @@ else
 	DEANSI_START="#"
 	DEANSI_END="	# disabled by ./autogen.sh script"
 fi
-sed -e "s|^__ng_Makefile_am_template__|${DEANSI_START}AUTOMAKE_OPTIONS = ansi2knr${DEANSI_END}|g" \
+# Serial test harness?
+if [ "$AM_MAJOR" -eq "1" -a "$AM_MINOR" -ge "13" ]; then
+	# automake >= 1.13 => enforce "serial test harness"
+	echo " - Enforcing serial test harness."
+	SERIAL_TESTS="serial-tests"
+else
+	# automake < 1.13 => no new test harness, nothing to do
+	SERIAL_TEST=""
+fi
+
+sed -e "s|^__ng_Makefile_am_template__|AUTOMAKE_OPTIONS = ${SERIAL_TESTS} ${DEANSI_START}ansi2knr${DEANSI_END}|g" \
 	src/portab/Makefile.ng >src/portab/Makefile.am
 for makefile_ng in $AM_MAKEFILES; do
 	makefile_am=`echo "$makefile_ng" | sed -e "s|\.ng\$|\.am|g"`
-	sed -e "s|^__ng_Makefile_am_template__|${DEANSI_START}AUTOMAKE_OPTIONS = ../portab/ansi2knr${DEANSI_END}|g" \
+	sed -e "s|^__ng_Makefile_am_template__|AUTOMAKE_OPTIONS = ${SERIAL_TESTS} ${DEANSI_START}../portab/ansi2knr${DEANSI_END}|g" \
 		$makefile_ng >$makefile_am
 done
 

configure.ng

@@ -120,14 +120,14 @@ AC_HEADER_TIME
 
 # Required header files
 AC_CHECK_HEADERS([ \
-	fcntl.h netdb.h netinet/in.h netinet/in_systm.h stdlib.h string.h \
+	fcntl.h netdb.h netinet/in.h stdlib.h string.h \
 	strings.h sys/socket.h sys/time.h unistd.h \
 	],,AC_MSG_ERROR([required C header missing!]))
 
 # Optional header files
 AC_CHECK_HEADERS_ONCE([ \
-	arpa/inet.h inttypes.h malloc.h netinet/ip.h stdbool.h stddef.h \
+	arpa/inet.h inttypes.h malloc.h netinet/in_systm.h netinet/ip.h \
-	stdint.h varargs.h \
+	stdbool.h stddef.h stdint.h varargs.h \
 	])
 
 # -- Datatypes --
@@ -711,7 +711,8 @@ echo $ECHO_N "   libiconv support: $ECHO_C"
 
 echo
 
-if ! grep "^AUTOMAKE_OPTIONS = ../portab/ansi2knr" src/ngircd/Makefile.am >/dev/null 2>&1; then
+define(_automake_regex_,[[^AUTOMAKE_OPTIONS = [a-z .\-]*/portab/ansi2knr]])
+if ! grep "_automake_regex_" src/ngircd/Makefile.am >/dev/null 2>&1; then
 	echo "WARNING:"
 	echo "This GNU automake generated build system does not support \"de-ANSI-fication\","
 	echo "therefore don't use it to generate \"official\" distribution archives!"

contrib/Debian/changelog

@@ -1,3 +1,15 @@
+ngircd (20.3-0ab1) unstable; urgency=high
+
+  * New "upstream" release, fixing a security related bug: ngIRCd 20.3.
+
+ -- Alexander Barton <alex@barton.de>  Fri, 23 Aug 2013 21:53:21 +0200
+
+ngircd (20.2-0ab1) unstable; urgency=high
+
+  * New "upstream" release, fixing a security related bug: ngIRCd 20.2.
+
+ -- Alexander Barton <alex@barton.de>  Fri, 15 Feb 2013 12:17:00 +0100
+
 ngircd (20.1-0ab1) unstable; urgency=low
 
   * New "upstream" release: ngIRCd 20.1.

contrib/ngircd.spec

@@ -1,5 +1,5 @@
 %define name    ngircd
-%define version 20.1
+%define version 20.3
 %define release 1
 %define prefix  %{_prefix}
 

src/ngircd/channel.c

@@ -326,6 +326,13 @@ Channel_Kick(CLIENT *Peer, CLIENT *Target, CLIENT *Origin, const char *Name,
 		}
 	}
 
+	/* Check that the client to be kicked is on the specified channel */
+	if (!Channel_IsMemberOf(chan, Target)) {
+		IRC_WriteStrClient(Origin, ERR_USERNOTINCHANNEL_MSG,
+				   Client_ID(Origin), Client_ID(Target), Name );
+		return;
+	}
+
 	if(Client_Type(Peer) == CLIENT_USER) {
 		/* Channel mode 'Q' and user mode 'q' on target: nobody but
 		 * IRC Operators and servers can kick the target user */
@@ -382,13 +389,6 @@ Channel_Kick(CLIENT *Peer, CLIENT *Target, CLIENT *Origin, const char *Name,
 		}
 	}
 
-	/* Check that the client to be kicked is on the specified channel */
-	if (!Channel_IsMemberOf(chan, Target)) {
-		IRC_WriteStrClient(Origin, ERR_USERNOTINCHANNEL_MSG,
-				   Client_ID(Origin), Client_ID(Target), Name );
-		return;
-	}
-
 	/* Kick Client from channel */
 	Remove_Client( REMOVE_KICK, chan, Target, Origin, Reason, true);
 } /* Channel_Kick */

src/ngircd/conf.c

@@ -108,6 +108,28 @@ ConfSSL_Init(void)
 	array_free(&Conf_SSLOptions.ListenPorts);
 }
 
+/**
+ * Check if the current configuration uses/requires SSL.
+ *
+ * @returns true if SSL is used and should be initialized.
+ */
+GLOBAL bool
+Conf_SSLInUse(void)
+{
+	int i;
+
+	/* SSL listen ports configured? */
+	if (array_bytes(&Conf_SSLOptions.ListenPorts))
+		return true;
+
+	for (i = 0; i < MAX_SERVERS; i++) {
+		if (Conf_Server[i].port > 0
+		    && Conf_Server[i].SSLConnect)
+			return true;
+	}
+	return false;
+}
+
 /**
  * Make sure that a configured file is readable.
  *

src/ngircd/conf.h

@@ -253,6 +253,10 @@ GLOBAL bool Conf_AddServer PARAMS(( const char *Name, UINT16 Port, const char *H
 GLOBAL bool Conf_NickIsService PARAMS((int ConfServer, const char *Nick));
 GLOBAL bool Conf_NickIsBlocked PARAMS((const char *Nick));
 
+#ifdef SSL_SUPPORT
+GLOBAL bool Conf_SSLInUse PARAMS((void));
+#endif
+
 /* Password required by WEBIRC command */
 GLOBAL char Conf_WebircPwd[CLIENT_PASS_LEN];
 

src/ngircd/conn-ssl.c

@@ -241,8 +241,10 @@ void ConnSSL_Free(CONNECTION *c)
 bool
 ConnSSL_InitLibrary( void )
 {
-	if (!array_bytes(&Conf_SSLOptions.ListenPorts))
+	if (!Conf_SSLInUse()) {
+		LogDebug("SSL not in use, skipping initialization.");
 		return true;
+	}
 
 #ifdef HAVE_LIBSSL
 	SSL_CTX *newctx;

src/ngircd/conn.c

@@ -1547,7 +1547,11 @@ Conn_StartLogin(CONN_ID Idx)
 #endif
 			(void)Conn_WriteStr(Idx,
 				"NOTICE AUTH :*** Looking up your hostname");
-		(void)Handle_Write(Idx);
+		/* Send buffered data to the client, but break on errors
+		 * because Handle_Write() would have closed the connection
+		 * again in this case! */
+		if (!Handle_Write(Idx))
+			return;
 	}
 
 	Resolve_Addr(&My_Connections[Idx].proc_stat, &My_Connections[Idx].addr,
@@ -2339,8 +2343,13 @@ cb_Read_Resolver_Result( int r_fd, UNUSED short events )
 		}
 #endif
 
-		if (Conf_NoticeAuth)
+		if (Conf_NoticeAuth) {
-			(void)Handle_Write(i);
+			/* Send buffered data to the client, but break on
+			 * errors because Handle_Write() would have closed
+			 * the connection again in this case! */
+			if (!Handle_Write(i))
+				return;
+		}
 
 		Class_HandleServerBans(c);
 	}

src/ngircd/irc-info.c

@@ -950,7 +950,7 @@ IRC_WHO_Mask(CLIENT *Client, char *Mask, bool OnlyOps)
 		if (Mask) {
 			/* Match pattern against user host/server/name/nick */
 			client_match = MatchCaseInsensitive(Mask,
-						Client_Hostname(c));
+						Client_HostnameDisplayed(c));
 			if (!client_match)
 				client_match = MatchCaseInsensitive(Mask,
 						Client_ID(Client_Introducer(c)));

src/ngircd/irc-mode.c

@@ -628,9 +628,13 @@ Channel_Mode(CLIENT *Client, REQUEST *Req, CLIENT *Origin, CHANNEL *Channel)
 				Req->argv[arg_arg][0] = '\0';
 				arg_arg++;
 			} else {
+#ifdef STRICT_RFC
+				/* Only send error message in "strict" mode,
+				 * this is how ircd2.11 and others behave ... */
 				connected = IRC_WriteStrClient(Origin,
 					ERR_NEEDMOREPARAMS_MSG,
 					Client_ID(Origin), Req->command);
+#endif
 				goto chan_exit;
 			}
 			break;
@@ -668,9 +672,13 @@ Channel_Mode(CLIENT *Client, REQUEST *Req, CLIENT *Origin, CHANNEL *Channel)
 				Req->argv[arg_arg][0] = '\0';
 				arg_arg++;
 			} else {
+#ifdef STRICT_RFC
+				/* Only send error message in "strict" mode,
+				 * this is how ircd2.11 and others behave ... */
 				connected = IRC_WriteStrClient(Origin,
 					ERR_NEEDMOREPARAMS_MSG,
 					Client_ID(Origin), Req->command);
+#endif
 				goto chan_exit;
 			}
 			break;
@@ -761,9 +769,17 @@ Channel_Mode(CLIENT *Client, REQUEST *Req, CLIENT *Origin, CHANNEL *Channel)
 				Req->argv[arg_arg][0] = '\0';
 				arg_arg++;
 			} else {
+#ifdef STRICT_RFC
+				/* Report an error to the client, when a user
+				 * mode should be changed but no nickname is
+				 * given. But don't do it when not in "strict"
+				 * mode, because most other servers don't do
+				 * it as well and some clients send "wired"
+				 * MODE commands like "MODE #chan -ooo nick". */
 				connected = IRC_WriteStrClient(Origin,
 					ERR_NEEDMOREPARAMS_MSG,
 					Client_ID(Origin), Req->command);
+#endif
 				goto chan_exit;
 			}
 			break;

src/ngircd/parse.c

@@ -514,10 +514,20 @@ Handle_Request( CONN_ID Idx, REQUEST *Req )
 			continue;
 		}
 
-		if (!(client_type & cmd->type))
+		if (!(client_type & cmd->type)) {
-			return IRC_WriteStrClient(client, ERR_NOTREGISTERED_MSG, Client_ID(client));
+			if (client_type == CLIENT_USER
+			    && cmd->type & CLIENT_SERVER)
+				return IRC_WriteStrClient(client,
+						 ERR_NOTREGISTEREDSERVER_MSG,
+						 Client_ID(client));
+			else
+				return IRC_WriteStrClient(client,
+						ERR_NOTREGISTERED_MSG,
+						Client_ID(client));
+		}
 
-		/* Command is allowed for this client: call it and count produced bytes */
+		/* Command is allowed for this client: call it and count
+		 * generated bytes in output */
 		Conn_ResetWCounter();
 		result = (cmd->function)(client, Req);
 		cmd->bytes += Conn_WCounter();