snprintf() made a single-byte buffer overflow, as it could write a zero

outside its given buffer. Discovered and reported by James Bursa.

diff --git a/lib/mprintf.c b/lib/mprintf.c
index af21041707a97f78a61e47e2eedffd50722fade4..6cb345a58922889b2f617b5d2aea3808c7522ebf 100644 (file)
--- a/lib/mprintf.c
+++ b/lib/mprintf.c
@@ -961,9 +961,9 @@ static int addbyter(int output, FILE *data)
   return -1;
 }
 
-int curl_msnprintf(char *buffer, size_t maxlength, const char *format, ...)
+int curl_mvsnprintf(char *buffer, size_t maxlength, const char *format,
+                    va_list ap_save)
 {
-  va_list ap_save; /* argument pointer */
   int retcode;
   struct nsprintf info;
 
@@ -971,31 +971,28 @@ int curl_msnprintf(char *buffer, size_t maxlength, const char *format, ...)
   info.length = 0;
   info.max = maxlength;
 
-  va_start(ap_save, format);
   retcode = dprintf_formatf(&info, addbyter, format, ap_save);
-  va_end(ap_save);
-  info.buffer[0] = 0; /* we terminate this with a zero byte */
-
-  /* we could even return things like */
-  
+  if(info.max) {
+    /* we terminate this with a zero byte */
+    if(info.max == info.length)
+      /* we're at maximum, scrap the last letter */
+      info.buffer[-1] = 0;
+    else
+      info.buffer[0] = 0;
+  }
   return retcode;
 }
 
-int curl_mvsnprintf(char *buffer, size_t maxlength, const char *format, va_list ap_save)
+int curl_msnprintf(char *buffer, size_t maxlength, const char *format, ...)
 {
   int retcode;
-  struct nsprintf info;
-
-  info.buffer = buffer;
-  info.length = 0;
-  info.max = maxlength;
-
-  retcode = dprintf_formatf(&info, addbyter, format, ap_save);
-  info.buffer[0] = 0; /* we terminate this with a zero byte */
+  va_list ap_save; /* argument pointer */
+  va_start(ap_save, format);
+  retcode = curl_mvsnprintf(buffer, maxlength, format, ap_save);
+  va_end(ap_save);
   return retcode;
 }
 
-
 /* fputc() look-alike */
 static int alloc_addbyter(int output, FILE *data)
 {