- (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@

1 files changed, 101 insertions, 63 deletions

diff --git a/openbsd-compat/bsd-snprintf.c b/openbsd-compat/bsd-snprintf.c
index 04651e1d4..cefb1d1ad 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);
-
-static size_t dopr(char *buffer, size_t maxlen, const char *format, va_list args_in)
+#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 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 @@ static size_t dopr(char *buffer, size_t maxlen, const char *format, va_list args
                 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 @@ static size_t dopr(char *buffer, size_t maxlen, const char *format, va_list args
                                         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 @@ static size_t dopr(char *buffer, size_t maxlen, const char *format, va_list args
                                         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 @@ static size_t dopr(char *buffer, size_t maxlen, const char *format, va_list args
                                         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 @@ static size_t dopr(char *buffer, size_t maxlen, const char *format, va_list args
                                         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 @@ static size_t dopr(char *buffer, size_t maxlen, const char *format, va_list args
                                         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 @@ static size_t dopr(char *buffer, size_t maxlen, const char *format, va_list args
                                         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 @@ static size_t dopr(char *buffer, size_t maxlen, const char *format, va_list args
                                         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 @@ static size_t dopr(char *buffer, size_t maxlen, const char *format, va_list args
                                 }
                                 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 @@ static size_t dopr(char *buffer, size_t maxlen, const char *format, va_list args
                         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 @@ static void fmtstr(char *buffer, size_t *currlen, size_t maxlen,
                 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 @@ static void fmtint(char *buffer, size_t *currlen, size_t maxlen,
 
         /* 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 @@ static LDOUBLE abs_val(LDOUBLE value)
         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 @@ static double my_modf(double x0, double *iptr)
         }
 
         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 double my_modf(double x0, double *iptr)
 }
 
 
-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 @@ static void fmtfp (char *buffer, size_t *currlen, size_t maxlen,
         
         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 @@ static void fmtfp (char *buffer, size_t *currlen, size_t maxlen,
          * 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 @@ int snprintf(char *str, size_t count, SNPRINTF_CONST char *fmt, ...)
         return ret;
 }
 #endif
-