Subject: Re: lib/35401
To: None <lib-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: Christian Biere <christianbiere@gmx.de>
List: netbsd-bugs
Date: 01/11/2007 11:45:02
The following reply was made to PR lib/35401; it has been noted by GNATS.
From: Christian Biere <christianbiere@gmx.de>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: lib/35401
Date: Thu, 11 Jan 2007 12:48:21 +0100
--45Z9DzgjV8m4Oswq
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Consider the attached patch as a suggestion. I have not tested, not even compiled it yet.
I think the other patch could cause a memory leak due to not calling __freedtoa() on
overflow. My patch also catches another issue which was mentioned in fefe's blog:
width = -width;
This is seeminly meant to ensure that width is positive but this won't help if width
is INT_MIN and -INT_MAX != INT_MIN. I suspect the latter is always the case but I
added a check anyways.
Maybe there should be another overflow check for the %hn case. I don't know why there's
a XXX after the ssize_t case in the code.
--
Christian
--45Z9DzgjV8m4Oswq
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="vfprintf.c.udif"
--- vfprintf.c.bak 2007-01-11 11:22:13.000000000 +0100
+++ vfprintf.c 2007-01-11 12:40:19.000000000 +0100
@@ -202,7 +202,7 @@ __vfprintf_unlocked(fp, fmt0, ap)
{
const char *fmt;/* format string */
int ch; /* character from fmt */
- int n, m; /* handy integers (short term usage) */
+ int n; /* handy integer (short term usage) */
const char *cp; /* handy char pointer (short term usage) */
char *bp; /* handy char pointer (short term usage) */
struct __siov *iovp;/* for PRINT macro */
@@ -350,7 +350,10 @@ __vfprintf_unlocked(fp, fmt0, ap)
}
}
}
- if ((m = fmt - cp) != 0) {
+ if (fmt != cp) {
+ ptrdiff_t m = fmt - cp;
+ if (m < 0 || m > INT_MAX - ret)
+ goto overflow;
PRINT(cp, m);
ret += m;
}
@@ -387,6 +390,8 @@ reswitch: switch (ch) {
*/
if ((width = va_arg(ap, int)) >= 0)
goto rflag;
+ if (-INT_MAX != INT_MIN && width <= INT_MIN)
+ goto overflow;
width = -width;
/* FALLTHROUGH */
case '-':
@@ -402,11 +407,19 @@ reswitch: switch (ch) {
goto rflag;
}
n = 0;
+ while ('0' == ch) {
+ ch = *fmt++;
+ }
while (is_digit(ch)) {
- n = 10 * n + to_digit(ch);
+ if (n > INT_MAX / 10)
+ goto overflow;
+ n *= 10;
+ if (to_digit(ch) > INT_MAX - n)
+ goto overflow;
+ n += to_digit(ch);
ch = *fmt++;
}
- prec = n < 0 ? -1 : n;
+ prec = n;
goto reswitch;
case '0':
/*
@@ -420,7 +433,12 @@ reswitch: switch (ch) {
case '5': case '6': case '7': case '8': case '9':
n = 0;
do {
- n = 10 * n + to_digit(ch);
+ if (n > INT_MAX / 10)
+ goto overflow;
+ n *= 10;
+ if (to_digit(ch) > INT_MAX - n)
+ goto overflow;
+ n += to_digit(ch);
ch = *fmt++;
} while (is_digit(ch));
width = n;
@@ -797,19 +815,28 @@ number: if ((dprec = prec) >= 0)
PAD(width - realsz, blanks);
/* finally, adjust ret */
- ret += width > realsz ? width : realsz;
+ if (MAX(width, realsz) > INT_MAX - ret)
+ goto overflow;
+ ret += MAX(width, realsz);
FLUSH(); /* copy out the I/O vectors */
}
done:
FLUSH();
error:
+ if (__sferror(fp))
+ ret = -1;
+ goto finish;
+
+overflow:
+ errno = EOVERFLOW;
+ ret = -1;
+
+finish:
#ifndef NO_FLOATING_POINT
if (dtoaresult)
__freedtoa(dtoaresult);
#endif
- if (__sferror(fp))
- ret = -1;
return (ret);
}
--45Z9DzgjV8m4Oswq--