BIO range checking.

Add length limits to avoid problems with sprintf, strcpy and strcat.  This replaces recently removed code but also guards some previously missing function calls (for DOS & Windows).

Reworked the BIO_dump_indent_cb code to reduce temporary storage.

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3870)

3 files changed, 90 insertions, 81 deletions

diff --git a/crypto/bio/b_dump.c b/crypto/bio/b_dump.c
index 491b973369..f5391406e8 100644
--- a/crypto/bio/b_dump.c
+++ b/crypto/bio/b_dump.c
@@ -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
@@ -16,7 +16,9 @@
 
 #define TRUNCATE
 #define DUMP_WIDTH      16
-#define DUMP_WIDTH_LESS_INDENT(i) (DUMP_WIDTH-((i-(i>6?6:i)+3)/4))
+#define DUMP_WIDTH_LESS_INDENT(i) (DUMP_WIDTH - ((i - (i > 6 ? 6 : i) + 3) / 4))
+
+#define SPACE(buf, pos, n)   (sizeof(buf) - (pos) > (n))
 
 int BIO_dump_cb(int (*cb) (const void *data, size_t len, void *u),
                 void *u, const char *s, int len)
@@ -28,8 +30,8 @@ int BIO_dump_indent_cb(int (*cb) (const void *data, size_t len, void *u),
                        void *u, const char *s, int len, int indent)
 {
     int ret = 0;
-    char buf[288 + 1], tmp[20], str[128 + 1];
-    int i, j, rows, trc;
+    char buf[288 + 1];
+    int i, j, rows, trc, n;
     unsigned char ch;
     int dump_width;
 
@@ -42,59 +44,65 @@ int BIO_dump_indent_cb(int (*cb) (const void *data, size_t len, void *u),
 
     if (indent < 0)
         indent = 0;
-    if (indent) {
-        if (indent > 128)
-            indent = 128;
-        memset(str, ' ', indent);
-    }
-    str[indent] = '\0';
+    else if (indent > 128)
+        indent = 128;
 
     dump_width = DUMP_WIDTH_LESS_INDENT(indent);
-    rows = (len / dump_width);
+    rows = len / dump_width;
     if ((rows * dump_width) < len)
         rows++;
     for (i = 0; i < rows; i++) {
-        strcpy(buf, str);
-        sprintf(tmp, "%04x - ", i * dump_width);
-        strcat(buf, tmp);
+        n = BIO_snprintf(buf, sizeof(buf), "%*s%04x - ", indent, "",
+                         i * dump_width);
         for (j = 0; j < dump_width; j++) {
-            if (((i * dump_width) + j) >= len) {
-                strcat(buf, "   ");
-            } else {
-                ch = ((unsigned char)*(s + i * dump_width + j)) & 0xff;
-                sprintf(tmp, "%02x%c", ch, j == 7 ? '-' : ' ');
-                strcat(buf, tmp);
+            if (SPACE(buf, n, 3)) {
+                if (((i * dump_width) + j) >= len) {
+                    strcpy(buf + n, "   ");
+                } else {
+                    ch = ((unsigned char)*(s + i * dump_width + j)) & 0xff;
+                    BIO_snprintf(buf + n, 4, "%02x%c", ch,
+                                 j == 7 ? '-' : ' ');
+                }
+                n += 3;
             }
         }
-        strcat(buf, "  ");
+        if (SPACE(buf, n, 2)) {
+            strcpy(buf + n, "  ");
+            n += 2;
+        }
         for (j = 0; j < dump_width; j++) {
             if (((i * dump_width) + j) >= len)
                 break;
-            ch = ((unsigned char)*(s + i * dump_width + j)) & 0xff;
+            if (SPACE(buf, n, 1)) {
+                ch = ((unsigned char)*(s + i * dump_width + j)) & 0xff;
 #ifndef CHARSET_EBCDIC
-            sprintf(tmp, "%c", ((ch >= ' ') && (ch <= '~')) ? ch : '.');
+                buf[n++] = ((ch >= ' ') && (ch <= '~')) ? ch : '.';
 #else
-            sprintf(tmp, "%c",
-                         ((ch >= os_toascii[' ']) && (ch <= os_toascii['~']))
-                         ? os_toebcdic[ch]
-                         : '.');
+                buf[n++] = ((ch >= os_toascii[' ']) && (ch <= os_toascii['~']))
+                           ? os_toebcdic[ch]
+                           : '.';
 #endif
-            strcat(buf, tmp);
+                buf[n] = '\0';
+            }
+        }
+        if (SPACE(buf, n, 1)) {
+            buf[n++] = '\n';
+            buf[n] = '\0';
         }
-        strcat(buf, "\n");
         /*
          * if this is the last call then update the ddt_dump thing so that we
          * will move the selection point in the debug window
          */
-        ret += cb((void *)buf, strlen(buf), u);
+        ret += cb((void *)buf, n, u);
     }
 #ifdef TRUNCATE
     if (trc > 0) {
-        sprintf(buf, "%s%04x - <SPACES/NULS>\n", str, len + trc);
-        ret += cb((void *)buf, strlen(buf), u);
+        n = BIO_snprintf(buf, sizeof(buf), "%*s%04x - <SPACES/NULS>\n",
+                         indent, "", len + trc);
+        ret += cb((void *)buf, n, u);
     }
 #endif
-    return (ret);
+    return ret;
 }
 
 #ifndef OPENSSL_NO_STDIO
diff --git a/crypto/bio/bio_cb.c b/crypto/bio/bio_cb.c
index 13368e82ee..1154c233af 100644
--- a/crypto/bio/bio_cb.c
+++ b/crypto/bio/bio_cb.c
@@ -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
@@ -21,68 +21,69 @@ long BIO_debug_callback(BIO *bio, int cmd, const char *argp,
     char buf[256];
     char *p;
     long r = 1;
-    int len;
+    int len, left;
 
     if (BIO_CB_RETURN & cmd)
         r = ret;
 
-    len = sprintf(buf, "BIO[%p]: ", (void *)bio);
+    len = BIO_snprintf(buf, sizeof(buf), "BIO[%p]: ", (void *)bio);
 
     /* Ignore errors and continue printing the other information. */
     if (len < 0)
         len = 0;
     p = buf + len;
+    left = sizeof(buf) - len;
 
     switch (cmd) {
     case BIO_CB_FREE:
-        sprintf(p, "Free - %s\n", bio->method->name);
+        BIO_snprintf(p, left, "Free - %s\n", bio->method->name);
         break;
     case BIO_CB_READ:
         if (bio->method->type & BIO_TYPE_DESCRIPTOR)
-            sprintf(p, "read(%d,%lu) - %s fd=%d\n",
-                    bio->num, (unsigned long)argi,
-                    bio->method->name, bio->num);
+            BIO_snprintf(p, left, "read(%d,%lu) - %s fd=%d\n",
+                         bio->num, (unsigned long)argi,
+                         bio->method->name, bio->num);
         else
-            sprintf(p, "read(%d,%lu) - %s\n",
+            BIO_snprintf(p, left, "read(%d,%lu) - %s\n",
                     bio->num, (unsigned long)argi, bio->method->name);
         break;
     case BIO_CB_WRITE:
         if (bio->method->type & BIO_TYPE_DESCRIPTOR)
-            sprintf(p, "write(%d,%lu) - %s fd=%d\n",
-                    bio->num, (unsigned long)argi,
-                    bio->method->name, bio->num);
+            BIO_snprintf(p, left, "write(%d,%lu) - %s fd=%d\n",
+                         bio->num, (unsigned long)argi,
+                         bio->method->name, bio->num);
         else
-            sprintf(p, "write(%d,%lu) - %s\n",
-                    bio->num, (unsigned long)argi, bio->method->name);
+            BIO_snprintf(p, left, "write(%d,%lu) - %s\n",
+                         bio->num, (unsigned long)argi, bio->method->name);
         break;
     case BIO_CB_PUTS:
-        sprintf(p, "puts() - %s\n", bio->method->name);
+        BIO_snprintf(p, left, "puts() - %s\n", bio->method->name);
         break;
     case BIO_CB_GETS:
-        sprintf(p, "gets(%lu) - %s\n", (unsigned long)argi,
-                bio->method->name);
+        BIO_snprintf(p, left, "gets(%lu) - %s\n", (unsigned long)argi,
+                     bio->method->name);
         break;
     case BIO_CB_CTRL:
-        sprintf(p, "ctrl(%lu) - %s\n", (unsigned long)argi,
-                bio->method->name);
+        BIO_snprintf(p, left, "ctrl(%lu) - %s\n", (unsigned long)argi,
+                     bio->method->name);
         break;
     case BIO_CB_RETURN | BIO_CB_READ:
-        sprintf(p, "read return %ld\n", ret);
+        BIO_snprintf(p, left, "read return %ld\n", ret);
         break;
     case BIO_CB_RETURN | BIO_CB_WRITE:
-        sprintf(p, "write return %ld\n", ret);
+        BIO_snprintf(p, left, "write return %ld\n", ret);
         break;
     case BIO_CB_RETURN | BIO_CB_GETS:
-        sprintf(p, "gets return %ld\n", ret);
+        BIO_snprintf(p, left, "gets return %ld\n", ret);
         break;
     case BIO_CB_RETURN | BIO_CB_PUTS:
-        sprintf(p, "puts return %ld\n", ret);
+        BIO_snprintf(p, left, "puts return %ld\n", ret);
         break;
     case BIO_CB_RETURN | BIO_CB_CTRL:
-        sprintf(p, "ctrl return %ld\n", ret);
+        BIO_snprintf(p, left, "ctrl return %ld\n", ret);
         break;
     default:
-        sprintf(p, "bio callback - unknown type (%d)\n", cmd);
+        BIO_snprintf(p, left, "bio callback - unknown type (%d)\n", cmd);
         break;
     }
 
@@ -93,5 +94,5 @@ long BIO_debug_callback(BIO *bio, int cmd, const char *argp,
     else
         fputs(buf, stderr);
 #endif
-    return (r);
+    return r;
 }
diff --git a/crypto/bio/bss_file.c b/crypto/bio/bss_file.c
index 49d8f09bc5..e7bbc317a3 100644
--- a/crypto/bio/bss_file.c
+++ b/crypto/bio/bss_file.c
@@ -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
@@ -85,17 +85,17 @@ BIO *BIO_new_file(const char *filename, const char *mode)
             BIOerr(BIO_F_BIO_NEW_FILE, BIO_R_NO_SUCH_FILE);
         else
             BIOerr(BIO_F_BIO_NEW_FILE, ERR_R_SYS_LIB);
-        return (NULL);
+        return NULL;
     }
     if ((ret = BIO_new(BIO_s_file())) == NULL) {
         fclose(file);
-        return (NULL);
+        return NULL;
     }
 
     BIO_clear_flags(ret, BIO_FLAGS_UPLINK); /* we did fopen -> we disengage
                                              * UPLINK */
     BIO_set_fp(ret, file, fp_flags);
-    return (ret);
+    return ret;
 }
 
 BIO *BIO_new_fp(FILE *stream, int close_flag)
@@ -103,17 +103,17 @@ BIO *BIO_new_fp(FILE *stream, int close_flag)
     BIO *ret;
 
     if ((ret = BIO_new(BIO_s_file())) == NULL)
-        return (NULL);
+        return NULL;
 
     /* redundant flag, left for documentation purposes */
     BIO_set_flags(ret, BIO_FLAGS_UPLINK);
     BIO_set_fp(ret, stream, close_flag);
-    return (ret);
+    return ret;
 }
 
 const BIO_METHOD *BIO_s_file(void)
 {
-    return (&methods_filep);
+    return &methods_filep;
 }
 
 static int file_new(BIO *bi)
@@ -122,13 +122,13 @@ static int file_new(BIO *bi)
     bi->num = 0;
     bi->ptr = NULL;
     bi->flags = BIO_FLAGS_UPLINK; /* default to UPLINK */
-    return (1);
+    return 1;
 }
 
 static int file_free(BIO *a)
 {
     if (a == NULL)
-        return (0);
+        return 0;
     if (a->shutdown) {
         if ((a->init) && (a->ptr != NULL)) {
             if (a->flags & BIO_FLAGS_UPLINK)
@@ -140,7 +140,7 @@ static int file_free(BIO *a)
         }
         a->init = 0;
     }
-    return (1);
+    return 1;
 }
 
 static int file_read(BIO *b, char *out, int outl)
@@ -160,7 +160,7 @@ static int file_read(BIO *b, char *out, int outl)
             ret = -1;
         }
     }
-    return (ret);
+    return ret;
 }
 
 static int file_write(BIO *b, const char *in, int inl)
@@ -181,7 +181,7 @@ static int file_write(BIO *b, const char *in, int inl)
          * implementations (VMS)
          */
     }
-    return (ret);
+    return ret;
 }
 
 static long file_ctrl(BIO *b, int cmd, long num, void *ptr)
@@ -271,15 +271,15 @@ static long file_ctrl(BIO *b, int cmd, long num, void *ptr)
         b->shutdown = (int)num & BIO_CLOSE;
         if (num & BIO_FP_APPEND) {
             if (num & BIO_FP_READ)
-                strcpy(p, "a+");
+                OPENSSL_strlcpy(p, "a+", sizeof(p));
             else
-                strcpy(p, "a");
+                OPENSSL_strlcpy(p, "a", sizeof(p));
         } else if ((num & BIO_FP_READ) && (num & BIO_FP_WRITE))
-            strcpy(p, "r+");
+            OPENSSL_strlcpy(p, "r+", sizeof(p));
         else if (num & BIO_FP_WRITE)
-            strcpy(p, "w");
+            OPENSSL_strlcpy(p, "w", sizeof(p));
         else if (num & BIO_FP_READ)
-            strcpy(p, "r");
+            OPENSSL_strlcpy(p, "r", sizeof(p));
         else {
             BIOerr(BIO_F_FILE_CTRL, BIO_R_BAD_FOPEN_MODE);
             ret = 0;
@@ -287,9 +287,9 @@ static long file_ctrl(BIO *b, int cmd, long num, void *ptr)
         }
 #  if defined(OPENSSL_SYS_MSDOS) || defined(OPENSSL_SYS_WINDOWS) || defined(OPENSSL_SYS_WIN32_CYGWIN)
         if (!(num & BIO_FP_TEXT))
-            strcat(p, "b");
+            OPENSSL_strlcat(p, "b", sizeof(p));
         else
-            strcat(p, "t");
+            OPENSSL_strlcat(p, "t", sizeof(p));
 #  endif
         fp = openssl_fopen(ptr, p);
         if (fp == NULL) {
@@ -339,7 +339,7 @@ static long file_ctrl(BIO *b, int cmd, long num, void *ptr)
         ret = 0;
         break;
     }
-    return (ret);
+    return ret;
 }
 
 static int file_gets(BIO *bp, char *buf, int size)
@@ -357,7 +357,7 @@ static int file_gets(BIO *bp, char *buf, int size)
     if (buf[0] != '\0')
         ret = strlen(buf);
  err:
-    return (ret);
+    return ret;
 }
 
 static int file_puts(BIO *bp, const char *str)
@@ -366,7 +366,7 @@ static int file_puts(BIO *bp, const char *str)
 
     n = strlen(str);
     ret = file_write(bp, str, n);
-    return (ret);
+    return ret;
 }
 
 #else
@@ -419,7 +419,7 @@ static const BIO_METHOD methods_filep = {
 
 const BIO_METHOD *BIO_s_file(void)
 {
-    return (&methods_filep);
+    return &methods_filep;
 }
 
 BIO *BIO_new_file(const char *filename, const char *mode)