]> git.ipfire.org Git - thirdparty/curl.git/commitdiff
mprintf: Fix stack overflows
authorTobias Stoeckmann <tobias@stoeckmann.org>
Sat, 25 Jul 2020 15:30:12 +0000 (17:30 +0200)
committerJay Satiro <raysatiro@yahoo.com>
Mon, 27 Jul 2020 07:43:00 +0000 (03:43 -0400)
Stack overflows can occur with precisions for integers and floats.

Proof of concepts:
- curl_mprintf("%d, %.*1$d", 500, 1);
- curl_mprintf("%d, %+0500.*1$f", 500, 1);

Ideally, compile with -fsanitize=address which makes this undefined
behavior a bit more defined for debug purposes.

The format strings are valid. The overflows occur due to invalid
arguments. If these arguments are variables with contents controlled
by an attacker, the function's stack can be corrupted.

Also see CVE-2016-9586 which partially fixed the float aspect.

Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Closes https://github.com/curl/curl/pull/5722

lib/mprintf.c
tests/libtest/lib557.c

index 7af2f4a8320c2e3683ce2e8efcf0a0989fdf1f83..80735be51bd2a8c2f45c709f2525faf9a0347b7b 100644 (file)
@@ -764,7 +764,7 @@ static int dprintf_formatf(
 
       if(prec > 0) {
         width -= prec;
-        while(prec-- > 0)
+        while(prec-- > 0 && w >= work)
           *w-- = '0';
       }
 
@@ -928,6 +928,8 @@ static int dprintf_formatf(
              precision */
           size_t maxprec = sizeof(work) - 2;
           double val = p->data.dnum;
+          if(width > 0 && prec <= width)
+            maxprec -= width;
           while(val >= 10.0) {
             val /= 10;
             maxprec--;
@@ -935,6 +937,8 @@ static int dprintf_formatf(
 
           if(prec > (long)maxprec)
             prec = (long)maxprec-1;
+          if(prec < 0)
+            prec = 0;
           /* RECURSIVE USAGE */
           len = curl_msnprintf(fptr, left, ".%ld", prec);
           fptr += len;
index 2e51b99c11b1cb0b8f086dc507ad3dc6a736e575..a1be9b7df391c1e62fd910e13fada52c259f8ea0 100644 (file)
@@ -1537,6 +1537,17 @@ static int test_weird_arguments(void)
 
   errors += string_check(buf, "");
 
+  /* Do not skip sanity checks with parameters! */
+  buf[0] = 0;
+  rc = curl_msnprintf(buf, sizeof(buf), "%d, %.*1$d", 500, 1);
+
+  if(rc != 256) {
+    printf("curl_mprintf() returned %d and not 256!\n", rc);
+    errors++;
+  }
+
+  errors += strlen_check(buf, 255);
+
   if(errors)
     printf("Some curl_mprintf() weird arguments tests failed!\n");