[comp.mail.misc] Bug in smail involving MAXCLEN

sullivan@aqdata.uucp (Michael T. Sullivan) (03/13/90)

I recently started a mailing list so I wanted to bump up MAXCLEN in our
smail 2.5 to keep the number of uux calls down.  I bumped it up
and instead of keeping the calls down it actually started generating a
uux for just about each member of the alias I had set up.  After much
digging, I found out that MAXCLEN is pretty much not used in smail.
Because of this, buffers were overflowing and variables were coming
out weird.  Here is a diff of my changes to deliver.c:

67,69c67,69
< 	char lcommand[SMLBUF];		/* local command issued 	*/
< 	char rcommand[SMLBUF];		/* remote command issued	*/
< 	char scommand[SMLBUF];		/* retry  command issued	*/
---
> 	char lcommand[MAXCLEN];		/* local command issued 	*/
> 	char rcommand[MAXCLEN];		/* remote command issued	*/
> 	char scommand[MAXCLEN];		/* retry  command issued	*/
163a164
> 			 || ((send - scommand) > MAXCLEN)
-- 
Michael Sullivan          uunet!jarthur!aqdata!sullivan
aQdata, Inc.              sullivan@aqdata.uucp
San Dimas, CA             +1 714 599 9992

rhg@cpsolv.CPS.COM (Richard H. Gumpertz) (04/08/90)

In article <1990Mar12.235134.22019@aqdata.uucp> sullivan@aqdata.uucp (Michael
T. Sullivan) writes:
|I recently started a mailing list so I wanted to bump up MAXCLEN in our
|smail 2.5 to keep the number of uux calls down.  I bumped it up
|and instead of keeping the calls down it actually started generating a
|uux for just about each member of the alias I had set up.  After much
|digging, I found out that MAXCLEN is pretty much not used in smail.
|Because of this, buffers were overflowing and variables were coming
|out weird.  Here is a diff of my changes to deliver.c:
|
|67,69c67,69
|< 	char lcommand[SMLBUF];		/* local command issued 	*/
|< 	char rcommand[SMLBUF];		/* remote command issued	*/
|< 	char scommand[SMLBUF];		/* retry  command issued	*/
|---
|> 	char lcommand[MAXCLEN];		/* local command issued 	*/
|> 	char rcommand[MAXCLEN];		/* remote command issued	*/
|> 	char scommand[MAXCLEN];		/* retry  command issued	*/

This patch looks bad to me: the code in deliver.c seems to think that it can
let the strings overflow MAXCLEN and still be OK.  I guess it gets away with
this when SMLBUF=512 is much bigger than MAXCLEN=128.  Hence, instead of
MAXCLEN, you need something like max(SMLBUF, MAXCLEN+FUDGE) or maybe just
MAXCLEN+FUDGE as the size of the strings where FUDGE is big enough to handle
any overflow (I havn't looked how big that would have to be).  The coding
style is LOUSY; you better think harder before you make a fix like this so
that you avoid overflows.

On a related subject: WHO IS COORDINATING OFFICIAL PATCHES TO SMAIL 2.5?  I
have a number of fixes that I would like to submit.


-- 
  ==========================================================================
  | Richard H. Gumpertz    rhg@CPS.COM    (913) 642-1777 or (816) 891-3561 |
  | Computer Problem Solving, 8905 Mohawk Lane, Leawood, Kansas 66206-1749 |
  ==========================================================================