Memory bounds checking in asn1 code.

Check that sprint, strcpy don't overflow.

Avoid some strlen operations when the previous sprintf return value can be used.

Also fix the undefined behaviour `*(long *)x = y` when x isn't a long or character pointer.
ISO/IEC 9899:1999 6.5/7 for the details.

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

6 files changed, 55 insertions, 48 deletions

diff --git a/crypto/asn1/a_gentm.c b/crypto/asn1/a_gentm.c
index 2c5fb1c3d2..5cfc3ff15e 100644
--- a/crypto/asn1/a_gentm.c
+++ b/crypto/asn1/a_gentm.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
@@ -207,7 +207,7 @@ ASN1_GENERALIZEDTIME *ASN1_GENERALIZEDTIME_adj(ASN1_GENERALIZEDTIME *s,
     char *p;
     struct tm *ts;
     struct tm data;
-    size_t len = 20;
+    const size_t len = 20;
     ASN1_GENERALIZEDTIME *tmps = NULL;
 
     if (s == NULL)
@@ -237,10 +237,10 @@ ASN1_GENERALIZEDTIME *ASN1_GENERALIZEDTIME_adj(ASN1_GENERALIZEDTIME *s,
         tmps->data = (unsigned char *)p;
     }
 
-    sprintf(p, "%04d%02d%02d%02d%02d%02dZ", ts->tm_year + 1900,
-            ts->tm_mon + 1, ts->tm_mday, ts->tm_hour, ts->tm_min,
-            ts->tm_sec);
-    tmps->length = strlen(p);
+    tmps->length = BIO_snprintf(p, len, "%04d%02d%02d%02d%02d%02dZ",
+                                ts->tm_year + 1900, ts->tm_mon + 1,
+                                ts->tm_mday, ts->tm_hour, ts->tm_min,
+                                ts->tm_sec);
     tmps->type = V_ASN1_GENERALIZEDTIME;
 #ifdef CHARSET_EBCDIC_not
     ebcdic2ascii(tmps->data, tmps->data, tmps->length);
diff --git a/crypto/asn1/a_mbstr.c b/crypto/asn1/a_mbstr.c
index 46764b292c..e644fe0542 100644
--- a/crypto/asn1/a_mbstr.c
+++ b/crypto/asn1/a_mbstr.c
@@ -1,5 +1,5 @@
 /*
- * Copyright 1999-2016 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 1999-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
@@ -100,14 +100,14 @@ int ASN1_mbstring_ncopy(ASN1_STRING **out, const unsigned char *in, int len,
 
     if ((minsize > 0) && (nchar < minsize)) {
         ASN1err(ASN1_F_ASN1_MBSTRING_NCOPY, ASN1_R_STRING_TOO_SHORT);
-        sprintf(strbuf, "%ld", minsize);
+        BIO_snprintf(strbuf, sizeof(strbuf), "%ld", minsize);
         ERR_add_error_data(2, "minsize=", strbuf);
         return -1;
     }
 
     if ((maxsize > 0) && (nchar > maxsize)) {
         ASN1err(ASN1_F_ASN1_MBSTRING_NCOPY, ASN1_R_STRING_TOO_LONG);
-        sprintf(strbuf, "%ld", maxsize);
+        BIO_snprintf(strbuf, sizeof(strbuf), "%ld", maxsize);
         ERR_add_error_data(2, "maxsize=", strbuf);
         return -1;
     }
diff --git a/crypto/asn1/a_time.c b/crypto/asn1/a_time.c
index f0ec42f71c..fc78e309a6 100644
--- a/crypto/asn1/a_time.c
+++ b/crypto/asn1/a_time.c
@@ -1,5 +1,5 @@
 /*
- * Copyright 1999-2016 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 1999-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
@@ -81,17 +81,20 @@ ASN1_GENERALIZEDTIME *ASN1_TIME_to_generalizedtime(const ASN1_TIME *t,
         goto done;
     }
 
-    /* grow the string */
+    /*
+     * Grow the string by two bytes.
+     * The actual allocation is t->length + 3 to include a terminator byte.
+     */
     if (!ASN1_STRING_set(ret, NULL, t->length + 2))
         goto err;
     str = (char *)ret->data;
     /* Work out the century and prepend */
-    if (t->data[0] >= '5')
-        strcpy(str, "19");
-    else
-        strcpy(str, "20");
-
-    strcat(str, (const char *)t->data);
+    memcpy(str, t->data[0] >= '5' ? "19" : "20", 2);
+    /*
+     * t->length + 1 is the size of the data and the allocated buffer has
+     * this much space after the first two characters.
+     */
+    OPENSSL_strlcpy(str + 2, (const char *)t->data, t->length + 1);
 
  done:
    if (out != NULL && *out == NULL)
diff --git a/crypto/asn1/a_utctm.c b/crypto/asn1/a_utctm.c
index 25393ee152..5a4b1742f7 100644
--- a/crypto/asn1/a_utctm.c
+++ b/crypto/asn1/a_utctm.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,7 +21,7 @@ int asn1_utctime_to_tm(struct tm *tm, const ASN1_UTCTIME *d)
     int n, i, l, o, min_l = 11, strict = 0;
 
     if (d->type != V_ASN1_UTCTIME)
-        return (0);
+        return 0;
     l = d->length;
     a = (char *)d->data;
     o = 0;
@@ -150,9 +150,9 @@ int ASN1_UTCTIME_set_string(ASN1_UTCTIME *s, const char *str)
                 return 0;
             s->type = V_ASN1_UTCTIME;
         }
-        return (1);
-    } else
-        return (0);
+        return 1;
+    }
+    return 0;
 }
 
 ASN1_UTCTIME *ASN1_UTCTIME_set(ASN1_UTCTIME *s, time_t t)
@@ -166,7 +166,7 @@ ASN1_UTCTIME *ASN1_UTCTIME_adj(ASN1_UTCTIME *s, time_t t,
     char *p;
     struct tm *ts;
     struct tm data;
-    size_t len = 20;
+    const size_t len = 20;
     int free_s = 0;
 
     if (s == NULL) {
@@ -199,15 +199,14 @@ ASN1_UTCTIME *ASN1_UTCTIME_adj(ASN1_UTCTIME *s, time_t t,
         s->data = (unsigned char *)p;
     }
 
-    sprintf(p, "%02d%02d%02d%02d%02d%02dZ", ts->tm_year % 100,
-            ts->tm_mon + 1, ts->tm_mday, ts->tm_hour, ts->tm_min,
-            ts->tm_sec);
-    s->length = strlen(p);
+    s->length = BIO_snprintf(p, len, "%02d%02d%02d%02d%02d%02dZ",
+                             ts->tm_year % 100, ts->tm_mon + 1, ts->tm_mday,
+                             ts->tm_hour, ts->tm_min, ts->tm_sec);
     s->type = V_ASN1_UTCTIME;
 #ifdef CHARSET_EBCDIC_not
     ebcdic2ascii(s->data, s->data, s->length);
 #endif
-    return (s);
+    return s;
  err:
     if (free_s)
         ASN1_UTCTIME_free(s);
@@ -272,10 +271,9 @@ int ASN1_UTCTIME_print(BIO *bp, const ASN1_UTCTIME *tm)
     if (BIO_printf(bp, "%s %2d %02d:%02d:%02d %d%s",
                    _asn1_mon[M - 1], d, h, m, s, y + 1900,
                    (gmt) ? " GMT" : "") <= 0)
-        return (0);
-    else
-        return (1);
+        return 0;
+    return 1;
  err:
     BIO_write(bp, "Bad time value", 14);
-    return (0);
+    return 0;
 }
diff --git a/crypto/asn1/asn1_par.c b/crypto/asn1/asn1_par.c
index 19b21e7d49..4b60c615de 100644
--- a/crypto/asn1/asn1_par.c
+++ b/crypto/asn1/asn1_par.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
@@ -38,32 +38,32 @@ static int asn1_print_info(BIO *bp, int tag, int xclass, int constructed,
 
     p = str;
     if ((xclass & V_ASN1_PRIVATE) == V_ASN1_PRIVATE)
-        sprintf(str, "priv [ %d ] ", tag);
+        BIO_snprintf(str, sizeof(str), "priv [ %d ] ", tag);
     else if ((xclass & V_ASN1_CONTEXT_SPECIFIC) == V_ASN1_CONTEXT_SPECIFIC)
-        sprintf(str, "cont [ %d ]", tag);
+        BIO_snprintf(str, sizeof(str), "cont [ %d ]", tag);
     else if ((xclass & V_ASN1_APPLICATION) == V_ASN1_APPLICATION)
-        sprintf(str, "appl [ %d ]", tag);
+        BIO_snprintf(str, sizeof(str), "appl [ %d ]", tag);
     else if (tag > 30)
-        sprintf(str, "<ASN1 %d>", tag);
+        BIO_snprintf(str, sizeof(str), "<ASN1 %d>", tag);
     else
         p = ASN1_tag2str(tag);
 
     if (BIO_printf(bp, fmt, p) <= 0)
         goto err;
-    return (1);
+    return 1;
  err:
-    return (0);
+    return 0;
 }
 
 int ASN1_parse(BIO *bp, const unsigned char *pp, long len, int indent)
 {
-    return (asn1_parse2(bp, &pp, len, 0, 0, indent, 0));
+    return asn1_parse2(bp, &pp, len, 0, 0, indent, 0);
 }
 
 int ASN1_parse_dump(BIO *bp, const unsigned char *pp, long len, int indent,
                     int dump)
 {
-    return (asn1_parse2(bp, &pp, len, 0, 0, indent, dump));
+    return asn1_parse2(bp, &pp, len, 0, 0, indent, dump);
 }
 
 static int asn1_parse2(BIO *bp, const unsigned char **pp, long length,
@@ -342,7 +342,7 @@ static int asn1_parse2(BIO *bp, const unsigned char **pp, long length,
     ASN1_OBJECT_free(o);
     ASN1_OCTET_STRING_free(os);
     *pp = p;
-    return (ret);
+    return ret;
 }
 
 const char *ASN1_tag2str(int tag)
diff --git a/crypto/asn1/x_long.c b/crypto/asn1/x_long.c
index 78f4b764c3..bf9371ef55 100644
--- a/crypto/asn1/x_long.c
+++ b/crypto/asn1/x_long.c
@@ -1,5 +1,5 @@
 /*
- * Copyright 2000-2016 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 2000-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
@@ -14,6 +14,9 @@
 #if !(OPENSSL_API_COMPAT < 0x10200000L)
 NON_EMPTY_TRANSLATION_UNIT
 #else
+
+#define COPY_SIZE(a, b) (sizeof(a) < sizeof(b) ? sizeof(a) : sizeof(b))
+
 /*
  * Custom primitive type for long handling. This converts between an
  * ASN1_INTEGER and a long directly.
@@ -49,13 +52,13 @@ ASN1_ITEM_end(ZLONG)
 
 static int long_new(ASN1_VALUE **pval, const ASN1_ITEM *it)
 {
-    *(long *)pval = it->size;
+    memcpy(pval, &it->size, COPY_SIZE(*pval, it->size));
     return 1;
 }
 
 static void long_free(ASN1_VALUE **pval, const ASN1_ITEM *it)
 {
-    *(long *)pval = it->size;
+    memcpy(pval, &it->size, COPY_SIZE(*pval, it->size));
 }
 
 /*
@@ -90,7 +93,7 @@ static int long_i2c(ASN1_VALUE **pval, unsigned char *cont, int *putype,
     unsigned long utmp, sign;
     int clen, pad, i;
 
-    ltmp = *(long *)pval;
+    memcpy(&ltmp, pval, COPY_SIZE(*pval, ltmp));
     if (ltmp == it->size)
         return -1;
     /*
@@ -183,13 +186,16 @@ static int long_c2i(ASN1_VALUE **pval, const unsigned char *cont, int len,
         ASN1err(ASN1_F_LONG_C2I, ASN1_R_INTEGER_TOO_LARGE_FOR_LONG);
         return 0;
     }
-    *(long*)pval = ltmp;
+    memcpy(pval, &ltmp, COPY_SIZE(*pval, ltmp));
     return 1;
 }
 
 static int long_print(BIO *out, ASN1_VALUE **pval, const ASN1_ITEM *it,
                       int indent, const ASN1_PCTX *pctx)
 {
-    return BIO_printf(out, "%ld\n", *(long *)pval);
+    long l;
+
+    memcpy(&l, pval, COPY_SIZE(*pval, l));
+    return BIO_printf(out, "%ld\n", l);
 }
 #endif