Adds missing checks of return from XXX_up_ref().

fwh-dc · Jan 1, 2025 · ed912f3 · ed912f3
1 parent 0baa3ac
commit ed912f3
Show file tree

Hide file tree

Showing 45 changed files with 414 additions and 246 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -630,16 +630,17 @@ 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;
         /* NULL out pointers in case of error */
         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
@@ -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)