Subject: Re: Patch for 8lgm syslog/sendmail vulnerability, 4.4lite machines
To: None <perry@piermont.com>
From: Charles Hannum <Charles-Hannum@deshaw.com>
List: current-users
Date: 08/29/1995 13:55:38
[Sigh.  I don't really have time to deal with this right now, but I
suppose it can't really wait.]

Your original patch has a couple of problems; mainly, `p' can still
overrun the end of the buffer, and thus more bytes are output than
should be.  In addition, the old special-case handling of `%m' is
annoying; it really should be done in vsnprintf().

Here's a replacement set of patches.  Please note that I have *not*
tested this.  (I'm also not happy with the static variable in
vfprintf(), but I'm not sure whether that's better or worse than
moving it outside the block and adding NL_TEXTMAX bytes of automatic
variables to vfprintf().)

I'll leave this to J.T. now...

(J.T.: Why does vfprintf() *always* call localeconv() to fetch the
decimal point string?  That seems inefficient at best.)


Index: gen/syslog.c
===================================================================
RCS file: /a/cvsroot/src/lib/libc/gen/syslog.c,v
retrieving revision 1.8
diff -c -2 -r1.8 syslog.c
*** syslog.c	1995/04/11 02:57:52	1.8
--- syslog.c	1995/08/29 17:46:59
***************
*** 70,73 ****
--- 70,75 ----
  extern char	*__progname;		/* Program name, from crt0. */
  
+ #define	TBUF_LEN	2048
+ 
  /*
   * syslog, vsyslog --
***************
*** 105,109 ****
  	time_t now;
  	int fd, saved_errno;
! 	char *stdp, tbuf[2048], fmt_cpy[1024];
  
  #define	INTERNALLOG	LOG_ERR|LOG_CONS|LOG_PERROR|LOG_PID
--- 107,114 ----
  	time_t now;
  	int fd, saved_errno;
! 	size_t tbuf_left, prlen;
! 	char *stdp, tbuf[TBUF_LEN];
! 
! 	tbuf_left = TBUF_LEN;
  
  #define	INTERNALLOG	LOG_ERR|LOG_CONS|LOG_PERROR|LOG_PID
***************
*** 127,156 ****
  	/* Build the message. */
  	(void)time(&now);
! 	p = tbuf + sprintf(tbuf, "<%d>", pri);
! 	p += strftime(p, sizeof (tbuf) - (p - tbuf), "%h %e %T ",
! 	    localtime(&now));
! 	if (LogStat & LOG_PERROR)
  		stdp = p;
  	if (LogTag == NULL)
  		LogTag = __progname;
- 	if (LogTag != NULL)
- 		p += sprintf(p, "%s", LogTag);
- 	if (LogStat & LOG_PID)
- 		p += sprintf(p, "[%d]", getpid());
  	if (LogTag != NULL) {
  		*p++ = ':';
! 		*p++ = ' ';
  	}
  
! 	/* Substitute error message for %m. */
! 	for (t = fmt_cpy; ch = *fmt; ++fmt)
! 		if (ch == '%' && fmt[1] == 'm') {
! 			++fmt;
! 			t += sprintf(t, "%s", strerror(saved_errno));
! 		} else
! 			*t++ = ch;
! 	*t = '\0';
  
! 	p += vsprintf(p, fmt_cpy, ap);
  	cnt = p - tbuf;
  
--- 132,187 ----
  	/* Build the message. */
  	(void)time(&now);
! 
! #define	SYNC() \
! 	p = tbuf + TBUF_LEN - tbuf_left
! 
! #define	DEC() \
! 	if (tbuf_left > prlen)		\
! 		tbuf_left -= prlen;	\
! 	else				\
! 		tbuf_left = 0;
! 
! 	SYNC();
! 	prlen = snprintf(p, tbuf_left, "<%d>", pri);
! 	DEC();
! 
! 	SYNC();
! 	prlen = strftime(p, tbuf_left, "%h %e %T ", localtime(&now));
! 	DEC();
! 
! 	if (LogStat & LOG_PERROR) {
! 		SYNC();
  		stdp = p;
+ 	}
+ 
  	if (LogTag == NULL)
  		LogTag = __progname;
  	if (LogTag != NULL) {
+ 		SYNC();
+ 		prlen = snprintf(p, tbuf_left, "%s", LogTag);
+ 		DEC();
+ 	}
+ 
+ 	if (LogStat & LOG_PID) {
+ 		SYNC();
+ 		prlen = snprintf(p, tbuf_left, "[%d]", getpid());
+ 		DEC();
+ 	}
+ 
+ 	if (LogTag != NULL && tbuf_left > 1) {
+ 		SYNC();
  		*p++ = ':';
! 		tbuf_left--;
! 		if (tbuf_left > 1) {
! 			*p++ = ' ';
! 			tbuf_left--;
! 		}
  	}
  
! 	SYNC();
! 	prlen = vsnprintf(p, tbuf_left, fmt, ap);
! 	DEC();
  
! 	SYNC();
  	cnt = p - tbuf;
  
Index: stdio/vfprintf.c
===================================================================
RCS file: /a/cvsroot/src/lib/libc/stdio/vfprintf.c,v
retrieving revision 1.17
diff -c -2 -r1.17 vfprintf.c
*** vfprintf.c	1995/05/02 19:52:41	1.17
--- vfprintf.c	1995/08/29 17:46:59
***************
*** 48,51 ****
--- 48,53 ----
  #include <sys/types.h>
  
+ #include <errno.h>
+ #include <limits.h>
  #include <stdio.h>
  #include <stdlib.h>
***************
*** 61,64 ****
--- 63,68 ----
  #include "fvwrite.h"
  
+ extern char *__strerror __P((int , char *));
+ 
  /*
   * Flush out all the vectors defined by the given uio,
***************
*** 172,175 ****
--- 176,180 ----
  	char sign;		/* sign prefix (' ', '+', '-', or \0) */
  	wchar_t wc;
+ 	int saved_errno = errno;/* saved errno value for `%m' */
  #ifdef FLOATING_POINT
  	char *decimal_point = localeconv()->decimal_point;
***************
*** 459,462 ****
--- 464,472 ----
  			break;
  #endif /* FLOATING_POINT */
+ 		case 'm': {
+ 			static char buf[NL_TEXTMAX];
+ 			cp = __strerror(saved_errno, buf);
+ 			goto string;
+ 		}
  		case 'n':
  			if (flags & QUADINT)
***************
*** 494,497 ****
--- 504,508 ----
  			if ((cp = va_arg(ap, char *)) == NULL)
  				cp = "(null)";
+ 		string:
  			if (prec >= 0) {
  				/*