Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 1 Sep 1995 07:44:47 +1000
From:      Bruce Evans <bde@zeta.org.au>
To:        bde@zeta.org.au, peter@haywire.DIALix.COM
Cc:        freebsd-security@freebsd.org
Subject:   Re: Eric Allman's syslog.c fixes
Message-ID:  <199508312144.HAA10330@godzilla.zeta.org.au>

next in thread | raw e-mail | index | archive | help
>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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199508312144.HAA10330>