[news.admin] What's this strange thing?

leres@ace.ee.lbl.gov (Craig Leres) (04/07/90)

Ah ha! Looks like Mark Bartelt is well onto a bug that has been
annoying me for the last few months.

The first thing I noticed was that getfield() uses strncpy(), one of my
favorite sources of bugs. Note the unusual (but documented) feature:

    Strncpy copies exactly "count" characters, appending nulls if
    "from" is less than "count" characters in length; the target may
    not be null-terminated if the length of "from" is "count" or more.

So even though the code is careful to leave room for the null
terminator, it doesn't actually install one. If that last byte happens
to be non-zero, string manipulations on the from field will munge on
the path field.

The next problem is that fixfrom() can (theoretically) increase the
length of the from header (say if the closing paren is missing as Mark
suggests). So if the address and comment are too large, just toss the
comment.

Context diffs to src/header.c are appended.

		Craig
------
RCS file: RCS/header.c,v
retrieving revision 1.5
diff -c -r1.5 header.c
*** /tmp/,RCSt1a22288	Fri Apr  6 21:16:27 1990
--- header.c	Fri Apr  6 21:15:40 1990
***************
*** 345,360 ****
  fixfrom(hp)
  register struct hbuf *hp;
  {
! 	char frombuf[PATHLEN];
! 	char fullname[BUFLEN];
  
  	skin(frombuf, fullname, hp->from);	/* remove RFC822-style comments */
! 	if (fullname[0] != '\0') {
  		strcat(frombuf, " (");
  		strcat(frombuf, fullname);
  		strcat(frombuf, ")");
  	}
  	strcpy(hp->from, frombuf);	/* stick the canonicalized "from" back in */
  }
  
  skin(name, fullname, hfield)
--- 345,363 ----
  fixfrom(hp)
  register struct hbuf *hp;
  {
! 	char frombuf[sizeof(hp->from)];
! 	char fullname[sizeof(hp->from)];
  
  	skin(frombuf, fullname, hp->from);	/* remove RFC822-style comments */
! 	/* Only append comment if it fits */
! 	if (fullname[0] != '\0' && strlen(frombuf) + 2 +
! 	    strlen(fullname) + 1 <= sizeof(hp->from) - 1) {
  		strcat(frombuf, " (");
  		strcat(frombuf, fullname);
  		strcat(frombuf, ")");
  	}
  	strcpy(hp->from, frombuf);	/* stick the canonicalized "from" back in */
+ 
  }
  
  skin(name, fullname, hfield)
***************
*** 533,538 ****
--- 536,542 ----
  		;
  	if (*ptr != '\0') {
  		(void) strncpy(hpfield, ptr, size - 1);
+ 		ptr[size - 1] = '\0';
  		(void) nstrip(hpfield);
  	}
  }