Date: Tue, 17 Dec 1996 22:59:56 -0500
Message-Id: <199612180359.WAA12075@daedalus.crosslink.net>
From: shields@crosslink.net (Michael Shields)
To: inn-workers@vix.com
Subject: Null pointer dereference in 1.5 (and 1.5.1)
Sender: owner-inn-workers@vix.com
Precedence: bulk

Today the news servers on both my read machines started to die
repeatedly and simultaneously, logging nothing.  This happened once
two or three weeks ago when I wasn't around; at the time it cleared up
mysteriously after a while.

Today I finally tracked this down.  It was caused by an article with a
1060-character Path: header.  Because INN always writes out Path:
before Message-ID:, the receiving machines receive that header first
(and simultaneously since they're fed by nntplink), and ARTclean()
sees the too-long header it stops parsing before it learns the
Message-ID:.  Then, iff you have DO_REMEMBER_TRASH set, HISwrite gets
called to log the article, with a null pointer for the message-ID.
Boom.  Of course the reason that it died repeatedly is that the master
feed machine kept trying to resend it, and the reason that it clears
up eventually is that the backlog gets trimmed or the article expires
(it's a short-expire feed-only server).

I'm cc'ing this to inn-workers because it would actually be optimal if
trash were not remembered when an article is rejected for its path --
since that header is modified in transit, the article might arrive
again by another route that caused it to be acceptable.  This could
apply to the exclusions list also.

This could also be used as a denial-of-service attack.

Here's the patch.  This is against 1.5 but 1.5.1 looks like it has the
same bug.

Index: art.c
===================================================================
RCS file: /usr/src/master/inn/innd/art.c,v
retrieving revision 1.9
diff -u -r1.9 art.c
--- art.c	1996/12/12 00:39:01	1.9
+++ art.c	1996/12/18 03:41:30
@@ -1784,7 +1784,7 @@
 	(void)sprintf(buff, "%d %s", NNTP_REJECTIT_VAL, error);
 	ARTlog(&Data, ART_REJECT, buff);
 #if	defined(DO_REMEMBER_TRASH)
-        if (Mode == OMrunning && !HISwrite(&Data, ""))
+        if (Data.MessageID && Mode == OMrunning && !HISwrite(&Data, ""))
             syslog(L_ERROR, "%s cant write history %s %m",
                    LogName, Data.MessageID);
 #endif	/* defined(DO_REMEMBER_TRASH) */
@@ -1798,7 +1798,7 @@
         syslog(L_NOTICE, "rejecting[perl] %s %s", HDR(_message_id), buff);
         ARTlog(&Data, ART_REJECT, buff);
 #if	defined(DO_REMEMBER_TRASH)
-        if (Mode == OMrunning && !HISwrite(&Data, ""))
+        if (Data.MessageID && Mode == OMrunning && !HISwrite(&Data, ""))
             syslog(L_ERROR, "%s cant write history %s %m",
                    LogName, Data.MessageID);
 #endif	/* defined(DO_REMEMBER_TRASH) */
@@ -1842,7 +1842,7 @@
                        buff);
 		ARTlog(&Data, ART_REJECT, buff);
 #if	defined(DO_REMEMBER_TRASH)
-                if (Mode == OMrunning && !HISwrite(&Data, ""))
+                if (Data.MessageID && Mode == OMrunning && !HISwrite(&Data, ""))
                     syslog(L_ERROR, "%s cant write history %s %m",
                            LogName, Data.MessageID);
 #endif	/* defined(DO_REMEMBER_TRASH) */

-- 
Shields, CrossLink.