Check returns of sk_X509_CRL_push and handle appropriately.

Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26234)

diff --git a/apps/crl2pkcs7.c b/apps/crl2pkcs7.c
index ba24d31d5f4e67a426bf0c6c175270fbb6ee944c..164b9eb640f03f6f29f858a6a110c7aa37225bba 100644 (file)
--- a/apps/crl2pkcs7.c
+++ b/apps/crl2pkcs7.c
@@ -138,7 +138,9 @@ int crl2pkcs7_main(int argc, char **argv)
         if ((crl_stack = sk_X509_CRL_new_null()) == NULL)
             goto end;
         p7s->crl = crl_stack;
-        sk_X509_CRL_push(crl_stack, crl);
+
+        if (!sk_X509_CRL_push(crl_stack, crl))
+            goto end;
         crl = NULL;             /* now part of p7 for OPENSSL_freeing */
     }
 

diff --git a/apps/lib/apps.c b/apps/lib/apps.c
index 3bc64659457e5641defd5ccac0f8085dbb7d69d1..85e5094b9e7967c2fa1bd61b7d386ab77d212816 100644 (file)
--- a/apps/lib/apps.c
+++ b/apps/lib/apps.c
@@ -2515,18 +2515,24 @@ static STACK_OF(X509_CRL) *crls_http_cb(const X509_STORE_CTX *ctx,
     crldp = X509_get_ext_d2i(x, NID_crl_distribution_points, NULL, NULL);
     crl = load_crl_crldp(crldp);
     sk_DIST_POINT_pop_free(crldp, DIST_POINT_free);
-    if (!crl) {
-        sk_X509_CRL_free(crls);
-        return NULL;
-    }
-    sk_X509_CRL_push(crls, crl);
+
+    if (crl == NULL || !sk_X509_CRL_push(crls, crl))
+        goto error;
+
     /* Try to download delta CRL */
     crldp = X509_get_ext_d2i(x, NID_freshest_crl, NULL, NULL);
     crl = load_crl_crldp(crldp);
     sk_DIST_POINT_pop_free(crldp, DIST_POINT_free);
-    if (crl)
-        sk_X509_CRL_push(crls, crl);
+
+    if (crl != NULL && !sk_X509_CRL_push(crls, crl))
+        goto error;
+
     return crls;
+
+error:
+    X509_CRL_free(crl);
+    sk_X509_CRL_free(crls);
+    return NULL;
 }
 
 void store_setup_crl_download(X509_STORE *st)

diff --git a/crypto/cmp/cmp_asn.c b/crypto/cmp/cmp_asn.c
index 4415ede449da71bc0ef700568ef072b7f7afa4b5..13fd6f898d084faf28206a139cf49bf0a43a8618 100644 (file)
--- a/crypto/cmp/cmp_asn.c
+++ b/crypto/cmp/cmp_asn.c
@@ -797,9 +797,10 @@ OSSL_CMP_ITAV *OSSL_CMP_ITAV_new_crls(const X509_CRL *crl)
 
     if (crl != NULL) {
         if ((crls = sk_X509_CRL_new_reserve(NULL, 1)) == NULL
-                || (crl_copy = X509_CRL_dup(crl)) == NULL)
+                || (crl_copy = X509_CRL_dup(crl)) == NULL
+                || !sk_X509_CRL_push(crls, crl_copy))
             goto err;
-        (void)sk_X509_CRL_push(crls, crl_copy); /* cannot fail */
+        crl_copy = NULL; /* ownership transferred to crls */
     }
 
     itav->infoType = OBJ_nid2obj(NID_id_it_crls);
@@ -807,6 +808,7 @@ OSSL_CMP_ITAV *OSSL_CMP_ITAV_new_crls(const X509_CRL *crl)
     return itav;
 
  err:
+    OPENSSL_free(crl_copy);
     sk_X509_CRL_free(crls);
     OSSL_CMP_ITAV_free(itav);
     return NULL;

diff --git a/fuzz/x509.c b/fuzz/x509.c
index ce28e80728d00f45c3807615faec33b51bbaf759..b22390f717a630461afc04d7c0d0d427549c9c2a 100644 (file)
--- a/fuzz/x509.c
+++ b/fuzz/x509.c
@@ -98,10 +98,10 @@ int FuzzerTestOneInput(const uint8_t *buf, size_t len)
 
     if (crl != NULL) {
         crls = sk_X509_CRL_new_null();
-        if (crls == NULL)
+        if (crls == NULL
+            || !sk_X509_CRL_push(crls, crl))
             goto err;
 
-        sk_X509_CRL_push(crls, crl);
         X509_STORE_CTX_set0_crls(ctx, crls);
     }