- (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@
diff --git a/openbsd-compat/bsd-snprintf.c b/openbsd-compat/bsd-snprintf.c
index 04651e1..cefb1d1 100644
--- a/openbsd-compat/bsd-snprintf.c
+++ b/openbsd-compat/bsd-snprintf.c
@@ -85,6 +85,11 @@
*
* Move #endif to make sure VA_COPY, LDOUBLE, etc are defined even
* 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"
@@ -112,6 +117,8 @@
#include <stdarg.h>
#include <stdlib.h>
#include <string.h>
+#include <limits.h>
+#include <errno.h>
#ifdef HAVE_LONG_DOUBLE
# define LDOUBLE long double
@@ -159,17 +166,27 @@
# define MAX(p,q) (((p) >= (q)) ? (p) : (q))
#endif
-static size_t dopr(char *buffer, size_t maxlen, const char *format,
- va_list args_in);
-static void fmtstr(char *buffer, size_t *currlen, size_t maxlen,
- char *value, int flags, int min, int max);
-static void fmtint(char *buffer, size_t *currlen, size_t maxlen,
- LLONG value, int base, int min, int max, int flags);
-static void fmtfp(char *buffer, size_t *currlen, size_t maxlen,
- LDOUBLE fvalue, int min, int max, int flags);
-static void dopr_outch(char *buffer, size_t *currlen, size_t maxlen, char c);
+#define DOPR_OUTCH(buf, pos, buflen, thechar) \
+ do { \
+ if (++pos >= INT_MAX) { \
+ errno = ERANGE; \
+ return -1; \
+ if (pos < buflen) \
+ buf[pos] = thechar; \
+ } \
+ } 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;
LLONG value;
@@ -198,8 +215,8 @@
case DP_S_DEFAULT:
if (ch == '%')
state = DP_S_FLAGS;
- else
- dopr_outch (buffer, &currlen, maxlen, ch);
+ else
+ DOPR_OUTCH(buffer, currlen, maxlen, ch);
ch = *format++;
break;
case DP_S_FLAGS:
@@ -298,7 +315,9 @@
value = va_arg (args, LLONG);
else
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;
case 'o':
flags |= DP_F_UNSIGNED;
@@ -310,7 +329,9 @@
value = (long)va_arg (args, unsigned LLONG);
else
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;
case 'u':
flags |= DP_F_UNSIGNED;
@@ -322,7 +343,9 @@
value = (LLONG)va_arg (args, unsigned LLONG);
else
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;
case 'X':
flags |= DP_F_UP;
@@ -336,15 +359,18 @@
value = (LLONG)va_arg (args, unsigned LLONG);
else
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;
case 'f':
if (cflags == DP_C_LDOUBLE)
fvalue = va_arg (args, LDOUBLE);
else
fvalue = va_arg (args, double);
- /* um, floating point? */
- fmtfp (buffer, &currlen, maxlen, fvalue, min, max, flags);
+ if (fmtfp(buffer, &currlen, maxlen, fvalue,
+ min, max, flags) == -1)
+ return -1;
break;
case 'E':
flags |= DP_F_UP;
@@ -353,7 +379,9 @@
fvalue = va_arg (args, LDOUBLE);
else
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;
case 'G':
flags |= DP_F_UP;
@@ -362,10 +390,13 @@
fvalue = va_arg (args, LDOUBLE);
else
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;
case 'c':
- dopr_outch (buffer, &currlen, maxlen, va_arg (args, int));
+ DOPR_OUTCH(buffer, currlen, maxlen,
+ va_arg (args, int));
break;
case 's':
strvalue = va_arg (args, char *);
@@ -374,11 +405,15 @@
max = strlen(strvalue);
}
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;
case 'p':
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;
case 'n':
if (cflags == DP_C_SHORT) {
@@ -400,7 +435,7 @@
}
break;
case '%':
- dopr_outch (buffer, &currlen, maxlen, ch);
+ DOPR_OUTCH(buffer, currlen, maxlen, ch);
break;
case 'w':
/* not supported yet, treat as next char */
@@ -429,11 +464,12 @@
buffer[maxlen - 1] = '\0';
}
- return currlen;
+ return currlen < INT_MAX ? (int)currlen : -1;
}
-static void fmtstr(char *buffer, size_t *currlen, size_t maxlen,
- char *value, int flags, int min, int max)
+static int
+fmtstr(char *buffer, size_t *currlen, size_t maxlen,
+ char *value, int flags, int min, int max)
{
int padlen, strln; /* amount to pad */
int cnt = 0;
@@ -453,24 +489,26 @@
padlen = -padlen; /* Left Justify */
while ((padlen > 0) && (cnt < max)) {
- dopr_outch (buffer, currlen, maxlen, ' ');
+ DOPR_OUTCH(buffer, *currlen, maxlen, ' ');
--padlen;
++cnt;
}
while (*value && (cnt < max)) {
- dopr_outch (buffer, currlen, maxlen, *value++);
+ DOPR_OUTCH(buffer, *currlen, maxlen, *value++);
++cnt;
}
while ((padlen < 0) && (cnt < max)) {
- dopr_outch (buffer, currlen, maxlen, ' ');
+ DOPR_OUTCH(buffer, *currlen, maxlen, ' ');
++padlen;
++cnt;
}
+ return 0;
}
/* 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)
{
int signvalue = 0;
@@ -527,31 +565,32 @@
/* Spaces */
while (spadlen > 0) {
- dopr_outch (buffer, currlen, maxlen, ' ');
+ DOPR_OUTCH(buffer, *currlen, maxlen, ' ');
--spadlen;
}
/* Sign */
if (signvalue)
- dopr_outch (buffer, currlen, maxlen, signvalue);
+ DOPR_OUTCH(buffer, *currlen, maxlen, signvalue);
/* Zeros */
if (zpadlen > 0) {
while (zpadlen > 0) {
- dopr_outch (buffer, currlen, maxlen, '0');
+ DOPR_OUTCH(buffer, *currlen, maxlen, '0');
--zpadlen;
}
}
/* Digits */
while (place > 0)
- dopr_outch (buffer, currlen, maxlen, convert[--place]);
+ DOPR_OUTCH(buffer, *currlen, maxlen, convert[--place]);
/* Left Justified spaces */
while (spadlen < 0) {
- dopr_outch (buffer, currlen, maxlen, ' ');
+ DOPR_OUTCH(buffer, *currlen, maxlen, ' ');
++spadlen;
}
+ return 0;
}
static LDOUBLE abs_val(LDOUBLE value)
@@ -564,13 +603,13 @@
return result;
}
-static LDOUBLE POW10(int exp)
+static LDOUBLE POW10(int val)
{
LDOUBLE result = 1;
- while (exp) {
+ while (val) {
result *= 10;
- exp--;
+ val--;
}
return result;
@@ -604,7 +643,10 @@
}
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;
return 0;
}
@@ -623,8 +665,9 @@
}
-static void fmtfp (char *buffer, size_t *currlen, size_t maxlen,
- LDOUBLE fvalue, 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)
{
int signvalue = 0;
double ufvalue;
@@ -729,24 +772,24 @@
if ((flags & DP_F_ZERO) && (padlen > 0)) {
if (signvalue) {
- dopr_outch (buffer, currlen, maxlen, signvalue);
+ DOPR_OUTCH(buffer, *currlen, maxlen, signvalue);
--padlen;
signvalue = 0;
}
while (padlen > 0) {
- dopr_outch (buffer, currlen, maxlen, '0');
+ DOPR_OUTCH(buffer, *currlen, maxlen, '0');
--padlen;
}
}
while (padlen > 0) {
- dopr_outch (buffer, currlen, maxlen, ' ');
+ DOPR_OUTCH(buffer, *currlen, maxlen, ' ');
--padlen;
}
if (signvalue)
- dopr_outch (buffer, currlen, maxlen, signvalue);
+ DOPR_OUTCH(buffer, *currlen, maxlen, signvalue);
while (iplace > 0)
- dopr_outch (buffer, currlen, maxlen, iconvert[--iplace]);
+ DOPR_OUTCH(buffer, *currlen, maxlen, iconvert[--iplace]);
#ifdef DEBUG_SNPRINTF
printf("fmtfp: fplace=%d zpadlen=%d\n", fplace, zpadlen);
@@ -757,41 +800,37 @@
* char to print out.
*/
if (max > 0) {
- dopr_outch (buffer, currlen, maxlen, '.');
+ DOPR_OUTCH(buffer, *currlen, maxlen, '.');
while (zpadlen > 0) {
- dopr_outch (buffer, currlen, maxlen, '0');
+ DOPR_OUTCH(buffer, *currlen, maxlen, '0');
--zpadlen;
}
while (fplace > 0)
- dopr_outch (buffer, currlen, maxlen, fconvert[--fplace]);
+ DOPR_OUTCH(buffer, *currlen, maxlen,
+ fconvert[--fplace]);
}
while (padlen < 0) {
- dopr_outch (buffer, currlen, maxlen, ' ');
+ DOPR_OUTCH(buffer, *currlen, maxlen, ' ');
++padlen;
}
-}
-
-static void dopr_outch(char *buffer, size_t *currlen, size_t maxlen, char c)
-{
- if (*currlen < maxlen) {
- buffer[(*currlen)] = c;
- }
- (*currlen)++;
+ return 0;
}
#endif /* !defined(HAVE_SNPRINTF) || !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);
}
#endif
#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;
va_list ap;
@@ -802,4 +841,3 @@
return ret;
}
#endif
-