diff options
author | Frederik Wedel-Heinen <frederik.wedel-heinen@dencrypt.dk> | 2024-12-21 15:15:11 +0100 |
---|---|---|
committer | Tomas Mraz <tomas@openssl.org> | 2025-01-08 11:11:00 +0100 |
commit | c626fda8a66a203d9f1435c34fcd3f7bda89d068 (patch) | |
tree | fe5219f4edf21176b559b96230cc68e9da0986e9 | |
parent | Pass functions with correct signatures to the evp_generic_fetch_xxx methods (diff) | |
download | openssl-c626fda8a66a203d9f1435c34fcd3f7bda89d068.tar.xz openssl-c626fda8a66a203d9f1435c34fcd3f7bda89d068.zip |
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)
-rw-r--r-- | apps/crl2pkcs7.c | 5 | ||||
-rw-r--r-- | apps/engine.c | 9 | ||||
-rw-r--r-- | apps/lib/names.c | 3 | ||||
-rw-r--r-- | apps/x509.c | 6 | ||||
-rw-r--r-- | crypto/conf/conf_lib.c | 3 | ||||
-rw-r--r-- | crypto/ocsp/ocsp_ext.c | 3 | ||||
-rw-r--r-- | engines/e_capi.c | 5 | ||||
-rw-r--r-- | fuzz/x509.c | 7 | ||||
-rw-r--r-- | test/cmp_client_test.c | 7 | ||||
-rw-r--r-- | test/ct_test.c | 6 | ||||
-rw-r--r-- | test/v3nametest.c | 3 |
11 files changed, 39 insertions, 18 deletions
diff --git a/apps/crl2pkcs7.c b/apps/crl2pkcs7.c index 681c60285f..ba24d31d5f 100644 --- 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 c3e8e4a27b..b539ec51db 100644 --- 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 716130c71e..4c4a26f1a5 100644 --- 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 cd5b7bf796..cecb8c6909 100644 --- 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 2a1c992eb2..f546744837 100644 --- 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 9707ccb94f..99507f854d 100644 --- 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 ffc5bf7a2a..5f49bcc804 100644 --- 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 e2d2639164..ce28e80728 100644 --- 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 208e0a1767..bacdac35c5 100644 --- 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 ff253414f8..b54d7d0fc4 100644 --- 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 3609eba045..73767abf46 100644 --- 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)) |