Check returns of various sk_*_push functions

Check returns of sk_POLICY_MAPPING_push, sk_GENERAL_NAME_push,
sk_ACCESS_DESCRIPTION_push, sk_X509_push, sk_X509_NAME_push,
sk_OPENSSL_CSTRING_push, sk_SCT_push, sk_DIST_POINT_push,
sk_OSSL_CMP_CRLSTATUS_push, sk_ASN1_UTF8STRING_push and
sk_ASN1_OBJECT_push and handle appropriately.

Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26240)

diff --git a/apps/crl2pkcs7.c b/apps/crl2pkcs7.c
index 681c60285f10b701a71cb594d17f403b42507c60..ba24d31d5f4e67a426bf0c6c175270fbb6ee944c 100644 (file)
--- a/apps/crl2pkcs7.c
+++ b/apps/crl2pkcs7.c
@@ -216,7 +216,10 @@ static int add_certs_from_file(STACK_OF(X509) *stack, char *certfile)
     while (sk_X509_INFO_num(sk)) {
         xi = sk_X509_INFO_shift(sk);
         if (xi->x509 != NULL) {
-            sk_X509_push(stack, xi->x509);
+            if (!sk_X509_push(stack, xi->x509)) {
+                X509_INFO_free(xi);
+                goto end;
+            }
             xi->x509 = NULL;
             count++;
         }

diff --git a/apps/engine.c b/apps/engine.c
index c3e8e4a27b0451b092d3ee979f9e4f297a960983..b539ec51dbdabcaaeba8a9be0a30557fa2a74ad0 100644 (file)
--- a/apps/engine.c
+++ b/apps/engine.c
@@ -316,7 +316,8 @@ int engine_main(int argc, char **argv)
      * names, and then setup to parse the rest of the line as flags. */
     prog = argv[0];
     while ((argv1 = argv[1]) != NULL && *argv1 != '-') {
-        sk_OPENSSL_CSTRING_push(engines, argv1);
+        if (!sk_OPENSSL_CSTRING_push(engines, argv1))
+            goto end;
         argc--;
         argv++;
     }
@@ -370,12 +371,14 @@ int engine_main(int argc, char **argv)
             BIO_printf(bio_err, "%s: Use -help for summary.\n", prog);
             goto end;
         }
-        sk_OPENSSL_CSTRING_push(engines, *argv);
+        if (!sk_OPENSSL_CSTRING_push(engines, *argv))
+            goto end;
     }
 
     if (sk_OPENSSL_CSTRING_num(engines) == 0) {
         for (e = ENGINE_get_first(); e != NULL; e = ENGINE_get_next(e)) {
-            sk_OPENSSL_CSTRING_push(engines, ENGINE_get_id(e));
+            if (!sk_OPENSSL_CSTRING_push(engines, ENGINE_get_id(e)))
+                goto end;
         }
     }
 

diff --git a/apps/lib/names.c b/apps/lib/names.c
index 716130c71e45c703cdc34512869743842323d6f4..4c4a26f1a5be4a22f28c29468d0f441938e5c12d 100644 (file)
--- a/apps/lib/names.c
+++ b/apps/lib/names.c
@@ -22,7 +22,8 @@ void collect_names(const char *name, void *vdata)
 {
     STACK_OF(OPENSSL_CSTRING) *names = vdata;
 
-    sk_OPENSSL_CSTRING_push(names, name);
+    /* A failure to push cannot be handled so we ignore the result. */
+    (void)sk_OPENSSL_CSTRING_push(names, name);
 }
 
 void print_names(BIO *out, STACK_OF(OPENSSL_CSTRING) *names)

diff --git a/apps/x509.c b/apps/x509.c
index cd5b7bf796bfeb2b8b5811c3db6583dc849f9db8..cecb8c6909f0eac50a361d48d87111cb1aa3cf56 100644 (file)
--- a/apps/x509.c
+++ b/apps/x509.c
@@ -453,7 +453,8 @@ int x509_main(int argc, char **argv)
                            prog, opt_arg());
                 goto opthelp;
             }
-            sk_ASN1_OBJECT_push(trust, objtmp);
+            if (!sk_ASN1_OBJECT_push(trust, objtmp))
+                goto end;
             trustout = 1;
             break;
         case OPT_ADDREJECT:
@@ -464,7 +465,8 @@ int x509_main(int argc, char **argv)
                            prog, opt_arg());
                 goto opthelp;
             }
-            sk_ASN1_OBJECT_push(reject, objtmp);
+            if (!sk_ASN1_OBJECT_push(trust, objtmp))
+                goto end;
             trustout = 1;
             break;
         case OPT_SETALIAS:

diff --git a/crypto/conf/conf_lib.c b/crypto/conf/conf_lib.c
index 2a1c992eb2154e4ae58a42a7a5a700ff5c64c3ee..f5467448378d0e531466cd20125f0d00cb26d882 100644 (file)
--- a/crypto/conf/conf_lib.c
+++ b/crypto/conf/conf_lib.c
@@ -228,7 +228,8 @@ static void collect_section_name(const CONF_VALUE *v, SECTION_NAMES *names)
 {
     /* A section is a CONF_VALUE with name == NULL */
     if (v->name == NULL)
-        sk_OPENSSL_CSTRING_push(names, v->section);
+        /* A failure to push cannot be handled so we ignore the result. */
+        (void)sk_OPENSSL_CSTRING_push(names, v->section);
 }
 
 static int section_name_cmp(OPENSSL_CSTRING const *a, OPENSSL_CSTRING const *b)

diff --git a/crypto/ocsp/ocsp_ext.c b/crypto/ocsp/ocsp_ext.c
index 9707ccb94f4c511986342e8e06efa35608ab2fa6..99507f854d557f9a621d368fee3d3e3b5951b870 100644 (file)
--- a/crypto/ocsp/ocsp_ext.c
+++ b/crypto/ocsp/ocsp_ext.c
@@ -400,7 +400,8 @@ X509_EXTENSION *OCSP_accept_responses_new(char **oids)
         goto err;
     while (oids && *oids) {
         if ((nid = OBJ_txt2nid(*oids)) != NID_undef && (o = OBJ_nid2obj(nid)))
-            sk_ASN1_OBJECT_push(sk, o);
+            if (!sk_ASN1_OBJECT_push(sk, o))
+                goto err;
         oids++;
     }
     x = X509V3_EXT_i2d(NID_id_pkix_OCSP_acceptableResponses, 0, sk);

diff --git a/engines/e_capi.c b/engines/e_capi.c
index ffc5bf7a2aa8d563c2b852c4acc5a3a71e7b8967..5f49bcc80485ffb1836f08a30c4ccd4d4122f4d8 100644 (file)
--- a/engines/e_capi.c
+++ b/engines/e_capi.c
@@ -1771,7 +1771,10 @@ static int capi_load_ssl_client_cert(ENGINE *e, SSL *ssl,
             if (!certs)
                 certs = sk_X509_new_null();
 
-            sk_X509_push(certs, x);
+            if (!sk_X509_push(certs, x)) {
+                X509_free(x);
+                continue;
+            }
         } else {
             X509_free(x);
         }

diff --git a/fuzz/x509.c b/fuzz/x509.c
index e2d2639164c01d0eda79d74329b078abfd80b0ca..ce28e80728d00f45c3807615faec33b51bbaf759 100644 (file)
--- a/fuzz/x509.c
+++ b/fuzz/x509.c
@@ -115,12 +115,11 @@ int FuzzerTestOneInput(const uint8_t *buf, size_t len)
         ASN1_GENERALIZEDTIME *revtime, *thisupd, *nextupd;
 
         certs = sk_X509_new_null();
-        if (certs == NULL)
+        if (certs == NULL
+            || !sk_X509_push(certs, x509_1)
+            || !sk_X509_push(certs, x509_2))
             goto err;
 
-        sk_X509_push(certs, x509_1);
-        sk_X509_push(certs, x509_2);
-
         OCSP_basic_verify(bs, certs, store, OCSP_PARTIAL_CHAIN);
 
         id = OCSP_cert_to_id(NULL, x509_1, x509_2);

diff --git a/test/cmp_client_test.c b/test/cmp_client_test.c
index 208e0a176733a405a3e397752cc397fd24965ab5..bacdac35c579b2742028290e53ff45c750fc5c30 100644 (file)
--- a/test/cmp_client_test.c
+++ b/test/cmp_client_test.c
@@ -187,8 +187,11 @@ static int test_exec_IR_ses(void)
     fixture->req_type = OSSL_CMP_PKIBODY_IR;
     fixture->expected = OSSL_CMP_PKISTATUS_accepted;
     fixture->caPubs = sk_X509_new_null();
-    sk_X509_push(fixture->caPubs, server_cert);
-    sk_X509_push(fixture->caPubs, server_cert);
+    if (!sk_X509_push(fixture->caPubs, server_cert)
+        || !sk_X509_push(fixture->caPubs, server_cert)) {
+        tear_down(fixture);
+        return 0;
+    }
     ossl_cmp_mock_srv_set1_caPubsOut(fixture->srv_ctx, fixture->caPubs);
     EXECUTE_TEST(execute_exec_certrequest_ses_test, tear_down);
     return result;

diff --git a/test/ct_test.c b/test/ct_test.c
index ff253414f8063fa95e8fb35c3cfaa44e55731e40..b54d7d0fc42a7cd3993a96410d9a48145de6e529 100644 (file)
--- a/test/ct_test.c
+++ b/test/ct_test.c
@@ -463,7 +463,11 @@ static int test_encode_tls_sct(void)
         return 0;
     }
 
-    sk_SCT_push(fixture->sct_list, sct);
+    if (!sk_SCT_push(fixture->sct_list, sct)) {
+        tear_down(fixture);
+        return 0;
+    }
+
     fixture->sct_dir = ct_dir;
     fixture->sct_text_file = "tls1.sct";
     EXECUTE_CT_TEST();

diff --git a/test/v3nametest.c b/test/v3nametest.c
index 3609eba04552ae7b372b5f822f6a46cf81ea05d5..73767abf46d382ebb6105aafe834c7c4b0e4e871 100644 (file)
--- a/test/v3nametest.c
+++ b/test/v3nametest.c
@@ -157,7 +157,8 @@ static int set_altname(X509 *crt, ...)
         default:
             abort();
         }
-        sk_GENERAL_NAME_push(gens, gen);
+        if (!sk_GENERAL_NAME_push(gens, gen))
+            goto out;
         gen = NULL;
     }
     if (!X509_add1_ext_i2d(crt, NID_subject_alt_name, gens, 0, 0))