Subject: DEFECT: expireover in INN 1.7, 1.6b3 (patched)

From: [email-masked] (Forrest J. Cavalier III)
Date: 5 Nov 1997 20:31:57 GMT

Defect in: expire/expireover.c
    The defect is in the expireover-memleak patch that Mib Software
    inserted into 1.5.1corr (and 1.7) to fix a very large memory leak
    in the 1.6b3 fixes.

Symptoms:   Under some circumstances, the defective code can
    attempt to free an invalid memory pointer.  This causes 
    expireover to fail (dump core) which will prevent expireover
    from completing normally.

Versions affected: Versions: 1.5.1corr and derivatives, including 1.7
    1.6b3 with the expireover-memleak patch) and 1.5.2.  Systems which
    are not running overviews are not affected.

The defect was discovered and reported by Mike Brudenell
<[email-masked]> to [email-masked].  Characterization,
the patch, and further details are provided here by Mib Software.

I already sent out a similar message to Usenet RKT subscribers
a couple of days ago, and expected Mike (or James Brister/ISC)
to get something out to the "rest of the world."

After a further e-mail message, (which uncovered another minor
defect in the same code) Mike Brudenell said that he was too
busy to get something out quickly, and asked me to do it....

Although this defect is not a security threat, it is not obvious
when expireover fails.  It is not easy to identify systems which
definitely trigger or do not trigger the defect.  A large number
of .overview files could remain not updated, slowing performance
and requiring larger disk storage usage than necessary.  It may
also interfere with news.daily running to completion.

Background:
    Mike Brudenell did correctly indicate which line was defective,
    and how it could be fixed.  I indicated a different fix which
    matched the semantics of the code.  Mike Brudenell then pointed
    out that there would STILL be a memory leak in the case of
    empty headers.  (But this would not be nearly as large as the
    original leak the patch fixed.)

    INN is not supposed to write empty headers to the spool.  There is
    code in art.c which prevents that.  But just to be safe, the
    following patch includes a fix for all of the defects discussed.

This message, including the patch, will also be available at:
   http://www.mibsoftware.com/userkt/inn/patches/eo971105.msg

For a list of known defects in release versions of inn 1.5.1 to 1.7,
see:
    http://www.mibsoftware.com/userkt/inn/patches/

Forrest J. Cavalier III, Mib Software

*** ../inn-1.7/expire/expireover.c	Tue Oct  7 11:17:57 1997
--- expire/expireover.c	Wed Nov  5 14:59:56 1997
***************
*** 438,444 ****
  	   caller's use.  mibsoft 8/22/97
  	 */
  	for (hp = Headers, i = ARTfieldsize; --i >= 0; hp++) {
! 	    if (hp->Header) {
  		DISPOSE(hp->Header);
  		hp->Header = 0;
  	    }
--- 438,444 ----
  	   caller's use.  mibsoft 8/22/97
  	 */
  	for (hp = Headers, i = ARTfieldsize; --i >= 0; hp++) {
! 	    if (hp->Length) { /* Was hp->Header. Fix due to [email-masked] 11/3/97 */
  		DISPOSE(hp->Header);
  		hp->Header = 0;
  	    }
***************
*** 481,486 ****
--- 481,491 ----
  		for (p = colon; *++p && ISWHITE(*p); )
  		    continue;
  	    size = strlen(p);
+ 	    if (!size) { /* Ignore empty headers 11/5/97 due to [email-masked] */
+ 		i = -1; /* Abort */
+ 		lasthp = 0;
+ 		break;
+ 	    }
              hp->Length = size;
              hp->Header = NEW(char, hp->Length + 1);
  	    (void)strcpy(hp->Header, p);