Combined patch for the more or less obvious issues

Fixed a memory leak in ASN1_digest and ASN1_item_digest.

asn1_template_noexp_d2i call ASN1_item_ex_free(&skfield,...) on error.

Reworked error handling in asn1_item_ex_combine_new:
- call ASN1_item_ex_free and return the correct error code if ASN1_template_new failed.
- dont call ASN1_item_ex_free if ASN1_OP_NEW_PRE failed.

Reworked error handing in x509_name_ex_d2i and x509_name_encode.

Fixed error handling in int_ctx_new and EVP_PKEY_CTX_dup.

Fixed a memory leak in def_get_class if lh_EX_CLASS_ITEM_insert fails due to OOM:
- to figure out if the insertion succeeded, use lh_EX_CLASS_ITEM_retrieve again.
- on error, p will be NULL, and gen needs to be cleaned up again.

int_free_ex_data needs to have a fallback solution if unable to allocate "storage":
- if free_func is non-zero this must be called to clean up all memory.

Fixed error handling in pkey_hmac_copy.

Fixed error handling in ssleay_rand_add and ssleay_rand_bytes.

Fixed error handling in X509_STORE_new.

Fixed a memory leak in ssl3_get_key_exchange.

Check for null pointer in ssl3_write_bytes.

Check for null pointer in ssl3_get_cert_verify.

Fixed a memory leak in ssl_cert_dup.

Fixes #2087 #2094 #2103 #2104 #2105 #2106 #2107 #2108 #2110 #2111 #2112 #2115

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

9 files changed, 130 insertions, 72 deletions

diff --git a/crypto/asn1/a_digest.c b/crypto/asn1/a_digest.c
index 7cbc4751cd..57a04f768c 100644
--- a/crypto/asn1/a_digest.c
+++ b/crypto/asn1/a_digest.c
@@ -86,8 +86,10 @@ int ASN1_digest(i2d_of_void *i2d, const EVP_MD *type, char *data,
     p = str;
     i2d(data, &p);
 
-    if (!EVP_Digest(str, i, md, len, type, NULL))
+    if (!EVP_Digest(str, i, md, len, type, NULL)) {
+        OPENSSL_free(str);
         return 0;
+    }
     OPENSSL_free(str);
     return (1);
 }
@@ -104,8 +106,10 @@ int ASN1_item_digest(const ASN1_ITEM *it, const EVP_MD *type, void *asn,
     if (!str)
         return (0);
 
-    if (!EVP_Digest(str, i, md, len, type, NULL))
+    if (!EVP_Digest(str, i, md, len, type, NULL)) {
+        OPENSSL_free(str);
         return 0;
+    }
     OPENSSL_free(str);
     return (1);
 }
diff --git a/crypto/asn1/tasn_dec.c b/crypto/asn1/tasn_dec.c
index d25402730b..d49a5d5792 100644
--- a/crypto/asn1/tasn_dec.c
+++ b/crypto/asn1/tasn_dec.c
@@ -673,6 +673,7 @@ static int asn1_template_noexp_d2i(ASN1_VALUE **val,
             }
             len -= p - q;
             if (!sk_ASN1_VALUE_push((STACK_OF(ASN1_VALUE) *)*val, skfield)) {
+                ASN1_item_ex_free(&skfield, ASN1_ITEM_ptr(tt->item));
                 ASN1err(ASN1_F_ASN1_TEMPLATE_NOEXP_D2I, ERR_R_MALLOC_FAILURE);
                 goto err;
             }
diff --git a/crypto/asn1/tasn_new.c b/crypto/asn1/tasn_new.c
index b0c73beeb5..54f459d1ed 100644
--- a/crypto/asn1/tasn_new.c
+++ b/crypto/asn1/tasn_new.c
@@ -158,7 +158,7 @@ static int asn1_item_ex_combine_new(ASN1_VALUE **pval, const ASN1_ITEM *it,
         }
         asn1_set_choice_selector(pval, -1, it);
         if (asn1_cb && !asn1_cb(ASN1_OP_NEW_POST, pval, it, NULL))
-            goto auxerr;
+            goto auxerr2;
         break;
 
     case ASN1_ITYPE_NDEF_SEQUENCE:
@@ -186,10 +186,10 @@ static int asn1_item_ex_combine_new(ASN1_VALUE **pval, const ASN1_ITEM *it,
         for (i = 0, tt = it->templates; i < it->tcount; tt++, i++) {
             pseqval = asn1_get_field_ptr(pval, tt);
             if (!ASN1_template_new(pseqval, tt))
-                goto memerr;
+                goto memerr2;
         }
         if (asn1_cb && !asn1_cb(ASN1_OP_NEW_POST, pval, it, NULL))
-            goto auxerr;
+            goto auxerr2;
         break;
     }
 #ifdef CRYPTO_MDEBUG
@@ -198,6 +198,8 @@ static int asn1_item_ex_combine_new(ASN1_VALUE **pval, const ASN1_ITEM *it,
 #endif
     return 1;
 
+ memerr2:
+    ASN1_item_ex_free(pval, it);
  memerr:
     ASN1err(ASN1_F_ASN1_ITEM_EX_COMBINE_NEW, ERR_R_MALLOC_FAILURE);
 #ifdef CRYPTO_MDEBUG
@@ -206,9 +208,10 @@ static int asn1_item_ex_combine_new(ASN1_VALUE **pval, const ASN1_ITEM *it,
 #endif
     return 0;
 
+ auxerr2:
+    ASN1_item_ex_free(pval, it);
  auxerr:
     ASN1err(ASN1_F_ASN1_ITEM_EX_COMBINE_NEW, ASN1_R_AUX_ERROR);
-    ASN1_item_ex_free(pval, it);
 #ifdef CRYPTO_MDEBUG
     if (it->sname)
         CRYPTO_pop_info();
diff --git a/crypto/asn1/x_name.c b/crypto/asn1/x_name.c
index 26378fdb2a..1fb7ad1cbf 100644
--- a/crypto/asn1/x_name.c
+++ b/crypto/asn1/x_name.c
@@ -178,6 +178,16 @@ static void x509_name_ex_free(ASN1_VALUE **pval, const ASN1_ITEM *it)
     *pval = NULL;
 }
 
+static void local_sk_X509_NAME_ENTRY_free(STACK_OF(X509_NAME_ENTRY) *ne)
+{
+    sk_X509_NAME_ENTRY_free(ne);
+}
+
+static void local_sk_X509_NAME_ENTRY_pop_free(STACK_OF(X509_NAME_ENTRY) *ne)
+{
+    sk_X509_NAME_ENTRY_pop_free(ne, X509_NAME_ENTRY_free);
+}
+
 static int x509_name_ex_d2i(ASN1_VALUE **val,
                             const unsigned char **in, long len,
                             const ASN1_ITEM *it, int tag, int aclass,
@@ -228,13 +238,14 @@ static int x509_name_ex_d2i(ASN1_VALUE **val,
             entry->set = i;
             if (!sk_X509_NAME_ENTRY_push(nm.x->entries, entry))
                 goto err;
+            sk_X509_NAME_ENTRY_set(entries, j, NULL);
         }
-        sk_X509_NAME_ENTRY_free(entries);
     }
-    sk_STACK_OF_X509_NAME_ENTRY_free(intname.s);
     ret = x509_name_canon(nm.x);
     if (!ret)
         goto err;
+    sk_STACK_OF_X509_NAME_ENTRY_pop_free(intname.s,
+                                         local_sk_X509_NAME_ENTRY_free);
     nm.x->modified = 0;
     *val = nm.a;
     *in = p;
@@ -242,6 +253,8 @@ static int x509_name_ex_d2i(ASN1_VALUE **val,
  err:
     if (nm.x != NULL)
         X509_NAME_free(nm.x);
+    sk_STACK_OF_X509_NAME_ENTRY_pop_free(intname.s,
+                                         local_sk_X509_NAME_ENTRY_pop_free);
     ASN1err(ASN1_F_X509_NAME_EX_D2I, ERR_R_NESTED_ASN1_ERROR);
     return 0;
 }
@@ -267,16 +280,6 @@ static int x509_name_ex_i2d(ASN1_VALUE **val, unsigned char **out,
     return ret;
 }
 
-static void local_sk_X509_NAME_ENTRY_free(STACK_OF(X509_NAME_ENTRY) *ne)
-{
-    sk_X509_NAME_ENTRY_free(ne);
-}
-
-static void local_sk_X509_NAME_ENTRY_pop_free(STACK_OF(X509_NAME_ENTRY) *ne)
-{
-    sk_X509_NAME_ENTRY_pop_free(ne, X509_NAME_ENTRY_free);
-}
-
 static int x509_name_encode(X509_NAME *a)
 {
     union {
@@ -299,8 +302,10 @@ static int x509_name_encode(X509_NAME *a)
             entries = sk_X509_NAME_ENTRY_new_null();
             if (!entries)
                 goto memerr;
-            if (!sk_STACK_OF_X509_NAME_ENTRY_push(intname.s, entries))
+            if (!sk_STACK_OF_X509_NAME_ENTRY_push(intname.s, entries)) {
+                sk_X509_NAME_ENTRY_free(entries);
                 goto memerr;
+            }
             set = entry->set;
         }
         if (!sk_X509_NAME_ENTRY_push(entries, entry))
@@ -370,8 +375,10 @@ static int x509_name_canon(X509_NAME *a)
             entries = sk_X509_NAME_ENTRY_new_null();
             if (!entries)
                 goto err;
-            if (!sk_STACK_OF_X509_NAME_ENTRY_push(intname, entries))
+            if (!sk_STACK_OF_X509_NAME_ENTRY_push(intname, entries)) {
+                sk_X509_NAME_ENTRY_free(entries);
                 goto err;
+            }
             set = entry->set;
         }
         tmpentry = X509_NAME_ENTRY_new();
diff --git a/crypto/evp/pmeth_lib.c b/crypto/evp/pmeth_lib.c
index d066862904..b7b7bdcd02 100644
--- a/crypto/evp/pmeth_lib.c
+++ b/crypto/evp/pmeth_lib.c
@@ -188,6 +188,7 @@ static EVP_PKEY_CTX *int_ctx_new(EVP_PKEY *pkey, ENGINE *e, int id)
 
     if (pmeth->init) {
         if (pmeth->init(ret) <= 0) {
+            ret->pmeth = NULL;
             EVP_PKEY_CTX_free(ret);
             return NULL;
         }
@@ -315,6 +316,7 @@ EVP_PKEY_CTX *EVP_PKEY_CTX_dup(EVP_PKEY_CTX *pctx)
     if (pctx->pmeth->copy(rctx, pctx) > 0)
         return rctx;
 
+    rctx->pmeth = NULL;
     EVP_PKEY_CTX_free(rctx);
     return NULL;
 
diff --git a/crypto/ex_data.c b/crypto/ex_data.c
index f96a51781a..d947f3edac 100644
--- a/crypto/ex_data.c
+++ b/crypto/ex_data.c
@@ -331,7 +331,11 @@ static EX_CLASS_ITEM *def_get_class(int class_index)
                  * from the insert will be NULL
                  */
                 (void)lh_EX_CLASS_ITEM_insert(ex_data, gen);
-                p = gen;
+                p = lh_EX_CLASS_ITEM_retrieve(ex_data, &d);
+                if (p != gen) {
+                    sk_CRYPTO_EX_DATA_FUNCS_free(gen->meth);
+                    OPENSSL_free(gen);
+                }
             }
         }
     }
@@ -499,11 +503,12 @@ static void int_free_ex_data(int class_index, void *obj, CRYPTO_EX_DATA *ad)
     int mx, i;
     EX_CLASS_ITEM *item;
     void *ptr;
+    CRYPTO_EX_DATA_FUNCS *f;
     CRYPTO_EX_DATA_FUNCS **storage = NULL;
     if (ex_data == NULL)
-        return;
+        goto err;
     if ((item = def_get_class(class_index)) == NULL)
-        return;
+        goto err;
     CRYPTO_r_lock(CRYPTO_LOCK_EX_DATA);
     mx = sk_CRYPTO_EX_DATA_FUNCS_num(item->meth);
     if (mx > 0) {
@@ -515,23 +520,23 @@ static void int_free_ex_data(int class_index, void *obj, CRYPTO_EX_DATA *ad)
     }
  skip:
     CRYPTO_r_unlock(CRYPTO_LOCK_EX_DATA);
-    if ((mx > 0) && !storage) {
-        CRYPTOerr(CRYPTO_F_INT_FREE_EX_DATA, ERR_R_MALLOC_FAILURE);
-        return;
-    }
     for (i = 0; i < mx; i++) {
-        if (storage[i] && storage[i]->free_func) {
+        if (storage != NULL)
+            f = storage[i];
+        else {
+            CRYPTO_r_lock(CRYPTO_LOCK_EX_DATA);
+            f = sk_CRYPTO_EX_DATA_FUNCS_value(item->meth, i);
+            CRYPTO_r_unlock(CRYPTO_LOCK_EX_DATA);
+        }
+        if (f != NULL && f->free_func != NULL) {
             ptr = CRYPTO_get_ex_data(ad, i);
-            storage[i]->free_func(obj, ptr, ad, i,
-                                  storage[i]->argl, storage[i]->argp);
+            f->free_func(obj, ptr, ad, i, f->argl, f->argp);
         }
     }
-    if (storage)
-        OPENSSL_free(storage);
-    if (ad->sk) {
-        sk_void_free(ad->sk);
-        ad->sk = NULL;
-    }
+    OPENSSL_free(storage);
+ err:
+    sk_void_free(ad->sk);
+    ad->sk = NULL;
 }
 
 /********************************************************************/
diff --git a/crypto/hmac/hm_pmeth.c b/crypto/hmac/hm_pmeth.c
index 0ffff79cc4..0a59a01cf0 100644
--- a/crypto/hmac/hm_pmeth.c
+++ b/crypto/hmac/hm_pmeth.c
@@ -99,15 +99,18 @@ static int pkey_hmac_copy(EVP_PKEY_CTX *dst, EVP_PKEY_CTX *src)
     sctx = src->data;
     dctx = dst->data;
     dctx->md = sctx->md;
-    HMAC_CTX_init(&dctx->ctx);
     if (!HMAC_CTX_copy(&dctx->ctx, &sctx->ctx))
-        return 0;
-    if (sctx->ktmp.data) {
+        goto err;
+    if (sctx->ktmp.data != NULL) {
         if (!ASN1_OCTET_STRING_set(&dctx->ktmp,
                                    sctx->ktmp.data, sctx->ktmp.length))
-            return 0;
+            goto err;
     }
     return 1;
+ err:
+    HMAC_CTX_cleanup(&dctx->ctx);
+    OPENSSL_free(dctx);
+    return 0;
 }
 
 static void pkey_hmac_cleanup(EVP_PKEY_CTX *ctx)
diff --git a/crypto/rand/md_rand.c b/crypto/rand/md_rand.c
index bd76e23e3d..29e465b075 100644
--- a/crypto/rand/md_rand.c
+++ b/crypto/rand/md_rand.c
@@ -266,17 +266,21 @@ static void ssleay_rand_add(const void *buf, int num, double add)
         j = (num - i);
         j = (j > MD_DIGEST_LENGTH) ? MD_DIGEST_LENGTH : j;
 
-        MD_Init(&m);
-        MD_Update(&m, local_md, MD_DIGEST_LENGTH);
+        if (!MD_Init(&m) ||
+            !MD_Update(&m, local_md, MD_DIGEST_LENGTH))
+            goto err;
         k = (st_idx + j) - STATE_SIZE;
         if (k > 0) {
-            MD_Update(&m, &(state[st_idx]), j - k);
-            MD_Update(&m, &(state[0]), k);
+            if (!MD_Update(&m, &(state[st_idx]), j - k) ||
+                !MD_Update(&m, &(state[0]), k))
+                goto err;
         } else
-            MD_Update(&m, &(state[st_idx]), j);
+            if (!MD_Update(&m, &(state[st_idx]), j))
+                goto err;
 
         /* DO NOT REMOVE THE FOLLOWING CALL TO MD_Update()! */
-        MD_Update(&m, buf, j);
+        if (!MD_Update(&m, buf, j))
+            goto err;
         /*
          * We know that line may cause programs such as purify and valgrind
          * to complain about use of uninitialized data.  The problem is not,
@@ -285,8 +289,9 @@ static void ssleay_rand_add(const void *buf, int num, double add)
          * insecure keys.
          */
 
-        MD_Update(&m, (unsigned char *)&(md_c[0]), sizeof(md_c));
-        MD_Final(&m, local_md);
+        if (!MD_Update(&m, (unsigned char *)&(md_c[0]), sizeof(md_c)) ||
+            !MD_Final(&m, local_md))
+            goto err;
         md_c[1]++;
 
         buf = (const char *)buf + j;
@@ -305,7 +310,6 @@ static void ssleay_rand_add(const void *buf, int num, double add)
                 st_idx = 0;
         }
     }
-    EVP_MD_CTX_cleanup(&m);
 
     if (!do_not_lock)
         CRYPTO_w_lock(CRYPTO_LOCK_RAND);
@@ -326,6 +330,9 @@ static void ssleay_rand_add(const void *buf, int num, double add)
 #if !defined(OPENSSL_THREADS) && !defined(OPENSSL_SYS_WIN32)
     assert(md_c[1] == md_count[1]);
 #endif
+
+ err:
+    EVP_MD_CTX_cleanup(&m);
 }
 
 static void ssleay_rand_seed(const void *buf, int num)
@@ -469,15 +476,18 @@ int ssleay_rand_bytes(unsigned char *buf, int num, int pseudo, int lock)
         /* num_ceil -= MD_DIGEST_LENGTH/2 */
         j = (num >= MD_DIGEST_LENGTH / 2) ? MD_DIGEST_LENGTH / 2 : num;
         num -= j;
-        MD_Init(&m);
+        if (!MD_Init(&m))
+           goto err;
 #ifndef GETPID_IS_MEANINGLESS
         if (curr_pid) {         /* just in the first iteration to save time */
-            MD_Update(&m, (unsigned char *)&curr_pid, sizeof curr_pid);
+            if (!MD_Update(&m, (unsigned char *)&curr_pid, sizeof curr_pid))
+                goto err;
             curr_pid = 0;
         }
 #endif
-        MD_Update(&m, local_md, MD_DIGEST_LENGTH);
-        MD_Update(&m, (unsigned char *)&(md_c[0]), sizeof(md_c));
+        if (!MD_Update(&m, local_md, MD_DIGEST_LENGTH) ||
+            !MD_Update(&m, (unsigned char *)&(md_c[0]), sizeof(md_c)))
+            goto err;
 
 #ifndef PURIFY                  /* purify complains */
         /*
@@ -487,16 +497,21 @@ int ssleay_rand_bytes(unsigned char *buf, int num, int pseudo, int lock)
          * builds it is not used: the removal of such a small source of
          * entropy has negligible impact on security.
          */
-        MD_Update(&m, buf, j);
+        if (!MD_Update(&m, buf, j))
+            goto err;
 #endif
 
         k = (st_idx + MD_DIGEST_LENGTH / 2) - st_num;
         if (k > 0) {
-            MD_Update(&m, &(state[st_idx]), MD_DIGEST_LENGTH / 2 - k);
-            MD_Update(&m, &(state[0]), k);
-        } else
-            MD_Update(&m, &(state[st_idx]), MD_DIGEST_LENGTH / 2);
-        MD_Final(&m, local_md);
+            if (!MD_Update(&m, &(state[st_idx]), MD_DIGEST_LENGTH / 2 - k) ||
+                !MD_Update(&m, &(state[0]), k))
+                goto err;
+        } else {
+            if (!MD_Update(&m, &(state[st_idx]), MD_DIGEST_LENGTH / 2))
+                goto err;
+        }
+        if (!MD_Final(&m, local_md))
+            goto err;
 
         for (i = 0; i < MD_DIGEST_LENGTH / 2; i++) {
             /* may compete with other threads */
@@ -508,13 +523,18 @@ int ssleay_rand_bytes(unsigned char *buf, int num, int pseudo, int lock)
         }
     }
 
-    MD_Init(&m);
-    MD_Update(&m, (unsigned char *)&(md_c[0]), sizeof(md_c));
-    MD_Update(&m, local_md, MD_DIGEST_LENGTH);
+    if (!MD_Init(&m) ||
+        !MD_Update(&m, (unsigned char *)&(md_c[0]), sizeof(md_c)) ||
+        !MD_Update(&m, local_md, MD_DIGEST_LENGTH))
+        goto err;
     if (lock)
         CRYPTO_w_lock(CRYPTO_LOCK_RAND);
-    MD_Update(&m, md, MD_DIGEST_LENGTH);
-    MD_Final(&m, md);
+    if (!MD_Update(&m, md, MD_DIGEST_LENGTH) ||
+        !MD_Final(&m, md)) {
+        if (lock)
+            CRYPTO_w_unlock(CRYPTO_LOCK_RAND);
+        goto err;
+    }
     if (lock)
         CRYPTO_w_unlock(CRYPTO_LOCK_RAND);
 
@@ -529,6 +549,10 @@ int ssleay_rand_bytes(unsigned char *buf, int num, int pseudo, int lock)
                            "http://www.openssl.org/support/faq.html");
         return (0);
     }
+
+ err:
+    EVP_MD_CTX_cleanup(&m);
+    return (0);
 }
 
 static int ssleay_rand_nopseudo_bytes(unsigned char *buf, int num)
diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c
index 50120a4d70..c0b6bddf9d 100644
--- a/crypto/x509/x509_lu.c
+++ b/crypto/x509/x509_lu.c
@@ -185,14 +185,16 @@ X509_STORE *X509_STORE_new(void)
 
     if ((ret = (X509_STORE *)OPENSSL_malloc(sizeof(X509_STORE))) == NULL)
         return NULL;
-    ret->objs = sk_X509_OBJECT_new(x509_object_cmp);
+    if ((ret->objs = sk_X509_OBJECT_new(x509_object_cmp)) == NULL)
+        goto err0;
     ret->cache = 1;
-    ret->get_cert_methods = sk_X509_LOOKUP_new_null();
+    if ((ret->get_cert_methods = sk_X509_LOOKUP_new_null()) == NULL)
+        goto err1;
     ret->verify = 0;
     ret->verify_cb = 0;
 
     if ((ret->param = X509_VERIFY_PARAM_new()) == NULL)
-        return NULL;
+        goto err2;
 
     ret->get_issuer = 0;
     ret->check_issued = 0;
@@ -204,14 +206,21 @@ X509_STORE *X509_STORE_new(void)
     ret->lookup_crls = 0;
     ret->cleanup = 0;
 
-    if (!CRYPTO_new_ex_data(CRYPTO_EX_INDEX_X509_STORE, ret, &ret->ex_data)) {
-        sk_X509_OBJECT_free(ret->objs);
-        OPENSSL_free(ret);
-        return NULL;
-    }
+    if (!CRYPTO_new_ex_data(CRYPTO_EX_INDEX_X509_STORE, ret, &ret->ex_data))
+       goto err3;
 
     ret->references = 1;
     return ret;
+
+ err3:
+    X509_VERIFY_PARAM_free(ret->param);
+ err2:
+    sk_X509_LOOKUP_free(ret->get_cert_methods);
+ err1:
+    sk_X509_OBJECT_free(ret->objs);
+ err0:
+    OPENSSL_free(ret);
+    return NULL;
 }
 
 static void cleanup(X509_OBJECT *a)