From owner-freebsd-security  Thu Aug 31 14:49:43 1995
Return-Path: security-owner
Received: (from majordom@localhost)
          by freefall.FreeBSD.org (8.6.11/8.6.6) id OAA27055
          for security-outgoing; Thu, 31 Aug 1995 14:49:43 -0700
Received: from godzilla.zeta.org.au (godzilla.zeta.org.au [203.2.228.34])
          by freefall.FreeBSD.org (8.6.11/8.6.6) with ESMTP id OAA27029
          for <freebsd-security@freebsd.org>; Thu, 31 Aug 1995 14:49:26 -0700
Received: (from bde@localhost) by godzilla.zeta.org.au (8.6.9/8.6.9) id HAA10330; Fri, 1 Sep 1995 07:44:47 +1000
Date: Fri, 1 Sep 1995 07:44:47 +1000
From: Bruce Evans <bde@zeta.org.au>
Message-Id: <199508312144.HAA10330@godzilla.zeta.org.au>
To: bde@zeta.org.au, peter@haywire.DIALix.COM
Subject: Re: Eric Allman's syslog.c fixes
Cc: freebsd-security@freebsd.org
Sender: security-owner@freebsd.org
Precedence: bulk

>Well, this is it, BTW....  Obviously, this has portability stuff in it 
>that can come out.  Note, it's berkeley version 8.8.  4.4Lite was version 
>8.4, and Lite2 is version 8.5.  If anybody's got any complaints with this 
>version of the code, we need to hear about it ASAP, before it gets 
>published. 

Unfortunately it has many of the bugs that we noticed in the review of pst's
version.

>	if (LogTag == NULL)
>		LogTag = __progname;
>	if (LogTag != NULL) {
>		sprintf(p, "%s", LogTag);
		^^^^^^^
>		p += strlen(p);
>	}

This can overrun (or cause overruns later) if LogTag is very log.  Perhaps
this doesn't matter because users can't change LogTag.

>	/* Substitute error message for %m. */
>	for (t = fmt_cpy; ch = *fmt; ++fmt)
>		if (ch == '%' && fmt[1] == 'm') {
>			++fmt;
>			sprintf(t, "%s", strerror(saved_errno));
			^^^^^^^
>			t += strlen(t);
>		} else
>			*t++ = ch;
			^^^^
>	*t = '\0';
	^^

More overrun possibilities.  Perhaps they don't matter because users can't
change the format.

>#if USESNPRINTF
>	cnt = maxsend - (p - tbuf) + 1;
>	p += vsnprintf(p, cnt, fmt_cpy, ap);
	^^^^
>	cnt = p - tbuf;
	^^^
>#else
>....
>#endif

vsnprintf() returns the number of characters that would be written if
they fitted, so the final pointer and count are bogus if not everything
fitted.  The most interesting case is if a couple of GB would be written
and the pointer wraps around.

>	/*
>	 * Output the message to the console; don't worry about blocking,
>	 * if console blocks everything will.  Make sure the error reported
>	 * is the one from the syslogd failure.
>	 */
>	if (LogStat & LOG_CONS &&
>	    (fd = open(_PATH_CONSOLE, O_WRONLY, 0)) >= 0) {
>		(void)strcat(tbuf, "\r\n");
>		cnt += 2;
>		p = strchr(tbuf, '>') + 1;
>		(void)write(fd, p, cnt - (p - tbuf));
		^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>		(void)close(fd);
>	}

The bogus pointer and count may be considered as a feature :-).  They
may cause junk to be written to the log as evidence of attempted breakins.

Bruce