Date: Thu, 31 Aug 1995 15:01:20 -0700 From: Paul Traina <pst@shockwave.com> To: Bruce Evans <bde@zeta.org.au> Cc: peter@haywire.DIALix.COM, freebsd-security@freebsd.org Subject: Re: Eric Allman's syslog.c fixes Message-ID: <199508312201.PAA03378@precipice.shockwave.com> In-Reply-To: Your message of "Fri, 01 Sep 1995 07:44:47 %2B1000." <199508312144.HAA10330@godzilla.zeta.org.au>
next in thread | previous in thread | raw e-mail | index | archive | help
From: Bruce Evans <bde@zeta.org.au>
Subject: Re: Eric Allman's syslog.c fixes
>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.
I'd rather keep the check here.
> /* 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.
That was my basic feeling.
>#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.
Yep, bummer.
> /*
> * 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?199508312201.PAA03378>