Address potential buffer overflows.
authorPauli <paul.dale@oracle.com>
Fri, 7 Jul 2017 00:17:59 +0000 (10:17 +1000)
committerPauli <paul.dale@oracle.com>
Fri, 7 Jul 2017 03:37:06 +0000 (13:37 +1000)
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3878)

crypto/bn/bn_print.c
crypto/mem_dbg.c
crypto/pem/pem_lib.c

index 956b2d520f6d1a2ced6e7b33da3385cf828c29e9..9f849978d8bcb4c2e680d0ed77e698a83216d625 100644 (file)
@@ -52,7 +52,7 @@ char *BN_bn2hex(const BIGNUM *a)
 /* Must 'OPENSSL_free' the returned data */
 char *BN_bn2dec(const BIGNUM *a)
 {
-    int i = 0, num, ok = 0;
+    int i = 0, num, ok = 0, n, tbytes;
     char *buf = NULL;
     char *p;
     BIGNUM *t = NULL;
@@ -67,9 +67,10 @@ char *BN_bn2dec(const BIGNUM *a)
      */
     i = BN_num_bits(a) * 3;
     num = (i / 10 + i / 1000 + 1) + 1;
+    tbytes = num + 3;   /* negative and terminator and one spare? */
     bn_data_num = num / BN_DEC_NUM + 1;
     bn_data = OPENSSL_malloc(bn_data_num * sizeof(BN_ULONG));
-    buf = OPENSSL_malloc(num + 3);
+    buf = OPENSSL_malloc(tbytes);
     if ((buf == NULL) || (bn_data == NULL)) {
         BNerr(BN_F_BN_BN2DEC, ERR_R_MALLOC_FAILURE);
         goto err;
@@ -100,14 +101,16 @@ char *BN_bn2dec(const BIGNUM *a)
          * the last one needs truncation. The blocks need to be reversed in
          * order.
          */
-        sprintf(p, BN_DEC_FMT1, *lp);
-        while (*p)
-            p++;
+        n = BIO_snprintf(p, tbytes - (size_t)(p - buf), BN_DEC_FMT1, *lp);
+        if (n < 0)
+            goto err;
+        p += n;
         while (lp != bn_data) {
             lp--;
-            sprintf(p, BN_DEC_FMT2, *lp);
-            while (*p)
-                p++;
+            n = BIO_snprintf(p, tbytes - (size_t)(p - buf), BN_DEC_FMT2, *lp);
+            if (n < 0)
+                goto err;
+            p += n;
         }
     }
     ok = 1;
@@ -331,11 +334,11 @@ char *BN_options(void)
     if (!init) {
         init++;
 #ifdef BN_LLONG
-        sprintf(data, "bn(%d,%d)",
-                (int)sizeof(BN_ULLONG) * 8, (int)sizeof(BN_ULONG) * 8);
+        BIO_snprintf(data, sizeof(data), "bn(%zu,%zu)",
+                     sizeof(BN_ULLONG) * 8, sizeof(BN_ULONG) * 8);
 #else
-        sprintf(data, "bn(%d,%d)",
-                (int)sizeof(BN_ULONG) * 8, (int)sizeof(BN_ULONG) * 8);
+        BIO_snprintf(data, sizeof(data), "bn(%zu,%zu)",
+                     sizeof(BN_ULONG) * 8, sizeof(BN_ULONG) * 8);
 #endif
     }
     return data;
index 1ab52a86e857698dfa3847e844d2b9ed6cdd1b8c..70b5e62ab55fc6d77b409daf2dd7ccad1aba6e0c 100644 (file)
@@ -453,8 +453,9 @@ static void print_leak(const MEM *m, MEM_LEAK *l)
 {
     char buf[1024];
     char *bufp = buf;
+    size_t len = sizeof(buf), ami_cnt;
     APP_INFO *amip;
-    int ami_cnt;
+    int n;
     struct tm *lcl = NULL;
     /*
      * Convert between CRYPTO_THREAD_ID (which could be anything at all) and
@@ -468,21 +469,37 @@ static void print_leak(const MEM *m, MEM_LEAK *l)
     CRYPTO_THREAD_ID ti;
 
     lcl = localtime(&m->time);
-    sprintf(bufp, "[%02d:%02d:%02d] ", lcl->tm_hour, lcl->tm_min, lcl->tm_sec);
-    bufp += strlen(bufp);
+    n = BIO_snprintf(bufp, len, "[%02d:%02d:%02d] ",
+                     lcl->tm_hour, lcl->tm_min, lcl->tm_sec);
+    if (n <= 0) {
+        bufp[0] = '\0';
+        return;
+    }
+    bufp += n;
+    len -= n;
 
-    sprintf(bufp, "%5lu file=%s, line=%d, ", m->order, m->file, m->line);
-    bufp += strlen(bufp);
+    n = BIO_snprintf(bufp, len, "%5lu file=%s, line=%d, ",
+                     m->order, m->file, m->line);
+    if (n <= 0)
+        return;
+    bufp += n;
+    len -= n;
 
     tid.ltid = 0;
     tid.tid = m->threadid;
-    sprintf(bufp, "thread=%lu, ", tid.ltid);
-    bufp += strlen(bufp);
+    n = BIO_snprintf(bufp, len, "thread=%lu, ", tid.ltid);
+    if (n <= 0)
+        return;
+    bufp += n;
+    len -= n;
 
-    sprintf(bufp, "number=%d, address=%p\n", m->num, m->addr);
-    bufp += strlen(bufp);
+    n = BIO_snprintf(bufp, len, "number=%d, address=%p\n", m->num, m->addr);
+    if (n <= 0)
+        return;
+    bufp += n;
+    len -= n;
 
-    l->print_cb(buf, strlen(buf), l->print_cb_arg);
+    l->print_cb(buf, (size_t)(bufp - buf), l->print_cb_arg);
 
     l->chunks++;
     l->bytes += m->num;
@@ -498,23 +515,34 @@ static void print_leak(const MEM *m, MEM_LEAK *l)
             int info_len;
 
             ami_cnt++;
+            if (ami_cnt >= sizeof(buf) - 1)
+                break;
             memset(buf, '>', ami_cnt);
+            buf[ami_cnt] = '\0';
             tid.ltid = 0;
             tid.tid = amip->threadid;
-            sprintf(buf + ami_cnt, " thread=%lu, file=%s, line=%d, info=\"",
-                    tid.ltid, amip->file, amip->line);
-            buf_len = strlen(buf);
+            n = BIO_snprintf(buf + ami_cnt, sizeof(buf) - ami_cnt,
+                             " thread=%lu, file=%s, line=%d, info=\"",
+                             tid.ltid, amip->file, amip->line);
+            if (n <= 0)
+                break;
+            buf_len = ami_cnt + n;
             info_len = strlen(amip->info);
             if (128 - buf_len - 3 < info_len) {
                 memcpy(buf + buf_len, amip->info, 128 - buf_len - 3);
                 buf_len = 128 - 3;
             } else {
-                strcpy(buf + buf_len, amip->info);
-                buf_len = strlen(buf);
+                n = BIO_snprintf(buf + buf_len, sizeof(buf) - buf_len, "%s",
+                                 amip->info);
+                if (n < 0)
+                    break;
+                buf_len += n;
             }
-            sprintf(buf + buf_len, "\"\n");
+            n = BIO_snprintf(buf + buf_len, sizeof(buf) - buf_len, "\"\n");
+            if (n <= 0)
+                break;
 
-            l->print_cb(buf, strlen(buf), l->print_cb_arg);
+            l->print_cb(buf, buf_len + n, l->print_cb_arg);
 
             amip = amip->next;
         }
index 7e5e3747b080c05d090bbb56131d18fef4929168..aacdad9e632e0c7c095401efeb61ec90159a0113 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright 1995-2016 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 1995-2017 The OpenSSL Project Authors. All Rights Reserved.
  *
  * Licensed under the OpenSSL license (the "License").  You may not use
  * this file except in compliance with the License.  You can obtain a copy
@@ -71,6 +71,7 @@ int PEM_def_callback(char *buf, int num, int w, void *key)
 void PEM_proc_type(char *buf, int type)
 {
     const char *str;
+    char *p = buf + strlen(buf);
 
     if (type == PEM_TYPE_ENCRYPTED)
         str = "ENCRYPTED";
@@ -81,27 +82,29 @@ void PEM_proc_type(char *buf, int type)
     else
         str = "BAD-TYPE";
 
-    strcat(buf, "Proc-Type: 4,");
-    strcat(buf, str);
-    strcat(buf, "\n");
+    BIO_snprintf(p, PEM_BUFSIZE - (size_t)(p - buf), "Proc-Type: 4,%s\n", str);
 }
 
 void PEM_dek_info(char *buf, const char *type, int len, char *str)
 {
-    static const unsigned char map[17] = "0123456789ABCDEF";
     long i;
-    int j;
-
-    strcat(buf, "DEK-Info: ");
-    strcat(buf, type);
-    strcat(buf, ",");
-    j = strlen(buf);
-    for (i = 0; i < len; i++) {
-        buf[j + i * 2] = map[(str[i] >> 4) & 0x0f];
-        buf[j + i * 2 + 1] = map[(str[i]) & 0x0f];
-    }
-    buf[j + i * 2] = '\n';
-    buf[j + i * 2 + 1] = '\0';
+    char *p = buf + strlen(buf);
+    int j = PEM_BUFSIZE - (size_t)(p - buf), n;
+
+    n = BIO_snprintf(p, j, "DEK-Info: %s,", type);
+    if (n > 0) {
+        j -= n;
+        p += n;
+        for (i = 0; i < len; i++) {
+            n = BIO_snprintf(p, j, "%02X", 0xff & str[i]);
+            if (n <= 0)
+                return;
+            j -= n;
+            p += n;
+        }
+        if (j > 1)
+            strcpy(p, "\n");
+    }
 }
 
 #ifndef OPENSSL_NO_STDIO