innd 2.2.1 and 2.2 crash on invalid Distribution: header

[Tomasz R. Surmacz]

server versions:

200 nntp-papaja.wroc.apk.net InterNetNews server INN 2.2.1 25-Aug-1999 ready
200 plonk.apk.net InterNetNews server INN 2.2 21-Jan-1999 ready

Problem:

If newsfeeds file contains a list of distributions, ie:

	ME:*/world,pl,pl-news,wroc::

and an article arrives (using IHAVE protocol from another peer) that has the
following header:

	Distribution: ,

then the server crashes in MaxLength (p=0x0, q=0x0) at innd.c:136, called from
ARTpost at art.c:2292, where distribution!=NULL, but (*distribution)==NULL, and
in the following code:

innd/art.c:
  2289      if (distributions) {
  2290          DISTparse(distributions, &Data);
  2291          if (ME.Distributions
  2292           && !DISTwantany(ME.Distributions, distributions)) {
  2293              (void)sprintf(buff, "%d Unwanted distribution \"%s\"",
  2294                      NNTP_REJECTIT_VAL,
  2295                      MaxLength(distributions[0], distributions[0]));
  2296              ARTlog(&Data, ART_REJECT, buff);

the check in line 2289 does not prevent passing a NULL pointer to MaxLength
in line 2295.

The patch is included at the end.  It seems, that in such case (the
Distribution: header is invalid, according to RFC1036, as a comma should
separate two distributions), but nevertheless it should not crash the
server.

Actually, the problem was spotted when UUNET servers tried to send article
<942925094.1792snx at wang.pc.my> to news.apk.net causing it to crash with a
segfault and a core dump.

I have succesfully reproduced it on my home server running innd 2.2.1, as below:

papaja 4 ~> telnet localhost 119
Trying 127.0.0.1...
Connected to localhost.
Escape character is '^]'.
200 nntp-papaja.wroc.apk.net InterNetNews server INN 2.2.1 25-Aug-1999 ready
ihave <1 at abc>
335
Message-ID: <1 at abc>
Path: local
From: <tester at local.host>
Subject: test 1
Date: Fri, 12 Nov 1999 12:18:24 GMT
Newsgroups: pl.test
Distribution: ,
Lines: 1
 
.
Connection closed by foreign host.

as you can see, it closed the connection crashing, instead of accepting
the article.  Here is the patch:

--- art.c.orig	Mon May  3 19:58:11 1999
+++ art.c	Thu Nov 18 00:15:40 1999
@@ -2285,7 +2287,7 @@
     /* If we limit what distributions we get, see if we want this one. */
     p = HDR(_distribution);
     distributions = *p ? CommaSplit(p) : NULL;
-    if (distributions) {
+    if (distributions && distributions[0]) {
 	DISTparse(distributions, &Data);
 	if (ME.Distributions
 	 && !DISTwantany(ME.Distributions, distributions)) {


Also, first attempts to solve the problem showed me that innd was crashing
on calling memcpy() with NULL pointer and i=0 in the following code,
so it should also be patched.

--- art.c.orig	Mon May  3 19:58:11 1999
+++ art.c	Thu Nov 18 00:15:40 1999
@@ -850,8 +850,10 @@
 	return NULL;
     }
     hp->Length = i;
-    (void)memcpy((POINTER)hp->Value, (POINTER)p, (SIZE_T)i);
-    hp->Value[i] = '\0';
+    if (i>0 && hp->Value) {
+	(void)memcpy((POINTER)hp->Value, (POINTER)p, (SIZE_T)i);
+	hp->Value[i] = '\0';
+    }
 
     return in;
 }

Tomasz Surmacz

-- 
 _________
(_   _' __) Tomasz R. Surmacz *--* Work:(071)320-2752 tsurmacz at ict.pwr,wroc.pl
  |  (__  \ http://www.ict.pwr.wroc.pl/~tsurmacz/ *---* Home: ts @wroc,apk,net
  |__(____/ For PGP key finger tsurmacz at asic.ict,pwr,wroc.pl *---* irc: TomekS