From 68efafc513788863339c199d22048ef275832094 Mon Sep 17 00:00:00 2001 From: FdaSilvaYY Date: Mon, 27 Jun 2016 21:57:58 +0200 Subject: [PATCH 1/1] Add checks on sk_TYPE_push() returned value Reviewed-by: Rich Salz Reviewed-by: Matt Caswell --- crypto/asn1/asn_mime.c | 24 +++++++++++++++++++----- crypto/ct/ct_log.c | 5 ++++- crypto/evp/evp_pbe.c | 5 ++++- crypto/objects/o_names.c | 12 +++++++++--- crypto/ocsp/ocsp_cl.c | 2 +- crypto/pkcs7/pk7_attr.c | 6 +++++- crypto/ui/ui_lib.c | 8 ++++++-- crypto/x509/x509_lu.c | 34 ++++++++++++++++++++++------------ 8 files changed, 70 insertions(+), 26 deletions(-) diff --git a/crypto/asn1/asn_mime.c b/crypto/asn1/asn_mime.c index a6b38935a2..a4527a160a 100644 --- a/crypto/asn1/asn_mime.c +++ b/crypto/asn1/asn_mime.c @@ -625,7 +625,7 @@ static STACK_OF(MIME_HEADER) *mime_parse_hdr(BIO *bio) char *p, *q, c; char *ntmp; char linebuf[MAX_SMLEN]; - MIME_HEADER *mhdr = NULL; + MIME_HEADER *mhdr = NULL, *new_hdr = NULL; STACK_OF(MIME_HEADER) *headers; int len, state, save_state = 0; @@ -662,8 +662,13 @@ static STACK_OF(MIME_HEADER) *mime_parse_hdr(BIO *bio) if (c == ';') { mime_debug("Found End Value\n"); *p = 0; - mhdr = mime_hdr_new(ntmp, strip_ends(q)); - sk_MIME_HEADER_push(headers, mhdr); + new_hdr = mime_hdr_new(ntmp, strip_ends(q)); + if (new_hdr == NULL) + goto err; + if (!sk_MIME_HEADER_push(headers, new_hdr)) + goto err; + mhdr = new_hdr; + new_hdr = NULL; ntmp = NULL; q = p + 1; state = MIME_NAME; @@ -714,8 +719,13 @@ static STACK_OF(MIME_HEADER) *mime_parse_hdr(BIO *bio) } if (state == MIME_TYPE) { - mhdr = mime_hdr_new(ntmp, strip_ends(q)); - sk_MIME_HEADER_push(headers, mhdr); + new_hdr = mime_hdr_new(ntmp, strip_ends(q)); + if (new_hdr == NULL) + goto err; + if (!sk_MIME_HEADER_push(headers, new_hdr)) + goto err; + mhdr = new_hdr; + new_hdr = NULL; } else if (state == MIME_VALUE) mime_hdr_addparam(mhdr, ntmp, strip_ends(q)); if (p == linebuf) @@ -724,6 +734,10 @@ static STACK_OF(MIME_HEADER) *mime_parse_hdr(BIO *bio) return headers; +err: + mime_hdr_free(new_hdr); + sk_MIME_HEADER_pop_free(headers, mime_hdr_free); + return NULL; } static char *strip_ends(char *name) diff --git a/crypto/ct/ct_log.c b/crypto/ct/ct_log.c index 6fc21b7269..7298f1bfd4 100644 --- a/crypto/ct/ct_log.c +++ b/crypto/ct/ct_log.c @@ -182,7 +182,10 @@ static int ctlog_store_load_log(const char *log_name, int log_name_len, return 1; } - sk_CTLOG_push(load_ctx->log_store->logs, ct_log); + if (!sk_CTLOG_push(load_ctx->log_store->logs, ct_log)) { + CTLOG_free(ct_log); + return -1; + } return 1; } diff --git a/crypto/evp/evp_pbe.c b/crypto/evp/evp_pbe.c index 623f447387..ce7aa2cfa1 100644 --- a/crypto/evp/evp_pbe.c +++ b/crypto/evp/evp_pbe.c @@ -173,7 +173,10 @@ int EVP_PBE_alg_add_type(int pbe_type, int pbe_nid, int cipher_nid, pbe_tmp->md_nid = md_nid; pbe_tmp->keygen = keygen; - sk_EVP_PBE_CTL_push(pbe_algs, pbe_tmp); + if (!sk_EVP_PBE_CTL_push(pbe_algs, pbe_tmp)) { + OPENSSL_free(pbe_tmp); + goto err; + } return 1; err: diff --git a/crypto/objects/o_names.c b/crypto/objects/o_names.c index c655a908dd..ed98df8c2f 100644 --- a/crypto/objects/o_names.c +++ b/crypto/objects/o_names.c @@ -76,8 +76,7 @@ int OBJ_NAME_new_index(unsigned long (*hash_func) (const char *), int (*cmp_func) (const char *, const char *), void (*free_func) (const char *, int, const char *)) { - int ret; - int i; + int ret, i, push; NAME_FUNCS *name_funcs; if (name_funcs_stack == NULL) { @@ -102,8 +101,15 @@ int OBJ_NAME_new_index(unsigned long (*hash_func) (const char *), name_funcs->hash_func = OPENSSL_LH_strhash; name_funcs->cmp_func = obj_strcmp; CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_DISABLE); - sk_NAME_FUNCS_push(name_funcs_stack, name_funcs); + + push = sk_NAME_FUNCS_push(name_funcs_stack, name_funcs); CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ENABLE); + + if (!push) { + OBJerr(OBJ_F_OBJ_NAME_NEW_INDEX, ERR_R_MALLOC_FAILURE); + OPENSSL_free(name_funcs); + return 0; + } } name_funcs = sk_NAME_FUNCS_value(name_funcs_stack, ret); if (hash_func != NULL) diff --git a/crypto/ocsp/ocsp_cl.c b/crypto/ocsp/ocsp_cl.c index 195d87c8be..33a30bdf1c 100644 --- a/crypto/ocsp/ocsp_cl.c +++ b/crypto/ocsp/ocsp_cl.c @@ -32,7 +32,7 @@ OCSP_ONEREQ *OCSP_request_add0_id(OCSP_REQUEST *req, OCSP_CERTID *cid) OCSP_ONEREQ *one = NULL; if ((one = OCSP_ONEREQ_new()) == NULL) - goto err; + return NULL; OCSP_CERTID_free(one->reqCert); one->reqCert = cid; if (req && !sk_OCSP_ONEREQ_push(req->tbsRequest.requestList, one)) diff --git a/crypto/pkcs7/pk7_attr.c b/crypto/pkcs7/pk7_attr.c index 5f7167048e..a96f41e1b9 100644 --- a/crypto/pkcs7/pk7_attr.c +++ b/crypto/pkcs7/pk7_attr.c @@ -74,7 +74,11 @@ int PKCS7_simple_smimecap(STACK_OF(X509_ALGOR) *sk, int nid, int arg) alg->parameter->value.integer = nbit; alg->parameter->type = V_ASN1_INTEGER; } - sk_X509_ALGOR_push(sk, alg); + if (!sk_X509_ALGOR_push(sk, alg)) { + PKCS7err(PKCS7_F_PKCS7_SIMPLE_SMIMECAP, ERR_R_MALLOC_FAILURE); + X509_ALGOR_free(alg); + return 0; + } return 1; } diff --git a/crypto/ui/ui_lib.c b/crypto/ui/ui_lib.c index 2940b2fd4a..8992ae77c2 100644 --- a/crypto/ui/ui_lib.c +++ b/crypto/ui/ui_lib.c @@ -127,8 +127,10 @@ static int general_allocate_string(UI *ui, const char *prompt, s->_.string_data.test_buf = test_buf; ret = sk_UI_STRING_push(ui->strings, s); /* sk_push() returns 0 on error. Let's adapt that */ - if (ret <= 0) + if (ret <= 0) { ret--; + free_string(s); + } } else free_string(s); } @@ -172,8 +174,10 @@ static int general_allocate_boolean(UI *ui, /* * sk_push() returns 0 on error. Let's adapt that */ - if (ret <= 0) + if (ret <= 0) { ret--; + free_string(s); + } } else free_string(s); } diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c index 0b5b5b9ed7..843f3519dc 100644 --- a/crypto/x509/x509_lu.c +++ b/crypto/x509/x509_lu.c @@ -301,7 +301,7 @@ int X509_STORE_CTX_get_by_subject(X509_STORE_CTX *vs, X509_LOOKUP_TYPE type, int X509_STORE_add_cert(X509_STORE *ctx, X509 *x) { X509_OBJECT *obj; - int ret = 1; + int ret = 1, added = 1; if (x == NULL) return 0; @@ -310,28 +310,33 @@ int X509_STORE_add_cert(X509_STORE *ctx, X509 *x) return 0; obj->type = X509_LU_X509; obj->data.x509 = x; + X509_OBJECT_up_ref_count(obj); CRYPTO_THREAD_write_lock(ctx->lock); - X509_OBJECT_up_ref_count(obj); - if (X509_OBJECT_retrieve_match(ctx->objs, obj)) { - X509_OBJECT_free(obj); X509err(X509_F_X509_STORE_ADD_CERT, X509_R_CERT_ALREADY_IN_HASH_TABLE); ret = 0; - } else - sk_X509_OBJECT_push(ctx->objs, obj); + } else { + added = sk_X509_OBJECT_push(ctx->objs, obj); + ret = added != 0; + } CRYPTO_THREAD_unlock(ctx->lock); + if (!ret) /* obj not pushed */ + X509_OBJECT_free(obj); + if (!added) /* on push failure */ + X509err(X509_F_X509_STORE_ADD_CERT, ERR_R_MALLOC_FAILURE); + return ret; } int X509_STORE_add_crl(X509_STORE *ctx, X509_CRL *x) { X509_OBJECT *obj; - int ret = 1; + int ret = 1, added = 1; if (x == NULL) return 0; @@ -340,20 +345,25 @@ int X509_STORE_add_crl(X509_STORE *ctx, X509_CRL *x) return 0; obj->type = X509_LU_CRL; obj->data.crl = x; + X509_OBJECT_up_ref_count(obj); CRYPTO_THREAD_write_lock(ctx->lock); - X509_OBJECT_up_ref_count(obj); - if (X509_OBJECT_retrieve_match(ctx->objs, obj)) { - X509_OBJECT_free(obj); X509err(X509_F_X509_STORE_ADD_CRL, X509_R_CERT_ALREADY_IN_HASH_TABLE); ret = 0; - } else - sk_X509_OBJECT_push(ctx->objs, obj); + } else { + added = sk_X509_OBJECT_push(ctx->objs, obj); + ret = added != 0; + } CRYPTO_THREAD_unlock(ctx->lock); + if (!ret) /* obj not pushed */ + X509_OBJECT_free(obj); + if (!added) /* on push failure */ + X509err(X509_F_X509_STORE_ADD_CRL, ERR_R_MALLOC_FAILURE); + return ret; } -- 2.34.1