CMS_get1_{certs,crls}(): make sure they return NULL only on error

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18916)

diff --git a/crypto/cms/cms_lib.c b/crypto/cms/cms_lib.c
index 759e1d46af4cc1dd8ef5c2f3f974e89aafa707e5..b09985ad3e27fe58effbfbb3c47250b520cc919f 100644 (file)
--- a/crypto/cms/cms_lib.c
+++ b/crypto/cms/cms_lib.c
@@ -622,12 +622,18 @@ STACK_OF(X509) *CMS_get1_certs(CMS_ContentInfo *cms)
     STACK_OF(X509) *certs = NULL;
     CMS_CertificateChoices *cch;
     STACK_OF(CMS_CertificateChoices) **pcerts;
-    int i;
+    int i, n;
 
     pcerts = cms_get0_certificate_choices(cms);
     if (pcerts == NULL)
         return NULL;
-    for (i = 0; i < sk_CMS_CertificateChoices_num(*pcerts); i++) {
+
+    /* make sure to return NULL only on error */
+    n = sk_CMS_CertificateChoices_num(*pcerts);
+    if ((certs = sk_X509_new_reserve(NULL, n)) == NULL)
+        return NULL;
+
+    for (i = 0; i < n; i++) {
         cch = sk_CMS_CertificateChoices_value(*pcerts, i);
         if (cch->type == 0) {
             if (!ossl_x509_add_cert_new(&certs, cch->d.certificate,
@@ -638,7 +644,6 @@ STACK_OF(X509) *CMS_get1_certs(CMS_ContentInfo *cms)
         }
     }
     return certs;
-
 }
 
 STACK_OF(X509_CRL) *CMS_get1_crls(CMS_ContentInfo *cms)
@@ -646,12 +651,18 @@ STACK_OF(X509_CRL) *CMS_get1_crls(CMS_ContentInfo *cms)
     STACK_OF(X509_CRL) *crls = NULL;
     STACK_OF(CMS_RevocationInfoChoice) **pcrls;
     CMS_RevocationInfoChoice *rch;
-    int i;
+    int i, n;
 
     pcrls = cms_get0_revocation_choices(cms);
     if (pcrls == NULL)
         return NULL;
-    for (i = 0; i < sk_CMS_RevocationInfoChoice_num(*pcrls); i++) {
+
+    /* make sure to return NULL only on error */
+    n = sk_CMS_RevocationInfoChoice_num(*pcrls);
+    if ((crls = sk_X509_CRL_new_reserve(NULL, n)) == NULL)
+        return NULL;
+
+    for (i = 0; i < n; i++) {
         rch = sk_CMS_RevocationInfoChoice_value(*pcrls, i);
         if (rch->type == 0) {
             if (crls == NULL) {

diff --git a/doc/man3/CMS_add0_cert.pod b/doc/man3/CMS_add0_cert.pod
index c876238fe4e53481f362d6d76c8d2fa8b5f75a0d..8f61813f6fc1c4356b17bc50fa88ab1a2688a24c 100644 (file)
--- a/doc/man3/CMS_add0_cert.pod
+++ b/doc/man3/CMS_add0_cert.pod
@@ -57,8 +57,9 @@ For enveloped data they are added to B<OriginatorInfo>.
 CMS_add0_cert(), CMS_add1_cert() and CMS_add0_crl() and CMS_add1_crl() return
 1 for success and 0 for failure.
 
-CMS_get1_certs() and CMS_get1_crls() return the STACK of certificates or CRLs
-or NULL if there are none or an error occurs. The only error which will occur
+CMS_get1_certs() and CMS_get1_crls() return the STACK of certificates or CRLs,
+which is empty if there are none. They return NULL on error.
+Besides out-of-memory, the only error which will occur
 in practice is if the I<cms> type is invalid.
 
 =head1 SEE ALSO