This adds return value check for sk_X509_push.
In error case, allocated memories are freed at 'export_end:'.
After this diff, I would like to add more another return value check.

ok?

Index: pkcs12.c
===================================================================
RCS file: /cvs/src/usr.bin/openssl/pkcs12.c,v
retrieving revision 1.20
diff -u -p -u -p -r1.20 pkcs12.c
--- pkcs12.c    28 Apr 2022 15:42:10 -0000      1.20
+++ pkcs12.c    28 Apr 2022 18:59:39 -0000
@@ -613,6 +613,9 @@ pkcs12_main(int argc, char **argv)
                EVP_PKEY *key = NULL;
                X509 *ucert = NULL, *x = NULL;
                STACK_OF(X509) *certs = NULL;
+               STACK_OF(X509) *morecerts = NULL;
+               STACK_OF(X509) *chain2 = NULL;
+               X509_STORE *store = NULL;
                const EVP_MD *macmd = NULL;
                unsigned char *catmp = NULL;
                int i;
@@ -664,23 +667,24 @@ pkcs12_main(int argc, char **argv)
 
                /* Add any more certificates asked for */
                if (pkcs12_config.certfile != NULL) {
-                       STACK_OF(X509) *morecerts = NULL;
                        if ((morecerts = load_certs(bio_err,
                            pkcs12_config.certfile, FORMAT_PEM, NULL,
                            "certificates from certfile")) == NULL)
                                goto export_end;
-                       while (sk_X509_num(morecerts) > 0)
-                               sk_X509_push(certs, sk_X509_shift(morecerts));
+                       while (sk_X509_num(morecerts) > 0) {
+                               if (!sk_X509_push(certs, 
sk_X509_shift(morecerts)))
+                                       goto export_end;
+                       }
                        sk_X509_free(morecerts);
+                       morecerts = NULL;
                }
 
 
                /* If chaining get chain from user cert */
                if (pkcs12_config.chain) {
                        int vret;
-                       STACK_OF(X509) *chain2;
-                       X509_STORE *store = X509_STORE_new();
-                       if (store == NULL) {
+
+                       if ((store = X509_STORE_new()) == NULL) {
                                BIO_printf(bio_err,
                                    "Memory allocation error\n");
                                goto export_end;
@@ -691,15 +695,19 @@ pkcs12_main(int argc, char **argv)
 
                        vret = get_cert_chain(ucert, store, &chain2);
                        X509_STORE_free(store);
+                       store = NULL;
 
                        if (vret == X509_V_OK) {
                                /* Exclude verified certificate */
-                               for (i = 1; i < sk_X509_num(chain2); i++)
-                                       sk_X509_push(certs, sk_X509_value(
-                                           chain2, i));
+                               for (i = 1; i < sk_X509_num(chain2); i++) {
+                                       if (!sk_X509_push(certs, sk_X509_value(
+                                           chain2, i)))
+                                               goto export_end;
+                               }
                                /* Free first certificate */
                                X509_free(sk_X509_value(chain2, 0));
                                sk_X509_free(chain2);
+                               chain2 = NULL;
                        } else {
                                if (vret != X509_V_ERR_UNSPECIFIED)
                                        BIO_printf(bio_err,
@@ -765,8 +773,11 @@ pkcs12_main(int argc, char **argv)
 
  export_end:
                EVP_PKEY_free(key);
-               sk_X509_pop_free(certs, X509_free);
                X509_free(ucert);
+               sk_X509_pop_free(certs, X509_free);
+               sk_X509_pop_free(morecerts, X509_free);
+               sk_X509_pop_free(chain2, X509_free);
+               X509_STORE_free(store);
 
                goto end;
 

Reply via email to