Bounds check string functions in apps.

This includes strcat, strcpy and sprintf.

In the x509 app, the code has been cleaned up as well.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3868)

diff --git a/apps/enc.c b/apps/enc.c
index 338307330a7c31073955fb7cd9b9c075f3b92fa8..cc6fa0a1c3677f97b027004aed66e4c5a505f499 100644 (file)
--- a/apps/enc.c
+++ b/apps/enc.c
@@ -312,7 +312,7 @@ int enc_main(int argc, char **argv)
             for (;;) {
                 char prompt[200];
 
-                sprintf(prompt, "enter %s %s password:",
+                BIO_snprintf(prompt, sizeof(prompt), "enter %s %s password:",
                         OBJ_nid2ln(EVP_CIPHER_nid(cipher)),
                         (enc) ? "encryption" : "decryption");
                 strbuf[0] = '\0';
@@ -565,7 +565,7 @@ int enc_main(int argc, char **argv)
 #endif
     release_engine(e);
     OPENSSL_free(pass);
-    return (ret);
+    return ret;
 }
 
 static void show_ciphers(const OBJ_NAME *name, void *arg)
@@ -599,7 +599,7 @@ static int set_hex(char *in, unsigned char *out, int size)
     n = strlen(in);
     if (n > (size * 2)) {
         BIO_printf(bio_err, "hex string is too long\n");
-        return (0);
+        return 0;
     }
     memset(out, 0, size);
     for (i = 0; i < n; i++) {
@@ -609,7 +609,7 @@ static int set_hex(char *in, unsigned char *out, int size)
             break;
         if (!isxdigit(j)) {
             BIO_printf(bio_err, "non-hex digit\n");
-            return (0);
+            return 0;
         }
         j = (unsigned char)OPENSSL_hexchar2int(j);
         if (i & 1)
@@ -617,5 +617,5 @@ static int set_hex(char *in, unsigned char *out, int size)
         else
             out[i / 2] = (j << 4);
     }
-    return (1);
+    return 1;
 }

diff --git a/apps/pkcs12.c b/apps/pkcs12.c
index 82d2bb972e3a7a546e3abf21e8015ae59d09d961..2ec8fdc856063c3b765ec45f4d81255797053e9e 100644 (file)
--- a/apps/pkcs12.c
+++ b/apps/pkcs12.c
@@ -27,6 +27,8 @@ NON_EMPTY_TRANSLATION_UNIT
 # define CLCERTS         0x8
 # define CACERTS         0x10
 
+#define PASSWD_BUF_SIZE 2048
+
 static int get_cert_chain(X509 *cert, X509_STORE *store,
                           STACK_OF(X509) **chain);
 int dump_certs_keys_p12(BIO *out, const PKCS12 *p12,
@@ -119,7 +121,7 @@ int pkcs12_main(int argc, char **argv)
 {
     char *infile = NULL, *outfile = NULL, *keyname = NULL, *certfile = NULL;
     char *name = NULL, *csp_name = NULL;
-    char pass[2048] = "", macpass[2048] = "";
+    char pass[PASSWD_BUF_SIZE] = "", macpass[PASSWD_BUF_SIZE] = "";
     int export_cert = 0, options = 0, chain = 0, twopass = 0, keytype = 0;
     int iter = PKCS12_DEFAULT_ITER, maciter = PKCS12_DEFAULT_ITER;
 # ifndef OPENSSL_NO_RC2
@@ -455,7 +457,7 @@ int pkcs12_main(int argc, char **argv)
         }
 
         if (!twopass)
-            strcpy(macpass, pass);
+            OPENSSL_strlcpy(macpass, pass, sizeof(macpass));
 
         p12 = PKCS12_create(cpass, name, key, ucert, certs,
                             key_pbe, cert_pbe, iter, -1, keytype);
@@ -583,7 +585,7 @@ int pkcs12_main(int argc, char **argv)
     OPENSSL_free(badpass);
     OPENSSL_free(passin);
     OPENSSL_free(passout);
-    return (ret);
+    return ret;
 }
 
 int dump_certs_keys_p12(BIO *out, const PKCS12 *p12, const char *pass,

diff --git a/apps/s_time.c b/apps/s_time.c
index c4f4037363022e877a7bfa5dff96bc717c7e6267..b10c7e1da7ae0c5422a1110167580defbe448006 100644 (file)
--- a/apps/s_time.c
+++ b/apps/s_time.c
@@ -49,7 +49,13 @@
 
 static SSL *doConnection(SSL *scon, const char *host, SSL_CTX *ctx);
 
+/*
+ * Define a HTTP get command globally.
+ * Also define the size of the command, this is two bytes less than
+ * the size of the string because the %s is replaced by the URL.
+ */
 static const char fmt_http_get_cmd[] = "GET %s HTTP/1.0\r\n\r\n";
+static const size_t fmt_http_get_cmd_size = sizeof(fmt_http_get_cmd) - 2;
 
 typedef enum OPTION_choice {
     OPT_ERR = -1, OPT_EOF = 0, OPT_HELP,
@@ -173,7 +179,7 @@ int s_time_main(int argc, char **argv)
             break;
         case OPT_WWW:
             www_path = opt_arg();
-            buf_size = strlen(www_path) + sizeof(fmt_http_get_cmd);
+            buf_size = strlen(www_path) + fmt_http_get_cmd_size;
             if (buf_size > sizeof(buf)) {
                 BIO_printf(bio_err, "%s: -www option is too long\n", prog);
                 goto end;
@@ -230,9 +236,9 @@ int s_time_main(int argc, char **argv)
             goto end;
 
         if (www_path != NULL) {
-            sprintf(buf, fmt_http_get_cmd, www_path);
-            buf_len = strlen(buf);
-            if (SSL_write(scon, buf, buf_len) <= 0)
+            buf_len = BIO_snprintf(buf, sizeof(buf), fmt_http_get_cmd,
+                                   www_path);
+            if (buf_len <= 0 || SSL_write(scon, buf, buf_len) <= 0)
                 goto end;
             while ((i = SSL_read(scon, buf, sizeof(buf))) > 0)
                 bytes_read += i;
@@ -288,9 +294,8 @@ int s_time_main(int argc, char **argv)
     }
 
     if (www_path != NULL) {
-        sprintf(buf, fmt_http_get_cmd, www_path);
-        buf_len = strlen(buf);
-        if (SSL_write(scon, buf, buf_len) <= 0)
+        buf_len = BIO_snprintf(buf, sizeof(buf), fmt_http_get_cmd, www_path);
+        if (buf_len <= 0 || SSL_write(scon, buf, buf_len) <= 0)
             goto end;
         while (SSL_read(scon, buf, sizeof(buf)) > 0)
             continue;
@@ -319,8 +324,9 @@ int s_time_main(int argc, char **argv)
             goto end;
 
         if (www_path != NULL) {
-            sprintf(buf, "GET %s HTTP/1.0\r\n\r\n", www_path);
-            if (SSL_write(scon, buf, strlen(buf)) <= 0)
+            buf_len = BIO_snprintf(buf, sizeof(buf), fmt_http_get_cmd,
+                                   www_path);
+            if (buf_len <= 0 || SSL_write(scon, buf, buf_len) <= 0)
                 goto end;
             while ((i = SSL_read(scon, buf, sizeof(buf))) > 0)
                 bytes_read += i;
@@ -361,7 +367,7 @@ int s_time_main(int argc, char **argv)
  end:
     SSL_free(scon);
     SSL_CTX_free(ctx);
-    return (ret);
+    return ret;
 }
 
 /*-
@@ -375,7 +381,7 @@ static SSL *doConnection(SSL *scon, const char *host, SSL_CTX *ctx)
     fd_set readfds;
 
     if ((conn = BIO_new(BIO_s_connect())) == NULL)
-        return (NULL);
+        return NULL;
 
     BIO_set_conn_hostname(conn, host);
 

diff --git a/apps/x509.c b/apps/x509.c
index 484192bbf143cfa41055b901a47bd6928f394ecc..840e12778bf4ec8f7c3cf2ce67660d64f8f14b9b 100644 (file)
--- a/apps/x509.c
+++ b/apps/x509.c
@@ -890,34 +890,27 @@ int x509_main(int argc, char **argv)
     ASN1_OBJECT_free(objtmp);
     release_engine(e);
     OPENSSL_free(passin);
-    return (ret);
+    return ret;
 }
 
-static ASN1_INTEGER *x509_load_serial(const char *CAfile, const char *serialfile,
-                                      int create)
+static ASN1_INTEGER *x509_load_serial(const char *CAfile,
+                                      const char *serialfile, int create)
 {
-    char *buf = NULL, *p;
+    char *buf = NULL;
     ASN1_INTEGER *bs = NULL;
     BIGNUM *serial = NULL;
-    size_t len;
 
-    len = ((serialfile == NULL)
-           ? (strlen(CAfile) + strlen(POSTFIX) + 1)
-           : (strlen(serialfile))) + 1;
-    buf = app_malloc(len, "serial# buffer");
     if (serialfile == NULL) {
-        strcpy(buf, CAfile);
-        for (p = buf; *p; p++)
-            if (*p == '.') {
-                *p = '\0';
-                break;
-            }
-        strcat(buf, POSTFIX);
-    } else {
-        strcpy(buf, serialfile);
+        const char *p = strchr(CAfile, '.');
+        size_t len = p != NULL ? (size_t)(p - CAfile) : strlen(CAfile);
+
+        buf = app_malloc(len + sizeof(POSTFIX), "serial# buffer");
+        memcpy(buf, CAfile, len);
+        memcpy(buf + len, POSTFIX, sizeof(POSTFIX));
+        serialfile = buf;
     }
 
-    serial = load_serial(buf, create, NULL);
+    serial = load_serial(serialfile, create, NULL);
     if (serial == NULL)
         goto end;
 
@@ -926,7 +919,7 @@ static ASN1_INTEGER *x509_load_serial(const char *CAfile, const char *serialfile
         goto end;
     }
 
-    if (!save_serial(buf, NULL, serial, &bs))
+    if (!save_serial(serialfile, NULL, serial, &bs))
         goto end;
 
  end: