From ed912f3dea3bb312a373649b461b8cea27c8d6da Mon Sep 17 00:00:00 2001 From: Frederik Wedel-Heinen Date: Sat, 28 Dec 2024 10:13:48 +0100 Subject: [PATCH] Adds missing checks of return from XXX_up_ref(). --- apps/list.c | 60 +++++++++++-------- apps/s_client.c | 3 +- apps/s_server.c | 4 +- apps/speed.c | 10 ++-- crypto/cms/cms_env.c | 8 ++- crypto/cms/cms_kari.c | 4 +- crypto/cms/cms_sd.c | 11 +++- crypto/cms/cms_smime.c | 3 +- crypto/evp/asymcipher.c | 4 +- crypto/evp/digest.c | 32 +++++----- crypto/evp/evp_enc.c | 25 ++++---- crypto/evp/exchange.c | 9 ++- crypto/evp/kdf_meth.c | 19 +++--- crypto/evp/kem.c | 4 +- crypto/evp/mac_meth.c | 23 ++++--- crypto/evp/p_legacy.c | 10 ++-- crypto/evp/p_lib.c | 19 +++--- crypto/evp/pmeth_lib.c | 18 ++++-- crypto/evp/signature.c | 4 +- crypto/pkcs7/pk7_lib.c | 11 +++- crypto/store/store_lib.c | 15 +++-- crypto/store/store_meth.c | 5 +- crypto/ts/ts_rsp_sign.c | 9 ++- crypto/ts/ts_rsp_verify.c | 4 +- crypto/x509/pcy_tree.c | 4 +- crypto/x509/x509_cmp.c | 5 +- crypto/x509/x509_vfy.c | 23 +++++-- doc/internal/man3/evp_generic_fetch.pod | 15 +++-- .../ciphers/cipher_aes_siv_hw.c | 12 ++-- ssl/bio_ssl.c | 11 +++- ssl/s3_lib.c | 8 ++- ssl/ssl_cert.c | 44 +++++++------- ssl/ssl_lib.c | 52 ++++++++-------- ssl/ssl_rsa.c | 19 ++++-- ssl/ssl_rsa_legacy.c | 12 +++- ssl/ssl_sess.c | 28 +++++---- ssl/statem/statem_clnt.c | 6 +- test/bio_prefix_text.c | 4 +- test/cmp_client_test.c | 4 +- test/cmp_protect_test.c | 2 +- test/crltest.c | 36 ++++++++--- test/evp_extra_test.c | 7 ++- test/helpers/ssltestlib.c | 10 +++- test/quicapitest.c | 12 ++-- test/sslapitest.c | 32 ++++++---- 45 files changed, 414 insertions(+), 246 deletions(-) diff --git a/apps/list.c b/apps/list.c index c0b0a5a4c4322..5082c7e826a8d 100644 --- a/apps/list.c +++ b/apps/list.c @@ -101,8 +101,9 @@ static void collect_ciphers(EVP_CIPHER *cipher, void *stack) STACK_OF(EVP_CIPHER) *cipher_stack = stack; if (is_cipher_fetchable(cipher) - && sk_EVP_CIPHER_push(cipher_stack, cipher) > 0) - EVP_CIPHER_up_ref(cipher); + && EVP_CIPHER_up_ref(cipher) + && sk_EVP_CIPHER_push(cipher_stack, cipher) <= 0) + EVP_CIPHER_free(cipher); /* up-ref successful but push to stack failed */ } static void list_ciphers(const char *prefix) @@ -185,8 +186,9 @@ static void collect_digests(EVP_MD *digest, void *stack) STACK_OF(EVP_MD) *digest_stack = stack; if (is_digest_fetchable(digest) - && sk_EVP_MD_push(digest_stack, digest) > 0) - EVP_MD_up_ref(digest); + && EVP_MD_up_ref(digest) + && sk_EVP_MD_push(digest_stack, digest) <= 0) + EVP_MD_free(digest); /* up-ref successful but push to stack failed */ } static void list_digests(const char *prefix) @@ -317,8 +319,9 @@ static void collect_kdfs(EVP_KDF *kdf, void *stack) STACK_OF(EVP_KDF) *kdf_stack = stack; if (is_kdf_fetchable(kdf) - && sk_EVP_KDF_push(kdf_stack, kdf) > 0) - EVP_KDF_up_ref(kdf); + && EVP_KDF_up_ref(kdf) + && sk_EVP_KDF_push(kdf_stack, kdf) <= 0) + EVP_KDF_free(kdf); /* up-ref successful but push to stack failed */ } static void list_kdfs(void) @@ -387,8 +390,9 @@ static void collect_rands(EVP_RAND *rand, void *stack) STACK_OF(EVP_RAND) *rand_stack = stack; if (is_rand_fetchable(rand) - && sk_EVP_RAND_push(rand_stack, rand) > 0) - EVP_RAND_up_ref(rand); + && EVP_RAND_up_ref(rand) + && sk_EVP_RAND_push(rand_stack, rand) <= 0) + EVP_RAND_free(rand); /* up-ref successful but push to stack failed */ } static void list_random_generators(void) @@ -513,8 +517,9 @@ static void collect_encoders(OSSL_ENCODER *encoder, void *stack) STACK_OF(OSSL_ENCODER) *encoder_stack = stack; if (is_encoder_fetchable(encoder) - && sk_OSSL_ENCODER_push(encoder_stack, encoder) > 0) - OSSL_ENCODER_up_ref(encoder); + && OSSL_ENCODER_up_ref(encoder) + && sk_OSSL_ENCODER_push(encoder_stack, encoder) <= 0) + OSSL_ENCODER_free(encoder); /* up-ref successful but push to stack failed */ } static void list_encoders(void) @@ -578,8 +583,9 @@ static void collect_decoders(OSSL_DECODER *decoder, void *stack) STACK_OF(OSSL_DECODER) *decoder_stack = stack; if (is_decoder_fetchable(decoder) - && sk_OSSL_DECODER_push(decoder_stack, decoder) > 0) - OSSL_DECODER_up_ref(decoder); + && OSSL_DECODER_up_ref(decoder) + && sk_OSSL_DECODER_push(decoder_stack, decoder) <= 0) + OSSL_DECODER_free(decoder); /* up-ref successful but push to stack failed */ } static void list_decoders(void) @@ -640,8 +646,9 @@ static void collect_keymanagers(EVP_KEYMGMT *km, void *stack) STACK_OF(EVP_KEYMGMT) *km_stack = stack; if (is_keymgmt_fetchable(km) - && sk_EVP_KEYMGMT_push(km_stack, km) > 0) - EVP_KEYMGMT_up_ref(km); + && EVP_KEYMGMT_up_ref(km) + && sk_EVP_KEYMGMT_push(km_stack, km) <= 0) + EVP_KEYMGMT_free(km); /* up-ref successful but push to stack failed */ } static void list_keymanagers(void) @@ -703,8 +710,9 @@ static void collect_signatures(EVP_SIGNATURE *sig, void *stack) STACK_OF(EVP_SIGNATURE) *sig_stack = stack; if (is_signature_fetchable(sig) - && sk_EVP_SIGNATURE_push(sig_stack, sig) > 0) - EVP_SIGNATURE_up_ref(sig); + && EVP_SIGNATURE_up_ref(sig) + && sk_EVP_SIGNATURE_push(sig_stack, sig) <= 0) + EVP_SIGNATURE_free(sig); /* up-ref successful but push to stack failed */ } static void list_signatures(void) @@ -810,8 +818,9 @@ static void collect_kem(EVP_KEM *kem, void *stack) STACK_OF(EVP_KEM) *kem_stack = stack; if (is_kem_fetchable(kem) - && sk_EVP_KEM_push(kem_stack, kem) > 0) - EVP_KEM_up_ref(kem); + && EVP_KEM_up_ref(kem) + && sk_EVP_KEM_push(kem_stack, kem) <= 0) + EVP_KEM_free(kem); /* up-ref successful but push to stack failed */ } static void list_kems(void) @@ -869,8 +878,9 @@ static void collect_asymciph(EVP_ASYM_CIPHER *asym_cipher, void *stack) STACK_OF(EVP_ASYM_CIPHER) *asym_cipher_stack = stack; if (is_asym_cipher_fetchable(asym_cipher) - && sk_EVP_ASYM_CIPHER_push(asym_cipher_stack, asym_cipher) > 0) - EVP_ASYM_CIPHER_up_ref(asym_cipher); + && EVP_ASYM_CIPHER_up_ref(asym_cipher) + && sk_EVP_ASYM_CIPHER_push(asym_cipher_stack, asym_cipher) <= 0) + EVP_ASYM_CIPHER_free(asym_cipher); /* up-ref successful but push to stack failed */ } static void list_asymciphers(void) @@ -931,8 +941,9 @@ static void collect_kex(EVP_KEYEXCH *kex, void *stack) STACK_OF(EVP_KEYEXCH) *kex_stack = stack; if (is_keyexch_fetchable(kex) - && sk_EVP_KEYEXCH_push(kex_stack, kex) > 0) - EVP_KEYEXCH_up_ref(kex); + && EVP_KEYEXCH_up_ref(kex) + && sk_EVP_KEYEXCH_push(kex_stack, kex) <= 0) + EVP_KEYEXCH_free(kex); /* up-ref successful but push to stack failed */ } static void list_keyexchanges(void) @@ -1211,8 +1222,9 @@ static void collect_store_loaders(OSSL_STORE_LOADER *store, void *stack) { STACK_OF(OSSL_STORE_LOADER) *store_stack = stack; - if (sk_OSSL_STORE_LOADER_push(store_stack, store) > 0) - OSSL_STORE_LOADER_up_ref(store); + if (OSSL_STORE_LOADER_up_ref(store) + && sk_OSSL_STORE_LOADER_push(store_stack, store) <= 0) + OSSL_STORE_LOADER_free(store); /* up-ref successful but push to stack failed */ } static void list_store_loaders(void) diff --git a/apps/s_client.c b/apps/s_client.c index c922653ee773e..d98bf3bbe68f7 100644 --- a/apps/s_client.c +++ b/apps/s_client.c @@ -208,7 +208,8 @@ static int psk_use_session_cb(SSL *s, const EVP_MD *md, const SSL_CIPHER *cipher = NULL; if (psksess != NULL) { - SSL_SESSION_up_ref(psksess); + if (!SSL_SESSION_up_ref(psksess)) + goto err; usesess = psksess; } else { long key_len; diff --git a/apps/s_server.c b/apps/s_server.c index 888e8f62cf0a0..0d5dd397d8719 100644 --- a/apps/s_server.c +++ b/apps/s_server.c @@ -209,7 +209,9 @@ static int psk_find_session_cb(SSL *ssl, const unsigned char *identity, } if (psksess != NULL) { - SSL_SESSION_up_ref(psksess); + if (!SSL_SESSION_up_ref(psksess)) + return 0; + *sess = psksess; return 1; } diff --git a/apps/speed.c b/apps/speed.c index 561e64115875f..6a9fb23b7f0fd 100644 --- a/apps/speed.c +++ b/apps/speed.c @@ -1817,8 +1817,9 @@ static void collect_kem(EVP_KEM *kem, void *stack) STACK_OF(EVP_KEM) *kem_stack = stack; if (is_kem_fetchable(kem) - && sk_EVP_KEM_push(kem_stack, kem) > 0) { - EVP_KEM_up_ref(kem); + && EVP_KEM_up_ref(kem) + && sk_EVP_KEM_push(kem_stack, kem) <= 0) { + EVP_KEM_free(kem); /* up-ref successful but push to stack failed */ } } @@ -1849,8 +1850,9 @@ static void collect_signatures(EVP_SIGNATURE *sig, void *stack) STACK_OF(EVP_SIGNATURE) *sig_stack = stack; if (is_signature_fetchable(sig) - && sk_EVP_SIGNATURE_push(sig_stack, sig) > 0) - EVP_SIGNATURE_up_ref(sig); + && EVP_SIGNATURE_up_ref(sig) + && sk_EVP_SIGNATURE_push(sig_stack, sig) <= 0) + EVP_SIGNATURE_free(sig); /* up-ref successful but push to stack failed */ } static int sig_locate(const char *algo, unsigned int *idx) diff --git a/crypto/cms/cms_env.c b/crypto/cms/cms_env.c index 71059edc9ad02..8423ca3f150d6 100644 --- a/crypto/cms/cms_env.c +++ b/crypto/cms/cms_env.c @@ -357,8 +357,12 @@ static int cms_RecipientInfo_ktri_init(CMS_RecipientInfo *ri, X509 *recip, if (!ossl_cms_set1_SignerIdentifier(ktri->rid, recip, idtype, ctx)) return 0; - X509_up_ref(recip); - EVP_PKEY_up_ref(pk); + if (!X509_up_ref(recip)) + return 0; + if (!EVP_PKEY_up_ref(pk)) { + X509_free(recip); + return 0; + } ktri->pkey = pk; ktri->recip = recip; diff --git a/crypto/cms/cms_kari.c b/crypto/cms/cms_kari.c index a2f422a78d8b5..0ab5c21e33696 100644 --- a/crypto/cms/cms_kari.c +++ b/crypto/cms/cms_kari.c @@ -405,7 +405,9 @@ int ossl_cms_RecipientInfo_kari_init(CMS_RecipientInfo *ri, X509 *recip, return 0; } - EVP_PKEY_up_ref(recipPubKey); + if (!EVP_PKEY_up_ref(recipPubKey)) + return 0; + rek->pkey = recipPubKey; return 1; } diff --git a/crypto/cms/cms_sd.c b/crypto/cms/cms_sd.c index aa83c7eaf880c..04b21455a90ec 100644 --- a/crypto/cms/cms_sd.c +++ b/crypto/cms/cms_sd.c @@ -356,8 +356,12 @@ CMS_SignerInfo *CMS_add1_signer(CMS_ContentInfo *cms, /* Call for side-effect of computing hash and caching extensions */ X509_check_purpose(signer, -1, -1); - X509_up_ref(signer); - EVP_PKEY_up_ref(pk); + if (!X509_up_ref(signer)) + goto err; + if (!EVP_PKEY_up_ref(pk)) { + X509_free(signer); + goto err; + } si->cms_ctx = ctx; si->pkey = pk; @@ -633,7 +637,8 @@ STACK_OF(X509) *CMS_get0_signers(CMS_ContentInfo *cms) void CMS_SignerInfo_set1_signer_cert(CMS_SignerInfo *si, X509 *signer) { if (signer != NULL) { - X509_up_ref(signer); + if (!X509_up_ref(signer)) + return; EVP_PKEY_free(si->pkey); si->pkey = X509_get_pubkey(signer); } diff --git a/crypto/cms/cms_smime.c b/crypto/cms/cms_smime.c index 27abae7461bf5..bd6f96cf94fc2 100644 --- a/crypto/cms/cms_smime.c +++ b/crypto/cms/cms_smime.c @@ -753,7 +753,8 @@ int CMS_decrypt_set1_pkey_and_peer(CMS_ContentInfo *cms, EVP_PKEY *pk, } /* If we have a cert, try matching RecipientInfo, else try them all */ else if (cert == NULL || !CMS_RecipientInfo_ktri_cert_cmp(ri, cert)) { - EVP_PKEY_up_ref(pk); + if (!EVP_PKEY_up_ref(pk)) + return 0; CMS_RecipientInfo_set0_pkey(ri, pk); r = CMS_RecipientInfo_decrypt(cms, ri); CMS_RecipientInfo_set0_pkey(ri, NULL); diff --git a/crypto/evp/asymcipher.c b/crypto/evp/asymcipher.c index d22ab2a01a16d..039f6be8cce84 100644 --- a/crypto/evp/asymcipher.c +++ b/crypto/evp/asymcipher.c @@ -324,12 +324,12 @@ static EVP_ASYM_CIPHER *evp_asym_cipher_new(OSSL_PROVIDER *prov) if (cipher == NULL) return NULL; - if (!CRYPTO_NEW_REF(&cipher->refcnt, 1)) { + if (!CRYPTO_NEW_REF(&cipher->refcnt, 1) + || !ossl_provider_up_ref(prov)) { OPENSSL_free(cipher); return NULL; } cipher->prov = prov; - ossl_provider_up_ref(prov); return cipher; } diff --git a/crypto/evp/digest.c b/crypto/evp/digest.c index 6aa175fb3f4be..7b32f7da33b2b 100644 --- a/crypto/evp/digest.c +++ b/crypto/evp/digest.c @@ -630,6 +630,10 @@ int EVP_MD_CTX_copy_ex(EVP_MD_CTX *out, const EVP_MD_CTX *in) } else { evp_md_ctx_reset_ex(out, 1); digest_change = (out->fetched_digest != in->fetched_digest); + + if (digest_change && in->fetched_digest != NULL + && !EVP_MD_up_ref(in->fetched_digest)) + return 0; if (digest_change && out->fetched_digest != NULL) EVP_MD_free(out->fetched_digest); *out = *in; @@ -637,9 +641,6 @@ int EVP_MD_CTX_copy_ex(EVP_MD_CTX *out, const EVP_MD_CTX *in) out->pctx = NULL; out->algctx = NULL; - if (digest_change && in->fetched_digest != NULL) - EVP_MD_up_ref(in->fetched_digest); - if (in->algctx != NULL) { out->algctx = in->digest->dupctx(in->algctx); if (out->algctx == NULL) { @@ -1030,16 +1031,14 @@ static void *evp_md_from_algorithm(int name_id, if (!evp_names_do_all(prov, name_id, set_legacy_nid, &md->type) || md->type == -1) { ERR_raise(ERR_LIB_EVP, ERR_R_INTERNAL_ERROR); - EVP_MD_free(md); - return NULL; + goto err; } #endif md->name_id = name_id; - if ((md->type_name = ossl_algorithm_get1_first_name(algodef)) == NULL) { - EVP_MD_free(md); - return NULL; - } + if ((md->type_name = ossl_algorithm_get1_first_name(algodef)) == NULL) + goto err; + md->description = algodef->algorithm_description; for (; fns->function_id != 0; fns++) { @@ -1130,21 +1129,24 @@ static void *evp_md_from_algorithm(int name_id, * The "digest" function can standalone. We at least need one way to * generate digests. */ - EVP_MD_free(md); ERR_raise(ERR_LIB_EVP, EVP_R_INVALID_PROVIDER_FUNCTIONS); - return NULL; + goto err; } + if (prov != NULL && !ossl_provider_up_ref(prov)) + goto err; + md->prov = prov; - if (prov != NULL) - ossl_provider_up_ref(prov); if (!evp_md_cache_constants(md)) { - EVP_MD_free(md); ERR_raise(ERR_LIB_EVP, EVP_R_CACHE_CONSTANTS_FAILED); - md = NULL; + goto err; } return md; + +err: + EVP_MD_free(md); + return NULL; } static int evp_md_up_ref(void *md) diff --git a/crypto/evp/evp_enc.c b/crypto/evp/evp_enc.c index 17320191340c2..a23123492ae06 100644 --- a/crypto/evp/evp_enc.c +++ b/crypto/evp/evp_enc.c @@ -1719,16 +1719,14 @@ static void *evp_cipher_from_algorithm(const int name_id, if (!evp_names_do_all(prov, name_id, set_legacy_nid, &cipher->nid) || cipher->nid == -1) { ERR_raise(ERR_LIB_EVP, ERR_R_INTERNAL_ERROR); - EVP_CIPHER_free(cipher); - return NULL; + goto err; } #endif cipher->name_id = name_id; - if ((cipher->type_name = ossl_algorithm_get1_first_name(algodef)) == NULL) { - EVP_CIPHER_free(cipher); - return NULL; - } + if ((cipher->type_name = ossl_algorithm_get1_first_name(algodef)) == NULL) + goto err; + cipher->description = algodef->algorithm_description; for (; fns->function_id != 0; fns++) { @@ -1848,21 +1846,24 @@ static void *evp_cipher_from_algorithm(const int name_id, * functions, or a single "cipher" function. In all cases we need both * the "newctx" and "freectx" functions. */ - EVP_CIPHER_free(cipher); ERR_raise(ERR_LIB_EVP, EVP_R_INVALID_PROVIDER_FUNCTIONS); - return NULL; + goto err; } + if (prov != NULL && !ossl_provider_up_ref(prov)) + goto err; + cipher->prov = prov; - if (prov != NULL) - ossl_provider_up_ref(prov); if (!evp_cipher_cache_constants(cipher)) { - EVP_CIPHER_free(cipher); ERR_raise(ERR_LIB_EVP, EVP_R_CACHE_CONSTANTS_FAILED); - cipher = NULL; + goto err; } return cipher; + +err: + EVP_CIPHER_free(cipher); + return NULL; } static int evp_cipher_up_ref(void *cipher) diff --git a/crypto/evp/exchange.c b/crypto/evp/exchange.c index d9eed1cea5be2..2c6110c9a77f9 100644 --- a/crypto/evp/exchange.c +++ b/crypto/evp/exchange.c @@ -25,12 +25,12 @@ static EVP_KEYEXCH *evp_keyexch_new(OSSL_PROVIDER *prov) if (exchange == NULL) return NULL; - if (!CRYPTO_NEW_REF(&exchange->refcnt, 1)) { + if (!CRYPTO_NEW_REF(&exchange->refcnt, 1) + || !ossl_provider_up_ref(prov)) { OPENSSL_free(exchange); return NULL; } exchange->prov = prov; - ossl_provider_up_ref(prov); return exchange; } @@ -483,17 +483,20 @@ int EVP_PKEY_derive_set_peer_ex(EVP_PKEY_CTX *ctx, EVP_PKEY *peer, return -1; } + if (!EVP_PKEY_up_ref(peer)) + return -1; + EVP_PKEY_free(ctx->peerkey); ctx->peerkey = peer; ret = ctx->pmeth->ctrl(ctx, EVP_PKEY_CTRL_PEER_KEY, 1, peer); if (ret <= 0) { + EVP_PKEY_free(ctx->peerkey); ctx->peerkey = NULL; return ret; } - EVP_PKEY_up_ref(peer); return 1; #endif } diff --git a/crypto/evp/kdf_meth.c b/crypto/evp/kdf_meth.c index 5ee36b2b4213e..ba80d5c874ae7 100644 --- a/crypto/evp/kdf_meth.c +++ b/crypto/evp/kdf_meth.c @@ -68,10 +68,9 @@ static void *evp_kdf_from_algorithm(int name_id, return NULL; } kdf->name_id = name_id; - if ((kdf->type_name = ossl_algorithm_get1_first_name(algodef)) == NULL) { - evp_kdf_free(kdf); - return NULL; - } + if ((kdf->type_name = ossl_algorithm_get1_first_name(algodef)) == NULL) + goto err; + kdf->description = algodef->algorithm_description; for (; fns->function_id != 0; fns++) { @@ -145,15 +144,19 @@ static void *evp_kdf_from_algorithm(int name_id, * a derive function, and a complete set of context management * functions. */ - evp_kdf_free(kdf); ERR_raise(ERR_LIB_EVP, EVP_R_INVALID_PROVIDER_FUNCTIONS); - return NULL; + goto err; } + if (prov != NULL && !ossl_provider_up_ref(prov)) + goto err; + kdf->prov = prov; - if (prov != NULL) - ossl_provider_up_ref(prov); return kdf; + +err: + evp_kdf_free(kdf); + return NULL; } EVP_KDF *EVP_KDF_fetch(OSSL_LIB_CTX *libctx, const char *algorithm, diff --git a/crypto/evp/kem.c b/crypto/evp/kem.c index f96012ccf01ed..14eb04d08fa82 100644 --- a/crypto/evp/kem.c +++ b/crypto/evp/kem.c @@ -278,12 +278,12 @@ static EVP_KEM *evp_kem_new(OSSL_PROVIDER *prov) if (kem == NULL) return NULL; - if (!CRYPTO_NEW_REF(&kem->refcnt, 1)) { + if (!CRYPTO_NEW_REF(&kem->refcnt, 1) + || !ossl_provider_up_ref(prov)) { OPENSSL_free(kem); return NULL; } kem->prov = prov; - ossl_provider_up_ref(prov); return kem; } diff --git a/crypto/evp/mac_meth.c b/crypto/evp/mac_meth.c index a3e7a0220850d..8ac73484cca86 100644 --- a/crypto/evp/mac_meth.c +++ b/crypto/evp/mac_meth.c @@ -64,13 +64,13 @@ static void *evp_mac_from_algorithm(int name_id, if ((mac = evp_mac_new()) == NULL) { ERR_raise(ERR_LIB_EVP, ERR_R_EVP_LIB); - return NULL; + goto err; } mac->name_id = name_id; - if ((mac->type_name = ossl_algorithm_get1_first_name(algodef)) == NULL) { - evp_mac_free(mac); - return NULL; - } + + if ((mac->type_name = ossl_algorithm_get1_first_name(algodef)) == NULL) + goto err; + mac->description = algodef->algorithm_description; for (; fns->function_id != 0; fns++) { @@ -152,15 +152,20 @@ static void *evp_mac_from_algorithm(int name_id, * a complete set of "mac" functions, and a complete set of context * management functions, as well as the size function. */ - evp_mac_free(mac); ERR_raise(ERR_LIB_EVP, EVP_R_INVALID_PROVIDER_FUNCTIONS); - return NULL; + goto err; } + + if (prov != NULL && !ossl_provider_up_ref(prov)) + goto err; + mac->prov = prov; - if (prov != NULL) - ossl_provider_up_ref(prov); return mac; + +err: + evp_mac_free(mac); + return NULL; } EVP_MAC *EVP_MAC_fetch(OSSL_LIB_CTX *libctx, const char *algorithm, diff --git a/crypto/evp/p_legacy.c b/crypto/evp/p_legacy.c index 6c65e7e1947a5..0773e2af3be40 100644 --- a/crypto/evp/p_legacy.c +++ b/crypto/evp/p_legacy.c @@ -26,8 +26,9 @@ int EVP_PKEY_set1_RSA(EVP_PKEY *pkey, RSA *key) { int ret = EVP_PKEY_assign_RSA(pkey, key); - if (ret) - RSA_up_ref(key); + if (ret && !RSA_up_ref(key)) + return 0; + return ret; } @@ -49,8 +50,9 @@ RSA *EVP_PKEY_get1_RSA(EVP_PKEY *pkey) { RSA *ret = evp_pkey_get0_RSA_int(pkey); - if (ret != NULL) - RSA_up_ref(ret); + if (ret != NULL && !RSA_up_ref(ret)) + ret = NULL; + return ret; } diff --git a/crypto/evp/p_lib.c b/crypto/evp/p_lib.c index 2eb142fa7660e..d84509b4b75e2 100644 --- a/crypto/evp/p_lib.c +++ b/crypto/evp/p_lib.c @@ -867,16 +867,17 @@ const DSA *EVP_PKEY_get0_DSA(const EVP_PKEY *pkey) int EVP_PKEY_set1_DSA(EVP_PKEY *pkey, DSA *key) { int ret = EVP_PKEY_assign_DSA(pkey, key); - if (ret) - DSA_up_ref(key); + if (ret && !DSA_up_ref(key)) + return 0; return ret; } DSA *EVP_PKEY_get1_DSA(EVP_PKEY *pkey) { DSA *ret = evp_pkey_get0_DSA_int(pkey); - if (ret != NULL) - DSA_up_ref(ret); + if (ret != NULL && !DSA_up_ref(ret)) + return NULL; + return ret; } # endif /* OPENSSL_NO_DSA */ @@ -944,8 +945,9 @@ int EVP_PKEY_set1_DH(EVP_PKEY *pkey, DH *dhkey) ret = EVP_PKEY_assign(pkey, type, dhkey); - if (ret) - DH_up_ref(dhkey); + if (ret && !DH_up_ref(dhkey)) + return 0; + return ret; } @@ -967,8 +969,9 @@ DH *EVP_PKEY_get1_DH(EVP_PKEY *pkey) { DH *ret = evp_pkey_get0_DH_int(pkey); - if (ret != NULL) - DH_up_ref(ret); + if (ret != NULL && !DH_up_ref(ret)) + ret = NULL; + return ret; } # endif diff --git a/crypto/evp/pmeth_lib.c b/crypto/evp/pmeth_lib.c index eb8c37eaf6306..dc3c649ee242f 100644 --- a/crypto/evp/pmeth_lib.c +++ b/crypto/evp/pmeth_lib.c @@ -321,9 +321,13 @@ static EVP_PKEY_CTX *int_ctx_new(OSSL_LIB_CTX *libctx, ret->engine = e; ret->pmeth = pmeth; ret->operation = EVP_PKEY_OP_UNDEFINED; + + if (pkey != NULL && !EVP_PKEY_up_ref(pkey)) { + EVP_PKEY_CTX_free(ret); + return NULL; + } + ret->pkey = pkey; - if (pkey != NULL) - EVP_PKEY_up_ref(pkey); if (pmeth != NULL && pmeth->init != NULL) { if (pmeth->init(ret) <= 0) { @@ -461,8 +465,9 @@ EVP_PKEY_CTX *EVP_PKEY_CTX_dup(const EVP_PKEY_CTX *pctx) if (rctx == NULL) return NULL; - if (pctx->pkey != NULL) - EVP_PKEY_up_ref(pctx->pkey); + if (pctx->pkey != NULL && !EVP_PKEY_up_ref(pctx->pkey)) + return NULL; + rctx->pkey = pctx->pkey; rctx->operation = pctx->operation; rctx->libctx = pctx->libctx; @@ -569,8 +574,9 @@ EVP_PKEY_CTX *EVP_PKEY_CTX_dup(const EVP_PKEY_CTX *pctx) rctx->engine = pctx->engine; # endif - if (pctx->peerkey != NULL) - EVP_PKEY_up_ref(pctx->peerkey); + if (pctx->peerkey != NULL && !EVP_PKEY_up_ref(pctx->peerkey)) + goto err; + rctx->peerkey = pctx->peerkey; if (pctx->pmeth == NULL) { diff --git a/crypto/evp/signature.c b/crypto/evp/signature.c index 7d619edfae317..adbf7c7d7edbe 100644 --- a/crypto/evp/signature.c +++ b/crypto/evp/signature.c @@ -27,13 +27,13 @@ static EVP_SIGNATURE *evp_signature_new(OSSL_PROVIDER *prov) if (signature == NULL) return NULL; - if (!CRYPTO_NEW_REF(&signature->refcnt, 1)) { + if (!CRYPTO_NEW_REF(&signature->refcnt, 1) + || !ossl_provider_up_ref(prov)) { OPENSSL_free(signature); return NULL; } signature->prov = prov; - ossl_provider_up_ref(prov); return signature; } diff --git a/crypto/pkcs7/pk7_lib.c b/crypto/pkcs7/pk7_lib.c index d7c5f1afbe4d3..f30478979554c 100644 --- a/crypto/pkcs7/pk7_lib.c +++ b/crypto/pkcs7/pk7_lib.c @@ -299,7 +299,8 @@ int PKCS7_add_crl(PKCS7 *p7, X509_CRL *crl) return 0; } - X509_CRL_up_ref(crl); + if (!X509_CRL_up_ref(crl)) + return 0; if (!sk_X509_CRL_push(*sk, crl)) { X509_CRL_free(crl); return 0; @@ -363,7 +364,9 @@ int PKCS7_SIGNER_INFO_set(PKCS7_SIGNER_INFO *p7i, X509 *x509, EVP_PKEY *pkey, return 0; /* lets keep the pkey around for a while */ - EVP_PKEY_up_ref(pkey); + if (!EVP_PKEY_up_ref(pkey)) + return 0; + p7i->pkey = pkey; /* Set the algorithms */ @@ -663,7 +666,9 @@ int PKCS7_RECIP_INFO_set(PKCS7_RECIP_INFO *p7i, X509 *x509) goto err; } finished: - X509_up_ref(x509); + if (!X509_up_ref(x509)) + goto err; + p7i->cert = x509; return 1; diff --git a/crypto/store/store_lib.c b/crypto/store/store_lib.c index 82874e8a52eb5..6b5a5487cbd0e 100644 --- a/crypto/store/store_lib.c +++ b/crypto/store/store_lib.c @@ -742,7 +742,8 @@ EVP_PKEY *OSSL_STORE_INFO_get0_PARAMS(const OSSL_STORE_INFO *info) EVP_PKEY *OSSL_STORE_INFO_get1_PARAMS(const OSSL_STORE_INFO *info) { if (info->type == OSSL_STORE_INFO_PARAMS) { - EVP_PKEY_up_ref(info->_.params); + if (!EVP_PKEY_up_ref(info->_.params)) + return NULL; return info->_.params; } ERR_raise(ERR_LIB_OSSL_STORE, OSSL_STORE_R_NOT_PARAMETERS); @@ -759,7 +760,8 @@ EVP_PKEY *OSSL_STORE_INFO_get0_PUBKEY(const OSSL_STORE_INFO *info) EVP_PKEY *OSSL_STORE_INFO_get1_PUBKEY(const OSSL_STORE_INFO *info) { if (info->type == OSSL_STORE_INFO_PUBKEY) { - EVP_PKEY_up_ref(info->_.pubkey); + if (!EVP_PKEY_up_ref(info->_.pubkey)) + return NULL; return info->_.pubkey; } ERR_raise(ERR_LIB_OSSL_STORE, OSSL_STORE_R_NOT_A_PUBLIC_KEY); @@ -776,7 +778,8 @@ EVP_PKEY *OSSL_STORE_INFO_get0_PKEY(const OSSL_STORE_INFO *info) EVP_PKEY *OSSL_STORE_INFO_get1_PKEY(const OSSL_STORE_INFO *info) { if (info->type == OSSL_STORE_INFO_PKEY) { - EVP_PKEY_up_ref(info->_.pkey); + if (!EVP_PKEY_up_ref(info->_.pkey)) + return NULL; return info->_.pkey; } ERR_raise(ERR_LIB_OSSL_STORE, OSSL_STORE_R_NOT_A_PRIVATE_KEY); @@ -793,7 +796,8 @@ X509 *OSSL_STORE_INFO_get0_CERT(const OSSL_STORE_INFO *info) X509 *OSSL_STORE_INFO_get1_CERT(const OSSL_STORE_INFO *info) { if (info->type == OSSL_STORE_INFO_CERT) { - X509_up_ref(info->_.x509); + if (!X509_up_ref(info->_.x509)) + return NULL; return info->_.x509; } ERR_raise(ERR_LIB_OSSL_STORE, OSSL_STORE_R_NOT_A_CERTIFICATE); @@ -810,7 +814,8 @@ X509_CRL *OSSL_STORE_INFO_get0_CRL(const OSSL_STORE_INFO *info) X509_CRL *OSSL_STORE_INFO_get1_CRL(const OSSL_STORE_INFO *info) { if (info->type == OSSL_STORE_INFO_CRL) { - X509_CRL_up_ref(info->_.crl); + if (!X509_CRL_up_ref(info->_.crl)) + return NULL; return info->_.crl; } ERR_raise(ERR_LIB_OSSL_STORE, OSSL_STORE_R_NOT_A_CRL); diff --git a/crypto/store/store_meth.c b/crypto/store/store_meth.c index 6ac8fd5f9374b..6c952e28962de 100644 --- a/crypto/store/store_meth.c +++ b/crypto/store/store_meth.c @@ -48,12 +48,13 @@ static OSSL_STORE_LOADER *new_loader(OSSL_PROVIDER *prov) OSSL_STORE_LOADER *loader; if ((loader = OPENSSL_zalloc(sizeof(*loader))) == NULL - || !CRYPTO_NEW_REF(&loader->refcnt, 1)) { + || !CRYPTO_NEW_REF(&loader->refcnt, 1) + || !ossl_provider_up_ref(prov)) { OPENSSL_free(loader); return NULL; } + loader->prov = prov; - ossl_provider_up_ref(prov); return loader; } diff --git a/crypto/ts/ts_rsp_sign.c b/crypto/ts/ts_rsp_sign.c index 17f553c1de803..093b517a26632 100644 --- a/crypto/ts/ts_rsp_sign.c +++ b/crypto/ts/ts_rsp_sign.c @@ -142,17 +142,22 @@ int TS_RESP_CTX_set_signer_cert(TS_RESP_CTX *ctx, X509 *signer) ERR_raise(ERR_LIB_TS, TS_R_INVALID_SIGNER_CERTIFICATE_PURPOSE); return 0; } + if (!X509_up_ref(signer)) + return 0; + X509_free(ctx->signer_cert); ctx->signer_cert = signer; - X509_up_ref(ctx->signer_cert); + return 1; } int TS_RESP_CTX_set_signer_key(TS_RESP_CTX *ctx, EVP_PKEY *key) { + if (!EVP_PKEY_up_ref(key)) + return 0; + EVP_PKEY_free(ctx->signer_key); ctx->signer_key = key; - EVP_PKEY_up_ref(ctx->signer_key); return 1; } diff --git a/crypto/ts/ts_rsp_verify.c b/crypto/ts/ts_rsp_verify.c index cdeac8ef8f564..c816ebedb94fd 100644 --- a/crypto/ts/ts_rsp_verify.c +++ b/crypto/ts/ts_rsp_verify.c @@ -150,8 +150,10 @@ int TS_RESP_verify_signature(PKCS7 *token, STACK_OF(X509) *certs, } if (signer_out) { + if (!X509_up_ref(signer)) + goto err; + *signer_out = signer; - X509_up_ref(signer); } ret = 1; diff --git a/crypto/x509/pcy_tree.c b/crypto/x509/pcy_tree.c index 7692dc21e6d70..6012cd25e1efe 100644 --- a/crypto/x509/pcy_tree.c +++ b/crypto/x509/pcy_tree.c @@ -211,7 +211,9 @@ static int tree_init(X509_POLICY_TREE **ptree, STACK_OF(X509) *certs, /* Access the cache which we now know exists */ cache = ossl_policy_cache_set(x); - X509_up_ref(x); + if (!X509_up_ref(x)) + goto bad_tree; + (++level)->cert = x; if (!cache->anyPolicy) diff --git a/crypto/x509/x509_cmp.c b/crypto/x509/x509_cmp.c index 7094280d485e8..ad69f64c745e2 100644 --- a/crypto/x509/x509_cmp.c +++ b/crypto/x509/x509_cmp.c @@ -214,13 +214,14 @@ int X509_add_cert(STACK_OF(X509) *sk, X509 *cert, int flags) if (ret != 0) return ret > 0 ? 1 : 0; } + if ((flags & X509_ADD_FLAG_UP_REF) != 0 && !X509_up_ref(cert)) + return 0; if (!sk_X509_insert(sk, cert, (flags & X509_ADD_FLAG_PREPEND) != 0 ? 0 : -1)) { + X509_free(cert); ERR_raise(ERR_LIB_X509, ERR_R_CRYPTO_LIB); return 0; } - if ((flags & X509_ADD_FLAG_UP_REF) != 0) - (void)X509_up_ref(cert); return 1; } diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c index b8a5ec49279d9..9fe9733bca47b 100644 --- a/crypto/x509/x509_vfy.c +++ b/crypto/x509/x509_vfy.c @@ -1188,12 +1188,13 @@ static int get_crl_sk(X509_STORE_CTX *ctx, X509_CRL **pcrl, X509_CRL **pdcrl, } if (best_crl != NULL) { + if (!X509_CRL_up_ref(best_crl)) + return 0; X509_CRL_free(*pcrl); *pcrl = best_crl; *pissuer = best_crl_issuer; *pscore = best_score; *preasons = best_reasons; - X509_CRL_up_ref(best_crl); X509_CRL_free(*pdcrl); *pdcrl = NULL; get_delta_sk(ctx, pdcrl, pscore, best_crl, crls); @@ -1279,10 +1280,16 @@ static void get_delta_sk(X509_STORE_CTX *ctx, X509_CRL **dcrl, int *pscore, for (i = 0; i < sk_X509_CRL_num(crls); i++) { delta = sk_X509_CRL_value(crls, i); if (check_delta_base(delta, base)) { + if (!X509_CRL_up_ref(delta)) { + *dcrl = NULL; + return; + } + + *dcrl = delta; + if (check_crl_time(ctx, delta, 0)) *pscore |= CRL_SCORE_TIME_DELTA; - X509_CRL_up_ref(delta); - *dcrl = delta; + return; } } @@ -2970,11 +2977,15 @@ static int dane_match_cert(X509_STORE_CTX *ctx, X509 *cert, int depth) if (DANETLS_USAGE_BIT(usage) & DANETLS_DANE_MASK) matched = 1; if (matched || dane->mdpth < 0) { - dane->mdpth = depth; - dane->mtlsa = t; + if (!X509_up_ref(cert)) { + matched = -1; + break; + } + OPENSSL_free(dane->mcert); dane->mcert = cert; - X509_up_ref(cert); + dane->mdpth = depth; + dane->mtlsa = t; } break; } diff --git a/doc/internal/man3/evp_generic_fetch.pod b/doc/internal/man3/evp_generic_fetch.pod index 8057a7170eca8..2d5596e14641f 100644 --- a/doc/internal/man3/evp_generic_fetch.pod +++ b/doc/internal/man3/evp_generic_fetch.pod @@ -121,10 +121,8 @@ And here's the implementation of the FOO method fetcher: if ((foo = OPENSSL_zalloc(sizeof(*foo))) == NULL) return NULL; - if (!CRYPTO_NEW_REF(&foo->refcnt, 1)) { - OPENSSL_free(foo); - return NULL; - } + if (!CRYPTO_NEW_REF(&foo->refcnt, 1)) + goto err; foo->name_id = name_id; @@ -147,11 +145,16 @@ And here's the implementation of the FOO method fetcher: break; } } + if (prov != NULL && !ossl_provider_up_ref(prov)) + goto err; + foo->prov = prov; - if (prov) - ossl_provider_up_ref(prov); return foo; + + err: + OPENSSL_free(foo); + return NULL } EVP_FOO_meth_free(EVP_FOO *foo) diff --git a/providers/implementations/ciphers/cipher_aes_siv_hw.c b/providers/implementations/ciphers/cipher_aes_siv_hw.c index fb302c3a88c64..ac5931c4bb87c 100644 --- a/providers/implementations/ciphers/cipher_aes_siv_hw.c +++ b/providers/implementations/ciphers/cipher_aes_siv_hw.c @@ -61,16 +61,20 @@ static int aes_siv_dupctx(void *in_vctx, void *out_vctx) PROV_AES_SIV_CTX *in = (PROV_AES_SIV_CTX *)in_vctx; PROV_AES_SIV_CTX *out = (PROV_AES_SIV_CTX *)out_vctx; + if (in->cbc != NULL && !EVP_CIPHER_up_ref(in->cbc)) + return 0; + if (in->ctr != NULL && !EVP_CIPHER_up_ref(in->ctr)) { + EVP_CIPHER_free(in->cbc); + return 0; + } + *out = *in; out->siv.cipher_ctx = NULL; out->siv.mac_ctx_init = NULL; out->siv.mac = NULL; if (!ossl_siv128_copy_ctx(&out->siv, &in->siv)) return 0; - if (out->cbc != NULL) - EVP_CIPHER_up_ref(out->cbc); - if (out->ctr != NULL) - EVP_CIPHER_up_ref(out->ctr); + return 1; } diff --git a/ssl/bio_ssl.c b/ssl/bio_ssl.c index ac65a3988bd10..3ad617d9bad21 100644 --- a/ssl/bio_ssl.c +++ b/ssl/bio_ssl.c @@ -297,10 +297,13 @@ static long ssl_ctrl(BIO *b, int cmd, long num, void *ptr) bs->ssl = ssl; bio = SSL_get_rbio(ssl); if (bio != NULL) { + if (!BIO_up_ref(bio)) { + ret = 0; + break; + } if (next != NULL) BIO_push(bio, next); BIO_set_next(b, bio); - BIO_up_ref(bio); } BIO_set_init(b, 1); break; @@ -336,8 +339,10 @@ static long ssl_ctrl(BIO *b, int cmd, long num, void *ptr) * We are going to pass ownership of next to the SSL object...but * we don't own a reference to pass yet - so up ref */ - BIO_up_ref(next); - SSL_set_bio(ssl, next, next); + if (!BIO_up_ref(next)) + ret = 0; + else + SSL_set_bio(ssl, next, next); } break; case BIO_CTRL_POP: diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index b98464256e6c5..e9be38a0c0951 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -3796,7 +3796,9 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg) if (sc->session == NULL || sc->s3.peer_tmp == NULL) { return 0; } else { - EVP_PKEY_up_ref(sc->s3.peer_tmp); + if (!EVP_PKEY_up_ref(sc->s3.peer_tmp)) + return 0; + *(EVP_PKEY **)parg = sc->s3.peer_tmp; return 1; } @@ -3805,7 +3807,9 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg) if (sc->session == NULL || sc->s3.tmp.pkey == NULL) { return 0; } else { - EVP_PKEY_up_ref(sc->s3.tmp.pkey); + if (!EVP_PKEY_up_ref(sc->s3.tmp.pkey)) + return 0; + *(EVP_PKEY **)parg = sc->s3.tmp.pkey; return 1; } diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c index 4aef14952006b..155338ac551a2 100644 --- a/ssl/ssl_cert.c +++ b/ssl/ssl_cert.c @@ -68,23 +68,18 @@ CERT *ssl_cert_new(size_t ssl_pkey_num) if (!ossl_assert(ssl_pkey_num >= SSL_PKEY_NUM)) return NULL; - ret = OPENSSL_zalloc(sizeof(*ret)); + ret = OPENSSL_zalloc(sizeof(*ret) + ssl_pkey_num * sizeof(CERT_PKEY)); if (ret == NULL) return NULL; ret->ssl_pkey_num = ssl_pkey_num; - ret->pkeys = OPENSSL_zalloc(ret->ssl_pkey_num * sizeof(CERT_PKEY)); - if (ret->pkeys == NULL) { - OPENSSL_free(ret); - return NULL; - } + ret->pkeys = (CERT_PKEY *)(ret + 1); ret->key = &(ret->pkeys[SSL_PKEY_RSA]); ret->sec_cb = ssl_security_default_callback; ret->sec_level = OPENSSL_TLS_SECURITY_LEVEL; ret->sec_ex = NULL; if (!CRYPTO_NEW_REF(&ret->references, 1)) { - OPENSSL_free(ret->pkeys); OPENSSL_free(ret); return NULL; } @@ -94,7 +89,7 @@ CERT *ssl_cert_new(size_t ssl_pkey_num) CERT *ssl_cert_dup(CERT *cert) { - CERT *ret = OPENSSL_zalloc(sizeof(*ret)); + CERT *ret = OPENSSL_zalloc(sizeof(*ret) + cert->ssl_pkey_num * sizeof(CERT_PKEY)); size_t i; #ifndef OPENSSL_NO_COMP_ALG int j; @@ -104,22 +99,18 @@ CERT *ssl_cert_dup(CERT *cert) return NULL; ret->ssl_pkey_num = cert->ssl_pkey_num; - ret->pkeys = OPENSSL_zalloc(ret->ssl_pkey_num * sizeof(CERT_PKEY)); - if (ret->pkeys == NULL) { - OPENSSL_free(ret); - return NULL; - } + ret->pkeys = (CERT_PKEY *)(ret + 1); ret->key = &ret->pkeys[cert->key - cert->pkeys]; if (!CRYPTO_NEW_REF(&ret->references, 1)) { - OPENSSL_free(ret->pkeys); OPENSSL_free(ret); return NULL; } if (cert->dh_tmp != NULL) { + if (!EVP_PKEY_up_ref(cert->dh_tmp)) + goto err; ret->dh_tmp = cert->dh_tmp; - EVP_PKEY_up_ref(ret->dh_tmp); } ret->dh_tmp_cb = cert->dh_tmp_cb; @@ -130,13 +121,15 @@ CERT *ssl_cert_dup(CERT *cert) CERT_PKEY *rpk = ret->pkeys + i; if (cpk->x509 != NULL) { + if (!X509_up_ref(cpk->x509)) + goto err; rpk->x509 = cpk->x509; - X509_up_ref(rpk->x509); } if (cpk->privatekey != NULL) { + if (!EVP_PKEY_up_ref(cpk->privatekey)) + goto err; rpk->privatekey = cpk->privatekey; - EVP_PKEY_up_ref(cpk->privatekey); } if (cpk->chain) { @@ -200,12 +193,14 @@ CERT *ssl_cert_dup(CERT *cert) ret->cert_cb_arg = cert->cert_cb_arg; if (cert->verify_store) { - X509_STORE_up_ref(cert->verify_store); + if (!X509_STORE_up_ref(cert->verify_store)) + goto err; ret->verify_store = cert->verify_store; } if (cert->chain_store) { - X509_STORE_up_ref(cert->chain_store); + if (!X509_STORE_up_ref(cert->chain_store)) + goto err; ret->chain_store = cert->chain_store; } @@ -286,7 +281,6 @@ void ssl_cert_free(CERT *c) #ifndef OPENSSL_NO_PSK OPENSSL_free(c->psk_identity_hint); #endif - OPENSSL_free(c->pkeys); CRYPTO_FREE_REF(&c->references); OPENSSL_free(c); } @@ -349,9 +343,8 @@ int ssl_cert_add0_chain_cert(SSL_CONNECTION *s, SSL_CTX *ctx, X509 *x) int ssl_cert_add1_chain_cert(SSL_CONNECTION *s, SSL_CTX *ctx, X509 *x) { - if (!ssl_cert_add0_chain_cert(s, ctx, x)) + if (!X509_up_ref(x) || !ssl_cert_add0_chain_cert(s, ctx, x)) return 0; - X509_up_ref(x); return 1; } @@ -1161,14 +1154,17 @@ int ssl_build_cert_chain(SSL_CONNECTION *s, SSL_CTX *ctx, int flags) int ssl_cert_set_cert_store(CERT *c, X509_STORE *store, int chain, int ref) { X509_STORE **pstore; + + if (ref && store && !X509_STORE_up_ref(store)) + return 0; + if (chain) pstore = &c->chain_store; else pstore = &c->verify_store; X509_STORE_free(*pstore); *pstore = store; - if (ref && store) - X509_STORE_up_ref(store); + return 1; } diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index 5afd89c3e9c6f..366b0be71ef3a 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -702,27 +702,26 @@ SSL *SSL_new(SSL_CTX *ctx) int ossl_ssl_init(SSL *ssl, SSL_CTX *ctx, const SSL_METHOD *method, int type) { - ssl->type = type; - ssl->lock = CRYPTO_THREAD_lock_new(); if (ssl->lock == NULL) return 0; if (!CRYPTO_NEW_REF(&ssl->references, 1)) { CRYPTO_THREAD_lock_free(ssl->lock); + ssl->lock = NULL; return 0; } - if (!CRYPTO_new_ex_data(CRYPTO_EX_INDEX_SSL, ssl, &ssl->ex_data)) { + if (!CRYPTO_new_ex_data(CRYPTO_EX_INDEX_SSL, ssl, &ssl->ex_data) + || !SSL_CTX_up_ref(ctx)) { CRYPTO_THREAD_lock_free(ssl->lock); CRYPTO_FREE_REF(&ssl->references); ssl->lock = NULL; return 0; } - SSL_CTX_up_ref(ctx); ssl->ctx = ctx; - + ssl->type = type; ssl->defltmeth = ssl->method = method; return 1; @@ -823,7 +822,10 @@ SSL *ossl_ssl_connection_new_int(SSL_CTX *ctx, SSL *user_ssl, s->ext.ocsp.exts = NULL; s->ext.ocsp.resp = NULL; s->ext.ocsp.resp_len = 0; - SSL_CTX_up_ref(ctx); + + if (!SSL_CTX_up_ref(ctx)) + goto err; + s->session_ctx = ctx; if (ctx->ext.ecpointformats) { s->ext.ecpointformats = @@ -1945,8 +1947,8 @@ X509 *SSL_get1_peer_certificate(const SSL *s) { X509 *r = SSL_get0_peer_certificate(s); - if (r != NULL) - X509_up_ref(r); + if (r != NULL && !X509_up_ref(r)) + return NULL; return r; } @@ -4675,8 +4677,7 @@ void ssl_update_cache(SSL_CONNECTION *s, int mode) * TLSv1.3 without early data because some applications just want to * know about the creation of a session and aren't doing a full cache. */ - if (s->session_ctx->new_session_cb != NULL) { - SSL_SESSION_up_ref(s->session); + if (s->session_ctx->new_session_cb != NULL && SSL_SESSION_up_ref(s->session)) { if (!s->session_ctx->new_session_cb(SSL_CONNECTION_GET_USER_SSL(s), s->session)) SSL_SESSION_free(s->session); @@ -5386,24 +5387,19 @@ SSL_CTX *SSL_set_SSL_CTX(SSL *ssl, SSL_CTX *ctx) if (ctx == NULL) ctx = sc->session_ctx; new_cert = ssl_cert_dup(ctx->cert); - if (new_cert == NULL) { - return NULL; - } - - if (!custom_exts_copy_flags(&new_cert->custext, &sc->cert->custext)) { - ssl_cert_free(new_cert); - return NULL; - } - - ssl_cert_free(sc->cert); - sc->cert = new_cert; + if (new_cert == NULL) + goto err; + if (!custom_exts_copy_flags(&new_cert->custext, &sc->cert->custext)) + goto err; /* * Program invariant: |sid_ctx| has fixed size (SSL_MAX_SID_CTX_LENGTH), * so setter APIs must prevent invalid lengths from entering the system. */ if (!ossl_assert(sc->sid_ctx_length <= sizeof(sc->sid_ctx))) - return NULL; + goto err; + if (!SSL_CTX_up_ref(ctx)) + goto err; /* * If the session ID context matches that of the parent SSL_CTX, @@ -5418,11 +5414,16 @@ SSL_CTX *SSL_set_SSL_CTX(SSL *ssl, SSL_CTX *ctx) memcpy(&sc->sid_ctx, &ctx->sid_ctx, sizeof(sc->sid_ctx)); } - SSL_CTX_up_ref(ctx); + ssl_cert_free(sc->cert); + sc->cert = new_cert; SSL_CTX_free(ssl->ctx); /* decrement reference count */ ssl->ctx = ctx; return ssl->ctx; + +err: + ssl_cert_free(new_cert); + return NULL; } int SSL_CTX_set_default_verify_paths(SSL_CTX *ctx) @@ -5647,8 +5648,9 @@ void SSL_CTX_set_cert_store(SSL_CTX *ctx, X509_STORE *store) void SSL_CTX_set1_cert_store(SSL_CTX *ctx, X509_STORE *store) { - if (store != NULL) - X509_STORE_up_ref(store); + if (store != NULL && !X509_STORE_up_ref(store)) + return; + SSL_CTX_set_cert_store(ctx, store); } diff --git a/ssl/ssl_rsa.c b/ssl/ssl_rsa.c index dee9d7baf0c49..789732445bd74 100644 --- a/ssl/ssl_rsa.c +++ b/ssl/ssl_rsa.c @@ -141,9 +141,10 @@ static int ssl_set_pkey(CERT *c, EVP_PKEY *pkey, SSL_CTX *ctx) if (c->pkeys[i].x509 != NULL && !X509_check_private_key(c->pkeys[i].x509, pkey)) return 0; + if (!EVP_PKEY_up_ref(pkey)) + return 0; EVP_PKEY_free(c->pkeys[i].privatekey); - EVP_PKEY_up_ref(pkey); c->pkeys[i].privatekey = pkey; c->key = &c->pkeys[i]; return 1; @@ -295,8 +296,10 @@ static int ssl_set_cert(CERT *c, X509 *x, SSL_CTX *ctx) } } + if (!X509_up_ref(x)) + return 0; + X509_free(c->pkeys[i].x509); - X509_up_ref(x); c->pkeys[i].x509 = x; c->key = &(c->pkeys[i]); @@ -1046,21 +1049,27 @@ static int ssl_set_cert_and_key(SSL *ssl, SSL_CTX *ctx, X509 *x509, EVP_PKEY *pr if (chain != NULL) { dup_chain = X509_chain_up_ref(chain); - if (dup_chain == NULL) { + if (dup_chain == NULL) { ERR_raise(ERR_LIB_SSL, ERR_R_X509_LIB); goto out; } } + if (!X509_up_ref(x509)) + goto out; + + if (!EVP_PKEY_up_ref(privatekey)) { + X509_free(x509); + goto out; + } + OSSL_STACK_OF_X509_free(c->pkeys[i].chain); c->pkeys[i].chain = dup_chain; X509_free(c->pkeys[i].x509); - X509_up_ref(x509); c->pkeys[i].x509 = x509; EVP_PKEY_free(c->pkeys[i].privatekey); - EVP_PKEY_up_ref(privatekey); c->pkeys[i].privatekey = privatekey; c->key = &(c->pkeys[i]); diff --git a/ssl/ssl_rsa_legacy.c b/ssl/ssl_rsa_legacy.c index de63c5b47a789..410c98e682b95 100644 --- a/ssl/ssl_rsa_legacy.c +++ b/ssl/ssl_rsa_legacy.c @@ -28,7 +28,11 @@ int SSL_use_RSAPrivateKey(SSL *ssl, RSA *rsa) return 0; } - RSA_up_ref(rsa); + if (!RSA_up_ref(rsa)) { + EVP_PKEY_free(pkey); + return 0; + } + if (EVP_PKEY_assign_RSA(pkey, rsa) <= 0) { RSA_free(rsa); EVP_PKEY_free(pkey); @@ -115,7 +119,11 @@ int SSL_CTX_use_RSAPrivateKey(SSL_CTX *ctx, RSA *rsa) return 0; } - RSA_up_ref(rsa); + if (!RSA_up_ref(rsa)) { + EVP_PKEY_free(pkey); + return 0; + } + if (EVP_PKEY_assign_RSA(pkey, rsa) <= 0) { RSA_free(rsa); EVP_PKEY_free(pkey); diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c index 69149de0507c9..dfe715a4aae1a 100644 --- a/ssl/ssl_sess.c +++ b/ssl/ssl_sess.c @@ -82,8 +82,8 @@ SSL_SESSION *SSL_get1_session(SSL *ssl) if (!CRYPTO_THREAD_read_lock(ssl->lock)) return NULL; sess = SSL_get_session(ssl); - if (sess != NULL) - SSL_SESSION_up_ref(sess); + if (sess != NULL && !SSL_SESSION_up_ref(sess)) + sess = NULL; CRYPTO_THREAD_unlock(ssl->lock); return sess; } @@ -512,7 +512,10 @@ SSL_SESSION *lookup_sess_in_cache(SSL_CONNECTION *s, ret = lh_SSL_SESSION_retrieve(s->session_ctx->sessions, &data); if (ret != NULL) { /* don't allow other threads to steal it: */ - SSL_SESSION_up_ref(ret); + if (!SSL_SESSION_up_ref(ret)) { + CRYPTO_THREAD_unlock(s->session_ctx->lock); + return NULL; + } } CRYPTO_THREAD_unlock(s->session_ctx->lock); if (ret == NULL) @@ -542,8 +545,8 @@ SSL_SESSION *lookup_sess_in_cache(SSL_CONNECTION *s, * reference count itself [i.e. copy == 0], or things won't be * thread-safe). */ - if (copy) - SSL_SESSION_up_ref(ret); + if (copy && !SSL_SESSION_up_ref(ret)) + return NULL; /* * Add the externally cached session to the internal cache as @@ -726,7 +729,8 @@ int SSL_CTX_add_session(SSL_CTX *ctx, SSL_SESSION *c) * it has two ways of access: each session is in a doubly linked list and * an lhash */ - SSL_SESSION_up_ref(c); + if (!SSL_SESSION_up_ref(c)) + return 0; /* * if session c is in already in cache, we take back the increment later */ @@ -890,16 +894,20 @@ int SSL_set_session(SSL *s, SSL_SESSION *session) if (sc == NULL) return 0; + if (session != NULL && !SSL_SESSION_up_ref(session)) + return 0; + ssl_clear_bad_session(sc); if (s->defltmeth != s->method) { - if (!SSL_set_ssl_method(s, s->defltmeth)) + if (!SSL_set_ssl_method(s, s->defltmeth)) { + SSL_SESSION_free(session); return 0; + } } - if (session != NULL) { - SSL_SESSION_up_ref(session); + if (session != NULL) sc->verify_result = session->verify_result; - } + SSL_SESSION_free(sc->session); sc->session = session; diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index 436b397346cf8..4ffc866bc960c 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -2127,8 +2127,12 @@ WORK_STATE tls_post_process_server_certificate(SSL_CONNECTION *s, } } + if (!X509_up_ref(x)) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); + return WORK_ERROR; + } + X509_free(s->session->peer); - X509_up_ref(x); s->session->peer = x; s->session->verify_result = s->verify_result; /* Ensure there is no RPK */ diff --git a/test/bio_prefix_text.c b/test/bio_prefix_text.c index 79ff8ec4a274c..ae35f594dcc35 100644 --- a/test/bio_prefix_text.c +++ b/test/bio_prefix_text.c @@ -101,8 +101,10 @@ static int setup_bio_chain(const char *progname) if (chain != NULL) { size_t i; + if (!BIO_up_ref(bio_out)) /* Protection against freeing */ + goto err; + next = bio_out; - BIO_up_ref(next); /* Protection against freeing */ for (i = 0; n > 0; i++, n--) { BIO *curr = BIO_new(BIO_f_prefix()); diff --git a/test/cmp_client_test.c b/test/cmp_client_test.c index 208e0a176733a..c38ccadf30552 100644 --- a/test/cmp_client_test.c +++ b/test/cmp_client_test.c @@ -273,7 +273,9 @@ static int test_exec_KUR_ses(int transfer_error, int pubkey, int raverified) if (pubkey) { EVP_PKEY *key = raverified /* wrong key */ ? server_key : client_key; - EVP_PKEY_up_ref(key); + if (!EVP_PKEY_up_ref(key)) + return 0; + OSSL_CMP_CTX_set0_newPkey(fixture->cmp_ctx, 0 /* not priv */, key); OSSL_CMP_SRV_CTX_set_accept_raverified(fixture->srv_ctx, 1); } diff --git a/test/cmp_protect_test.c b/test/cmp_protect_test.c index 89f342458ec4e..4eac1664e11e9 100644 --- a/test/cmp_protect_test.c +++ b/test/cmp_protect_test.c @@ -262,11 +262,11 @@ static int test_MSG_protect_certificate_based_without_cert(void) if (!TEST_ptr(fixture->msg = OSSL_CMP_MSG_dup(ir_unprotected)) || !TEST_true(SET_OPT_UNPROTECTED_SEND(ctx, 0)) + || !TEST_true(EVP_PKEY_up_ref(server_key)) || !TEST_true(OSSL_CMP_CTX_set0_newPkey(ctx, 1, server_key))) { tear_down(fixture); fixture = NULL; } - EVP_PKEY_up_ref(server_key); EXECUTE_TEST(execute_MSG_protect_test, tear_down); return result; } diff --git a/test/crltest.c b/test/crltest.c index 37fa6c13c2648..76502fd28a22b 100644 --- a/test/crltest.c +++ b/test/crltest.c @@ -266,9 +266,13 @@ static int verify(X509 *leaf, X509 *root, STACK_OF(X509_CRL) *crls, goto err; /* Create a stack; upref the cert because we free it below. */ - X509_up_ref(root); - if (!TEST_true(sk_X509_push(roots, root)) - || !TEST_true(X509_STORE_CTX_init(ctx, store, leaf, NULL))) + if (!TEST_true(X509_up_ref(root))) + goto err; + if (!TEST_true(sk_X509_push(roots, root))) { + X509_free(root); + goto err; + } + if (!TEST_true(X509_STORE_CTX_init(ctx, store, leaf, NULL))) goto err; X509_STORE_CTX_set0_trusted_stack(ctx, roots); X509_STORE_CTX_set0_crls(ctx, crls); @@ -302,13 +306,31 @@ static STACK_OF(X509_CRL) *make_CRL_stack(X509_CRL *x1, X509_CRL *x2) { STACK_OF(X509_CRL) *sk = sk_X509_CRL_new_null(); - sk_X509_CRL_push(sk, x1); - X509_CRL_up_ref(x1); + if (x1 != NULL) { + if (!X509_CRL_up_ref(x1)) { + goto err; + } + if (!sk_X509_CRL_push(sk, x1)) { + X509_CRL_free(x1); + goto err; + } + } + if (x2 != NULL) { - sk_X509_CRL_push(sk, x2); - X509_CRL_up_ref(x2); + if (!X509_CRL_up_ref(x2)) { + goto err; + } + if (!sk_X509_CRL_push(sk, x2)) { + X509_CRL_free(x2); + goto err; + } } + return sk; + +err: + sk_X509_CRL_pop_free(sk, X509_CRL_free); + return NULL; } static int test_basic_crl(void) diff --git a/test/evp_extra_test.c b/test/evp_extra_test.c index 674180de3564b..306ec8e6478c6 100644 --- a/test/evp_extra_test.c +++ b/test/evp_extra_test.c @@ -2754,7 +2754,9 @@ static int test_EVP_PKEY_check(int i) #ifndef OPENSSL_NO_DEPRECATED_3_0 ctx2 = EVP_PKEY_CTX_new_id(0xdefaced, NULL); /* assign the pkey directly, as an internal test */ - EVP_PKEY_up_ref(pkey); + if (!EVP_PKEY_up_ref(pkey)) + goto done; + ctx2->pkey = pkey; if (!TEST_int_eq(EVP_PKEY_check(ctx2), 0xbeef)) @@ -2772,9 +2774,10 @@ static int test_EVP_PKEY_check(int i) done: EVP_PKEY_CTX_free(ctx); #ifndef OPENSSL_NO_DEPRECATED_3_0 + if (ctx2 != NULL) + EVP_PKEY_free(ctx2->pkey); EVP_PKEY_CTX_free(ctx2); #endif - EVP_PKEY_free(pkey); return ret; } diff --git a/test/helpers/ssltestlib.c b/test/helpers/ssltestlib.c index 07e0756803cad..4a678a058bf6d 100644 --- a/test/helpers/ssltestlib.c +++ b/test/helpers/ssltestlib.c @@ -1191,8 +1191,14 @@ int create_ssl_objects(SSL_CTX *serverctx, SSL_CTX *clientctx, SSL **sssl, /* Up ref these as we are passing them to two SSL objects */ SSL_set_bio(serverssl, c_to_s_bio, s_to_c_bio); - BIO_up_ref(s_to_c_bio); - BIO_up_ref(c_to_s_bio); + + if (!BIO_up_ref(s_to_c_bio)) + goto error; + if (!BIO_up_ref(c_to_s_bio)) { + BIO_free(s_to_c_bio); + goto error; + } + SSL_set_bio(clientssl, s_to_c_bio, c_to_s_bio); *sssl = serverssl; *cssl = clientssl; diff --git a/test/quicapitest.c b/test/quicapitest.c index d384d17cbd17d..5e74a0217793b 100644 --- a/test/quicapitest.c +++ b/test/quicapitest.c @@ -1155,11 +1155,9 @@ static int use_session_cb(SSL *ssl, const EVP_MD *md, const unsigned char **id, { use_session_cb_cnt++; - if (clientpsk == NULL) + if (clientpsk == NULL || !SSL_SESSION_up_ref(clientpsk)) return 0; - SSL_SESSION_up_ref(clientpsk); - *sess = clientpsk; *id = (const unsigned char *)pskid; *idlen = strlen(pskid); @@ -1172,7 +1170,7 @@ static int find_session_cb(SSL *ssl, const unsigned char *identity, { find_session_cb_cnt++; - if (serverpsk == NULL) + if (serverpsk == NULL || !SSL_SESSION_up_ref(serverpsk)) return 0; /* Identity should match that set by the client */ @@ -1180,7 +1178,6 @@ static int find_session_cb(SSL *ssl, const unsigned char *identity, || strncmp(pskid, (const char *)identity, identity_len) != 0) return 0; - SSL_SESSION_up_ref(serverpsk); *sess = serverpsk; return 1; @@ -1206,10 +1203,9 @@ static int test_quic_psk(void) find_session_cb_cnt = 0; clientpsk = serverpsk = create_a_psk(clientquic, SHA384_DIGEST_LENGTH); - if (!TEST_ptr(clientpsk)) - goto end; /* We already had one ref. Add another one */ - SSL_SESSION_up_ref(clientpsk); + if (!TEST_ptr(clientpsk) || !TEST_true(SSL_SESSION_up_ref(clientpsk))) + goto end; if (!TEST_true(qtest_create_quic_connection(qtserv, clientquic)) || !TEST_int_eq(1, find_session_cb_cnt) diff --git a/test/sslapitest.c b/test/sslapitest.c index 914471d7861be..d8192f8293e5e 100644 --- a/test/sslapitest.c +++ b/test/sslapitest.c @@ -2692,9 +2692,8 @@ static int test_psk_tickets(void) NULL, NULL))) goto end; clientpsk = serverpsk = create_a_psk(clientssl, SHA384_DIGEST_LENGTH); - if (!TEST_ptr(clientpsk)) + if (!TEST_ptr(clientpsk) || !TEST_true(SSL_SESSION_up_ref(clientpsk))) goto end; - SSL_SESSION_up_ref(clientpsk); if (!TEST_true(create_ssl_connection(serverssl, clientssl, SSL_ERROR_NONE)) @@ -3004,10 +3003,13 @@ static int test_ssl_set_bio(int idx) * each BIO that will have ownership transferred in the SSL_set_bio() * call */ - if (irbio != NULL) - BIO_up_ref(irbio); - if (iwbio != NULL && iwbio != irbio) - BIO_up_ref(iwbio); + if (irbio != NULL && !BIO_up_ref(irbio)) + goto end; + if (iwbio != NULL && iwbio != irbio && !BIO_up_ref(iwbio)) { + if (irbio != NULL) + BIO_free(irbio); + goto end; + } } if (conntype != CONNTYPE_NO_CONNECTION @@ -3027,11 +3029,18 @@ static int test_ssl_set_bio(int idx) if (nrbio != NULL && nrbio != irbio && (nwbio != iwbio || nrbio != nwbio)) - BIO_up_ref(nrbio); + if (!TEST_true(BIO_up_ref(nrbio))) + goto end; if (nwbio != NULL && nwbio != nrbio && (nwbio != iwbio || (nwbio == iwbio && irbio == iwbio))) - BIO_up_ref(nwbio); + if (!TEST_true(BIO_up_ref(nwbio))) { + if (nrbio != NULL + && nrbio != irbio + && (nwbio != iwbio || nrbio != nwbio)) + BIO_free(nrbio); + goto end; + } SSL_set_bio(clientssl, nrbio, nwbio); @@ -3276,8 +3285,8 @@ static int use_session_cb(SSL *ssl, const EVP_MD *md, const unsigned char **id, return 0; } - if (clientpsk != NULL) - SSL_SESSION_up_ref(clientpsk); + if (clientpsk != NULL && !SSL_SESSION_up_ref(clientpsk)) + return 0; *sess = clientpsk; *id = (const unsigned char *)pskid; @@ -3325,7 +3334,7 @@ static int find_session_cb(SSL *ssl, const unsigned char *identity, if (find_session_cb_cnt > 2) return 0; - if (serverpsk == NULL) + if (serverpsk == NULL || !SSL_SESSION_up_ref(serverpsk)) return 0; /* Identity should match that set by the client */ @@ -3336,7 +3345,6 @@ static int find_session_cb(SSL *ssl, const unsigned char *identity, return 1; } - SSL_SESSION_up_ref(serverpsk); *sess = serverpsk; return 1;