Path: utzoo!utgpu!news-server.csri.toronto.edu!cs.utexas.edu!sun-barr!apple!agate!shelby!rutgers!ub!dsinc!syd
From: syd@DSI.COM (Syd Weinstein)
Newsgroups: comp.mail.elm
Subject: Re: Fatal bus error condition under elm 2.3 pl9
Summary: I'm going to regret this reply.....
Message-ID: <1990Dec11.204412.27600@DSI.COM>
Date: 11 Dec 90 20:44:12 GMT
References: <1990Dec10.204010.7183@wubios.wustl.edu> <1990Dec10.225957.4587@macc.wisc.edu> <2X6LL4K@geminix.in-berlin.de>
Followup-To: poster
Organization: Datacomp Systems, Inc., Huntingdon Valley, PA 19006
Lines: 88

In article <2X6LL4K@geminix.in-berlin.de> gemini@geminix.in-berlin.de (Uwe Doering) writes:
>anderson@udder.macc.wisc.edu (Jess Anderson) writes:
>Now I wonder why there is no protection against buffer overflow at this
>point. And I don't understand why Elm uses static rather than dynamically
>allocated buffers to store header lines. I really don't feel comfortable
>knowing that other people (who create the header lines) can decide whether
>my mailer breaks or not. One simply can't rely on a miximum header line
>length.

I was going to let this pass, but I cannot, after I found the problem.

Why is their no protection?
But their was, the call that overflowed was a strncat, which did have
a length check.

Why didn't it work?
Because a line got deleted that updated the length during my testing
and re-assembly to check in the code prior to a patch.

Why does Elm uses static (actually on the stack) buffers and not dynamic
ones?
Because Dave Taylor wrote it that way.

Note, it wouldn't have broken if my boundry check didn't get messed
up by the line deletion.

Note, if you noticed that the problem was due to that buffer being
overwritten, why didn't you notice that the length check wasn't
getting the proper length? :-)

Anyway, folks, its a one line patch to fix this one, I am debating whether
to just release the one line patch as patch 10, or bundle more stuff in
with it.  Their is plenty in to in-box awaiting handling.  For those
interested here is the unofficial version of the diff (note the one
line code change and the mucho doc change just to support it):

Index: src/returnadd.c
Prereq: 4.1.1.2
*** ../elm2.3/src/returnadd.c	Wed Dec  5 23:42:09 1990
--- src/returnadd.c	Tue Dec 11 15:36:10 1990
***************
*** 1,8 ****
  
! static char rcsid[] = "@(#)$Id: returnadd.c,v 4.1.1.2 90/12/05 22:33:58 syd Exp $";
  
  /*******************************************************************************
!  *  The Elm Mail System  -  $Revision: 4.1.1.2 $   $State: Exp $
   *
   * 			Copyright (c) 1986, 1987 Dave Taylor
   * 			Copyright (c) 1988, 1989, 1990 USENET Community Trust
--- 1,8 ----
  
! static char rcsid[] = "@(#)$Id: returnadd.c,v 4.1.1.3 90/12/11 15:35:56 syd Exp $";
  
  /*******************************************************************************
!  *  The Elm Mail System  -  $Revision: 4.1.1.3 $   $State: Exp $
   *
   * 			Copyright (c) 1986, 1987 Dave Taylor
   * 			Copyright (c) 1988, 1989, 1990 USENET Community Trust
***************
*** 14,19 ****
--- 14,23 ----
   *
   *******************************************************************************
   * $Log:	returnadd.c,v $
+  * Revision 4.1.1.3  90/12/11  15:35:56  syd
+  * Add back missing  strlen line, fixes segv problem
+  * From: Syd
+  * 
   * Revision 4.1.1.2  90/12/05  22:33:58  syd
   * Fix missing close brace due to indention error
   * From: Syd
***************
*** 295,300 ****
--- 299,305 ----
  	      buf[len_buf] = '\0';
  	    }
  	    strncat(buf, buf2, (SLEN-len_buf-1));
+ 	    len_buf = strlen(buf);
  	    ok = (int) (fgets(buf2, SLEN, mailfile) != NULL);
  	    if (ok) {
  	      len_buf2 = strlen(buf2);

-- 
=====================================================================
Sydney S. Weinstein, CDP, CCP                   Elm Coordinator
Datacomp Systems, Inc.                          Voice: (215) 947-9900
syd@DSI.COM or dsinc!syd                        FAX:   (215) 938-0235