From: Bernd Edlinger Date: Wed, 1 Feb 2017 17:29:47 +0000 (+0100) Subject: Combined patch against master branch for the following issues: X-Git-Tag: OpenSSL_1_1_1-pre1~2477 X-Git-Url: https://git.openssl.org/?p=openssl.git;a=commitdiff_plain;h=83b4049ab75e9da1815e9c854a9297bca3d4af6b Combined patch against master branch for the following issues: Fixed a memory leak in ASN1_digest and ASN1_item_digest. Reworked error handling in asn1_item_embed_new. Fixed error handling in int_ctx_new and EVP_PKEY_CTX_dup. Fixed a memory leak in CRYPTO_free_ex_data. Reworked error handing in x509_name_ex_d2i, x509_name_encode and x509_name_canon. Check for null pointer in tls_process_cert_verify. Fixes #2103 #2104 #2105 #2109 #2111 #2115 Reviewed-by: Rich Salz Reviewed-by: Richard Levitte (Merged from https://github.com/openssl/openssl/pull/2342) --- diff --git a/crypto/asn1/a_digest.c b/crypto/asn1/a_digest.c index 2f9b63b2a1..46bff0d88f 100644 --- a/crypto/asn1/a_digest.c +++ b/crypto/asn1/a_digest.c @@ -37,8 +37,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); } @@ -55,8 +57,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_new.c b/crypto/asn1/tasn_new.c index 897120d26c..e9b83773f1 100644 --- a/crypto/asn1/tasn_new.c +++ b/crypto/asn1/tasn_new.c @@ -100,7 +100,7 @@ int asn1_item_embed_new(ASN1_VALUE **pval, const ASN1_ITEM *it, int embed) } 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: @@ -125,15 +125,15 @@ int asn1_item_embed_new(ASN1_VALUE **pval, const ASN1_ITEM *it, int embed) } /* 0 : init. lock */ if (asn1_do_lock(pval, 0, it) < 0) - goto memerr; + goto memerr2; asn1_enc_init(pval, 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; } #ifndef OPENSSL_NO_CRYPTO_MDEBUG @@ -141,6 +141,8 @@ int asn1_item_embed_new(ASN1_VALUE **pval, const ASN1_ITEM *it, int embed) #endif return 1; + memerr2: + ASN1_item_ex_free(pval, it); memerr: ASN1err(ASN1_F_ASN1_ITEM_EMBED_NEW, ERR_R_MALLOC_FAILURE); #ifndef OPENSSL_NO_CRYPTO_MDEBUG @@ -148,9 +150,10 @@ int asn1_item_embed_new(ASN1_VALUE **pval, const ASN1_ITEM *it, int embed) #endif return 0; + auxerr2: + ASN1_item_ex_free(pval, it); auxerr: ASN1err(ASN1_F_ASN1_ITEM_EMBED_NEW, ASN1_R_AUX_ERROR); - ASN1_item_ex_free(pval, it); #ifndef OPENSSL_NO_CRYPTO_MDEBUG OPENSSL_mem_debug_pop(); #endif diff --git a/crypto/evp/pmeth_lib.c b/crypto/evp/pmeth_lib.c index 681e5e06c7..a204901a6a 100644 --- a/crypto/evp/pmeth_lib.c +++ b/crypto/evp/pmeth_lib.c @@ -142,6 +142,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; } @@ -267,6 +268,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 bb1af0b3b1..84b65551c6 100644 --- a/crypto/ex_data.c +++ b/crypto/ex_data.c @@ -307,11 +307,12 @@ void CRYPTO_free_ex_data(int class_index, void *obj, CRYPTO_EX_DATA *ad) int mx, i; EX_CALLBACKS *ip; void *ptr; + EX_CALLBACK *f; EX_CALLBACK *stack[10]; EX_CALLBACK **storage = NULL; if ((ip = get_and_lock(class_index)) == NULL) - return; + goto err; mx = sk_EX_CALLBACK_num(ip->meth); if (mx > 0) { @@ -325,20 +326,23 @@ void CRYPTO_free_ex_data(int class_index, void *obj, CRYPTO_EX_DATA *ad) } CRYPTO_THREAD_unlock(ex_data_lock); - if (mx > 0 && storage == NULL) { - CRYPTOerr(CRYPTO_F_CRYPTO_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_THREAD_write_lock(ex_data_lock); + f = sk_EX_CALLBACK_value(ip->meth, i); + CRYPTO_THREAD_unlock(ex_data_lock); + } + 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 != stack) OPENSSL_free(storage); + err: sk_void_free(ad->sk); ad->sk = NULL; } diff --git a/crypto/x509/x_name.c b/crypto/x509/x_name.c index c863c69213..97d735f8f2 100644 --- a/crypto/x509/x_name.c +++ b/crypto/x509/x_name.c @@ -125,9 +125,14 @@ static void x509_name_ex_free(ASN1_VALUE **pval, const ASN1_ITEM *it) *pval = NULL; } -static void name_entry_stack_free(STACK_OF(X509_NAME_ENTRY) *ents) +static void local_sk_X509_NAME_ENTRY_free(STACK_OF(X509_NAME_ENTRY) *ne) { - sk_X509_NAME_ENTRY_pop_free(ents, X509_NAME_ENTRY_free); + 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, @@ -180,33 +185,24 @@ 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); } } - /* - * All entries have now been pushed to nm->x.entries - * free up the stacks in intname.s but not the entries - * themselves. - */ - sk_STACK_OF_X509_NAME_ENTRY_pop_free(intname.s, sk_X509_NAME_ENTRY_free); - intname.s = NULL; 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; return ret; err: - /* If intname.s is not NULL only some entries exist in nm->x.entries: - * zero references in nm->x.entries list. Since all entries exist - * in intname.s we can free them all there - */ - if (intname.s != NULL) { - sk_X509_NAME_ENTRY_zero(nm.x->entries); - sk_STACK_OF_X509_NAME_ENTRY_pop_free(intname.s, name_entry_stack_free); - } - X509_NAME_free(nm.x); + 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; } @@ -232,16 +228,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 { @@ -264,8 +250,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)) @@ -333,8 +321,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/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c index 0f30c54271..16c765fabf 100644 --- a/ssl/statem/statem_lib.c +++ b/ssl/statem/statem_lib.c @@ -289,6 +289,11 @@ MSG_PROCESS_RETURN tls_process_cert_verify(SSL *s, PACKET *pkt) peer = s->session->peer; pkey = X509_get0_pubkey(peer); + if (pkey == NULL) { + al = SSL_AD_INTERNAL_ERROR; + goto f_err; + } + pktype = EVP_PKEY_id(pkey); type = X509_certificate_type(peer, pkey);