- (djm) [openbsd-compat/bsd-snprintf.c] Fix integer overflow in return

value of snprintf replacement, similar to bugs in various libc
   implementations. This overflow is not exploitable in OpenSSH.
   While I'm fiddling with it, make it a fair bit faster by inlining the
   append-char routine; ok dtucker@
This commit is contained in:
Damien Miller 2007-01-14 21:20:30 +11:00
parent e67ac00b9b
commit 742cc1c194
2 changed files with 106 additions and 63 deletions

View File

@ -1,5 +1,10 @@
20070114 20070114
- (dtucker) [ssh-keygen.c] av -> argv to match earlier sync. - (dtucker) [ssh-keygen.c] av -> argv to match earlier sync.
- (djm) [openbsd-compat/bsd-snprintf.c] Fix integer overflow in return
value of snprintf replacement, similar to bugs in various libc
implementations. This overflow is not exploitable in OpenSSH.
While I'm fiddling with it, make it a fair bit faster by inlining the
append-char routine; ok dtucker@
20070105 20070105
- (djm) OpenBSD CVS Sync - (djm) OpenBSD CVS Sync
@ -2664,4 +2669,4 @@
OpenServer 6 and add osr5bigcrypt support so when someone migrates OpenServer 6 and add osr5bigcrypt support so when someone migrates
passwords between UnixWare and OpenServer they will still work. OK dtucker@ passwords between UnixWare and OpenServer they will still work. OK dtucker@
$Id: ChangeLog,v 1.4604 2007/01/13 23:26:25 dtucker Exp $ $Id: ChangeLog,v 1.4605 2007/01/14 10:20:30 djm Exp $

View File

@ -85,6 +85,11 @@
* *
* Move #endif to make sure VA_COPY, LDOUBLE, etc are defined even * Move #endif to make sure VA_COPY, LDOUBLE, etc are defined even
* if the C library has some snprintf functions already. * if the C library has some snprintf functions already.
*
* Damien Miller (djm@mindrot.org) Jan 2007
* Fix integer overflows in return value.
* Make formatting quite a bit faster by inlining dopr_outch()
*
**************************************************************/ **************************************************************/
#include "includes.h" #include "includes.h"
@ -112,6 +117,8 @@
#include <stdarg.h> #include <stdarg.h>
#include <stdlib.h> #include <stdlib.h>
#include <string.h> #include <string.h>
#include <limits.h>
#include <errno.h>
#ifdef HAVE_LONG_DOUBLE #ifdef HAVE_LONG_DOUBLE
# define LDOUBLE long double # define LDOUBLE long double
@ -159,17 +166,27 @@
# define MAX(p,q) (((p) >= (q)) ? (p) : (q)) # define MAX(p,q) (((p) >= (q)) ? (p) : (q))
#endif #endif
static size_t dopr(char *buffer, size_t maxlen, const char *format, #define DOPR_OUTCH(buf, pos, buflen, thechar) \
va_list args_in); do { \
static void fmtstr(char *buffer, size_t *currlen, size_t maxlen, if (++pos >= INT_MAX) { \
char *value, int flags, int min, int max); errno = ERANGE; \
static void fmtint(char *buffer, size_t *currlen, size_t maxlen, return -1; \
LLONG value, int base, int min, int max, int flags); if (pos < buflen) \
static void fmtfp(char *buffer, size_t *currlen, size_t maxlen, buf[pos] = thechar; \
LDOUBLE fvalue, int min, int max, int flags); } \
static void dopr_outch(char *buffer, size_t *currlen, size_t maxlen, char c); } while (0)
static size_t dopr(char *buffer, size_t maxlen, const char *format, va_list args_in) static int dopr(char *buffer, size_t maxlen, const char *format,
va_list args_in);
static int fmtstr(char *buffer, size_t *currlen, size_t maxlen,
char *value, int flags, int min, int max);
static int fmtint(char *buffer, size_t *currlen, size_t maxlen,
LLONG value, int base, int min, int max, int flags);
static int fmtfp(char *buffer, size_t *currlen, size_t maxlen,
LDOUBLE fvalue, int min, int max, int flags);
static int
dopr(char *buffer, size_t maxlen, const char *format, va_list args_in)
{ {
char ch; char ch;
LLONG value; LLONG value;
@ -198,8 +215,8 @@ static size_t dopr(char *buffer, size_t maxlen, const char *format, va_list args
case DP_S_DEFAULT: case DP_S_DEFAULT:
if (ch == '%') if (ch == '%')
state = DP_S_FLAGS; state = DP_S_FLAGS;
else else
dopr_outch (buffer, &currlen, maxlen, ch); DOPR_OUTCH(buffer, currlen, maxlen, ch);
ch = *format++; ch = *format++;
break; break;
case DP_S_FLAGS: case DP_S_FLAGS:
@ -298,7 +315,9 @@ static size_t dopr(char *buffer, size_t maxlen, const char *format, va_list args
value = va_arg (args, LLONG); value = va_arg (args, LLONG);
else else
value = va_arg (args, int); value = va_arg (args, int);
fmtint (buffer, &currlen, maxlen, value, 10, min, max, flags); if (fmtint(buffer, &currlen, maxlen,
value, 10, min, max, flags) == -1)
return -1;
break; break;
case 'o': case 'o':
flags |= DP_F_UNSIGNED; flags |= DP_F_UNSIGNED;
@ -310,7 +329,9 @@ static size_t dopr(char *buffer, size_t maxlen, const char *format, va_list args
value = (long)va_arg (args, unsigned LLONG); value = (long)va_arg (args, unsigned LLONG);
else else
value = (long)va_arg (args, unsigned int); value = (long)va_arg (args, unsigned int);
fmtint (buffer, &currlen, maxlen, value, 8, min, max, flags); if (fmtint(buffer, &currlen, maxlen, value,
8, min, max, flags) == -1)
return -1;
break; break;
case 'u': case 'u':
flags |= DP_F_UNSIGNED; flags |= DP_F_UNSIGNED;
@ -322,7 +343,9 @@ static size_t dopr(char *buffer, size_t maxlen, const char *format, va_list args
value = (LLONG)va_arg (args, unsigned LLONG); value = (LLONG)va_arg (args, unsigned LLONG);
else else
value = (long)va_arg (args, unsigned int); value = (long)va_arg (args, unsigned int);
fmtint (buffer, &currlen, maxlen, value, 10, min, max, flags); if (fmtint(buffer, &currlen, maxlen, value,
10, min, max, flags) == -1)
return -1;
break; break;
case 'X': case 'X':
flags |= DP_F_UP; flags |= DP_F_UP;
@ -336,15 +359,18 @@ static size_t dopr(char *buffer, size_t maxlen, const char *format, va_list args
value = (LLONG)va_arg (args, unsigned LLONG); value = (LLONG)va_arg (args, unsigned LLONG);
else else
value = (long)va_arg (args, unsigned int); value = (long)va_arg (args, unsigned int);
fmtint (buffer, &currlen, maxlen, value, 16, min, max, flags); if (fmtint(buffer, &currlen, maxlen, value,
16, min, max, flags) == -1)
return -1;
break; break;
case 'f': case 'f':
if (cflags == DP_C_LDOUBLE) if (cflags == DP_C_LDOUBLE)
fvalue = va_arg (args, LDOUBLE); fvalue = va_arg (args, LDOUBLE);
else else
fvalue = va_arg (args, double); fvalue = va_arg (args, double);
/* um, floating point? */ if (fmtfp(buffer, &currlen, maxlen, fvalue,
fmtfp (buffer, &currlen, maxlen, fvalue, min, max, flags); min, max, flags) == -1)
return -1;
break; break;
case 'E': case 'E':
flags |= DP_F_UP; flags |= DP_F_UP;
@ -353,7 +379,9 @@ static size_t dopr(char *buffer, size_t maxlen, const char *format, va_list args
fvalue = va_arg (args, LDOUBLE); fvalue = va_arg (args, LDOUBLE);
else else
fvalue = va_arg (args, double); fvalue = va_arg (args, double);
fmtfp (buffer, &currlen, maxlen, fvalue, min, max, flags); if (fmtfp(buffer, &currlen, maxlen, fvalue,
min, max, flags) == -1)
return -1;
break; break;
case 'G': case 'G':
flags |= DP_F_UP; flags |= DP_F_UP;
@ -362,10 +390,13 @@ static size_t dopr(char *buffer, size_t maxlen, const char *format, va_list args
fvalue = va_arg (args, LDOUBLE); fvalue = va_arg (args, LDOUBLE);
else else
fvalue = va_arg (args, double); fvalue = va_arg (args, double);
fmtfp (buffer, &currlen, maxlen, fvalue, min, max, flags); if (fmtfp(buffer, &currlen, maxlen, fvalue,
min, max, flags) == -1)
return -1;
break; break;
case 'c': case 'c':
dopr_outch (buffer, &currlen, maxlen, va_arg (args, int)); DOPR_OUTCH(buffer, currlen, maxlen,
va_arg (args, int));
break; break;
case 's': case 's':
strvalue = va_arg (args, char *); strvalue = va_arg (args, char *);
@ -374,11 +405,15 @@ static size_t dopr(char *buffer, size_t maxlen, const char *format, va_list args
max = strlen(strvalue); max = strlen(strvalue);
} }
if (min > 0 && max >= 0 && min > max) max = min; if (min > 0 && max >= 0 && min > max) max = min;
fmtstr (buffer, &currlen, maxlen, strvalue, flags, min, max); if (fmtstr(buffer, &currlen, maxlen,
strvalue, flags, min, max) == -1)
return -1;
break; break;
case 'p': case 'p':
strvalue = va_arg (args, void *); strvalue = va_arg (args, void *);
fmtint (buffer, &currlen, maxlen, (long) strvalue, 16, min, max, flags); if (fmtint(buffer, &currlen, maxlen,
(long) strvalue, 16, min, max, flags) == -1)
return -1;
break; break;
case 'n': case 'n':
if (cflags == DP_C_SHORT) { if (cflags == DP_C_SHORT) {
@ -400,7 +435,7 @@ static size_t dopr(char *buffer, size_t maxlen, const char *format, va_list args
} }
break; break;
case '%': case '%':
dopr_outch (buffer, &currlen, maxlen, ch); DOPR_OUTCH(buffer, currlen, maxlen, ch);
break; break;
case 'w': case 'w':
/* not supported yet, treat as next char */ /* not supported yet, treat as next char */
@ -429,11 +464,12 @@ static size_t dopr(char *buffer, size_t maxlen, const char *format, va_list args
buffer[maxlen - 1] = '\0'; buffer[maxlen - 1] = '\0';
} }
return currlen; return currlen < INT_MAX ? (int)currlen : -1;
} }
static void fmtstr(char *buffer, size_t *currlen, size_t maxlen, static int
char *value, int flags, int min, int max) fmtstr(char *buffer, size_t *currlen, size_t maxlen,
char *value, int flags, int min, int max)
{ {
int padlen, strln; /* amount to pad */ int padlen, strln; /* amount to pad */
int cnt = 0; int cnt = 0;
@ -453,24 +489,26 @@ static void fmtstr(char *buffer, size_t *currlen, size_t maxlen,
padlen = -padlen; /* Left Justify */ padlen = -padlen; /* Left Justify */
while ((padlen > 0) && (cnt < max)) { while ((padlen > 0) && (cnt < max)) {
dopr_outch (buffer, currlen, maxlen, ' '); DOPR_OUTCH(buffer, *currlen, maxlen, ' ');
--padlen; --padlen;
++cnt; ++cnt;
} }
while (*value && (cnt < max)) { while (*value && (cnt < max)) {
dopr_outch (buffer, currlen, maxlen, *value++); DOPR_OUTCH(buffer, *currlen, maxlen, *value++);
++cnt; ++cnt;
} }
while ((padlen < 0) && (cnt < max)) { while ((padlen < 0) && (cnt < max)) {
dopr_outch (buffer, currlen, maxlen, ' '); DOPR_OUTCH(buffer, *currlen, maxlen, ' ');
++padlen; ++padlen;
++cnt; ++cnt;
} }
return 0;
} }
/* Have to handle DP_F_NUM (ie 0x and 0 alternates) */ /* Have to handle DP_F_NUM (ie 0x and 0 alternates) */
static void fmtint(char *buffer, size_t *currlen, size_t maxlen, static int
fmtint(char *buffer, size_t *currlen, size_t maxlen,
LLONG value, int base, int min, int max, int flags) LLONG value, int base, int min, int max, int flags)
{ {
int signvalue = 0; int signvalue = 0;
@ -527,31 +565,32 @@ static void fmtint(char *buffer, size_t *currlen, size_t maxlen,
/* Spaces */ /* Spaces */
while (spadlen > 0) { while (spadlen > 0) {
dopr_outch (buffer, currlen, maxlen, ' '); DOPR_OUTCH(buffer, *currlen, maxlen, ' ');
--spadlen; --spadlen;
} }
/* Sign */ /* Sign */
if (signvalue) if (signvalue)
dopr_outch (buffer, currlen, maxlen, signvalue); DOPR_OUTCH(buffer, *currlen, maxlen, signvalue);
/* Zeros */ /* Zeros */
if (zpadlen > 0) { if (zpadlen > 0) {
while (zpadlen > 0) { while (zpadlen > 0) {
dopr_outch (buffer, currlen, maxlen, '0'); DOPR_OUTCH(buffer, *currlen, maxlen, '0');
--zpadlen; --zpadlen;
} }
} }
/* Digits */ /* Digits */
while (place > 0) while (place > 0)
dopr_outch (buffer, currlen, maxlen, convert[--place]); DOPR_OUTCH(buffer, *currlen, maxlen, convert[--place]);
/* Left Justified spaces */ /* Left Justified spaces */
while (spadlen < 0) { while (spadlen < 0) {
dopr_outch (buffer, currlen, maxlen, ' '); DOPR_OUTCH(buffer, *currlen, maxlen, ' ');
++spadlen; ++spadlen;
} }
return 0;
} }
static LDOUBLE abs_val(LDOUBLE value) static LDOUBLE abs_val(LDOUBLE value)
@ -564,13 +603,13 @@ static LDOUBLE abs_val(LDOUBLE value)
return result; return result;
} }
static LDOUBLE POW10(int exp) static LDOUBLE POW10(int val)
{ {
LDOUBLE result = 1; LDOUBLE result = 1;
while (exp) { while (val) {
result *= 10; result *= 10;
exp--; val--;
} }
return result; return result;
@ -604,7 +643,10 @@ static double my_modf(double x0, double *iptr)
} }
if (i == 100) { if (i == 100) {
/* yikes! the number is beyond what we can handle. What do we do? */ /*
* yikes! the number is beyond what we can handle.
* What do we do?
*/
(*iptr) = 0; (*iptr) = 0;
return 0; return 0;
} }
@ -623,8 +665,9 @@ static double my_modf(double x0, double *iptr)
} }
static void fmtfp (char *buffer, size_t *currlen, size_t maxlen, static int
LDOUBLE fvalue, int min, int max, int flags) fmtfp (char *buffer, size_t *currlen, size_t maxlen,
LDOUBLE fvalue, int min, int max, int flags)
{ {
int signvalue = 0; int signvalue = 0;
double ufvalue; double ufvalue;
@ -729,24 +772,24 @@ static void fmtfp (char *buffer, size_t *currlen, size_t maxlen,
if ((flags & DP_F_ZERO) && (padlen > 0)) { if ((flags & DP_F_ZERO) && (padlen > 0)) {
if (signvalue) { if (signvalue) {
dopr_outch (buffer, currlen, maxlen, signvalue); DOPR_OUTCH(buffer, *currlen, maxlen, signvalue);
--padlen; --padlen;
signvalue = 0; signvalue = 0;
} }
while (padlen > 0) { while (padlen > 0) {
dopr_outch (buffer, currlen, maxlen, '0'); DOPR_OUTCH(buffer, *currlen, maxlen, '0');
--padlen; --padlen;
} }
} }
while (padlen > 0) { while (padlen > 0) {
dopr_outch (buffer, currlen, maxlen, ' '); DOPR_OUTCH(buffer, *currlen, maxlen, ' ');
--padlen; --padlen;
} }
if (signvalue) if (signvalue)
dopr_outch (buffer, currlen, maxlen, signvalue); DOPR_OUTCH(buffer, *currlen, maxlen, signvalue);
while (iplace > 0) while (iplace > 0)
dopr_outch (buffer, currlen, maxlen, iconvert[--iplace]); DOPR_OUTCH(buffer, *currlen, maxlen, iconvert[--iplace]);
#ifdef DEBUG_SNPRINTF #ifdef DEBUG_SNPRINTF
printf("fmtfp: fplace=%d zpadlen=%d\n", fplace, zpadlen); printf("fmtfp: fplace=%d zpadlen=%d\n", fplace, zpadlen);
@ -757,41 +800,37 @@ static void fmtfp (char *buffer, size_t *currlen, size_t maxlen,
* char to print out. * char to print out.
*/ */
if (max > 0) { if (max > 0) {
dopr_outch (buffer, currlen, maxlen, '.'); DOPR_OUTCH(buffer, *currlen, maxlen, '.');
while (zpadlen > 0) { while (zpadlen > 0) {
dopr_outch (buffer, currlen, maxlen, '0'); DOPR_OUTCH(buffer, *currlen, maxlen, '0');
--zpadlen; --zpadlen;
} }
while (fplace > 0) while (fplace > 0)
dopr_outch (buffer, currlen, maxlen, fconvert[--fplace]); DOPR_OUTCH(buffer, *currlen, maxlen,
fconvert[--fplace]);
} }
while (padlen < 0) { while (padlen < 0) {
dopr_outch (buffer, currlen, maxlen, ' '); DOPR_OUTCH(buffer, *currlen, maxlen, ' ');
++padlen; ++padlen;
} }
} return 0;
static void dopr_outch(char *buffer, size_t *currlen, size_t maxlen, char c)
{
if (*currlen < maxlen) {
buffer[(*currlen)] = c;
}
(*currlen)++;
} }
#endif /* !defined(HAVE_SNPRINTF) || !defined(HAVE_VSNPRINTF) */ #endif /* !defined(HAVE_SNPRINTF) || !defined(HAVE_VSNPRINTF) */
#if !defined(HAVE_VSNPRINTF) #if !defined(HAVE_VSNPRINTF)
int vsnprintf (char *str, size_t count, const char *fmt, va_list args) static int
vsnprintf (char *str, size_t count, const char *fmt, va_list args)
{ {
return dopr(str, count, fmt, args); return dopr(str, count, fmt, args);
} }
#endif #endif
#if !defined(HAVE_SNPRINTF) #if !defined(HAVE_SNPRINTF)
int snprintf(char *str, size_t count, SNPRINTF_CONST char *fmt, ...) static int
snprintf(char *str, size_t count, const char *fmt, ...)
{ {
size_t ret; size_t ret;
va_list ap; va_list ap;
@ -802,4 +841,3 @@ int snprintf(char *str, size_t count, SNPRINTF_CONST char *fmt, ...)
return ret; return ret;
} }
#endif #endif