Date: Tue, 29 Aug 1995 10:43:44 +0200 (MET DST) From: guido@spooky.lss.cp.philips.com (Guido van Rooij) To: pst@freefall.FreeBSD.org (Paul Traina) Cc: security@freefall.FreeBSD.org Subject: Re: please code review proposed fix for syslog problem Message-ID: <m0snMGm-000HngC@spooky.lss.cp.philips.com> In-Reply-To: <199508290749.AAA16552@freefall.FreeBSD.org> from "Paul Traina" at Aug 29, 95 00:49:50 am
next in thread | previous in thread | raw e-mail | index | archive | help
I think the general idea is well. A few chenages have been added.
Marked all with XXX
One question: what if SPACELEFT(X) is smaller then zero?
snprintf assumes size_t which is unsigned....Should this be guarded
against? (I always mix up signed unsigned operations and am
to lazy to look it up right now)
-Guido
> ***************
> *** 120,136 ****
>
> /* 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++ = ' ';
> --- 122,137 ----
>
> /* Build the message. */
> (void)time(&now);
> ! p = tbuf + snprintf(tbuf, sizeof(tbuf), "<%d>", pri);
> ! p += strftime(p, SPACELEFT(p), "%h %e %T ", localtime(&now));
> if (LogStat & LOG_PERROR)
> stdp = p;
> if (LogTag == NULL)
> LogTag = __progname;
> if (LogTag != NULL)
> ! p += snprintf(p, SPACELEFT(p), "%s", LogTag);
> if (LogStat & LOG_PID)
> ! p += snprintf(p, SPACELEFT(p), "[%d]", getpid());
> if (LogTag != NULL) {
> *p++ = ':';
> *p++ = ' ';
^^^ can overwrite outside of the buffer as well.XXX
> ***************
> *** 140,151 ****
> 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;
>
> /* Output to stderr if requested. */
> --- 141,153 ----
> for (t = fmt_cpy; ch = *fmt; ++fmt)
> if (ch == '%' && fmt[1] == 'm') {
> ++fmt;
> ! t += snprintf(t, sizeof(fmt_cpy) - (t - fmt_cpy), "%s",
> ! strerror(saved_errno));
> } else
> *t++ = ch;
^^^^^^^^^ can write outside fmt_copy as wellXXX
> *t = '\0';
^^^^^^^^^ can write outside fmt_copy as wellXXX
>
> ! p += vsnprintf(p, SPACELEFT(p), fmt_cpy, ap);
> cnt = p - tbuf;
>
> /* Output to stderr if requested. */
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?m0snMGm-000HngC>