Minor fixes in PKCS12_parse()

Pull up clearing of output parameters before first return
(OpenSSL 524fdd51 by Bernd Edlinger), explicit comparisons
against NULL, '\0', etc.

ok jsing

1 files changed, 23 insertions, 24 deletions

diff --git a/src/lib/libcrypto/pkcs12/p12_kiss.c b/src/lib/libcrypto/pkcs12/p12_kiss.c
index 1e221f4436..6bbfa2aeef 100644
--- a/src/lib/libcrypto/pkcs12/p12_kiss.c
+++ b/src/lib/libcrypto/pkcs12/p12_kiss.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: p12_kiss.c,v 1.22 2022/05/20 08:00:05 tb Exp $ */
+/* $OpenBSD: p12_kiss.c,v 1.23 2022/07/24 18:51:16 tb Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project 1999.
  */
@@ -84,18 +84,17 @@ PKCS12_parse(PKCS12 *p12, const char *pass, EVP_PKEY **pkey, X509 **cert,
 {
         STACK_OF(X509) *ocerts = NULL;
         X509 *x = NULL;
-        /* Check for NULL PKCS12 structure */
 
-        if (!p12) {
-                PKCS12error(PKCS12_R_INVALID_NULL_PKCS12_POINTER);
-                return 0;
-        }
-
-        if (pkey)
+        if (pkey != NULL)
                 *pkey = NULL;
-        if (cert)
+        if (cert != NULL)
                 *cert = NULL;
 
+        if (p12 == NULL) {
+                PKCS12error(PKCS12_R_INVALID_NULL_PKCS12_POINTER);
+                goto err;
+        }
+
         /* Check the mac */
 
         /* If password is zero length or NULL then try verifying both cases
@@ -104,7 +103,7 @@ PKCS12_parse(PKCS12 *p12, const char *pass, EVP_PKEY **pkey, X509 **cert,
          * password are two different things...
          */
 
-        if (!pass || !*pass) {
+        if (pass == NULL || *pass == '\0') {
                 if (PKCS12_verify_mac(p12, NULL, 0))
                         pass = NULL;
                 else if (PKCS12_verify_mac(p12, "", 0))
@@ -119,10 +118,9 @@ PKCS12_parse(PKCS12 *p12, const char *pass, EVP_PKEY **pkey, X509 **cert,
         }
 
         /* Allocate stack for other certificates */
-        ocerts = sk_X509_new_null();
-        if (!ocerts) {
+        if ((ocerts = sk_X509_new_null()) == NULL) {
                 PKCS12error(ERR_R_MALLOC_FAILURE);
-                return 0;
+                goto err;
         }
 
         if (!parse_pk12(p12, pass, -1, pkey, ocerts)) {
@@ -130,8 +128,9 @@ PKCS12_parse(PKCS12 *p12, const char *pass, EVP_PKEY **pkey, X509 **cert,
                 goto err;
         }
 
-        while ((x = sk_X509_pop(ocerts))) {
-                if (pkey && *pkey && cert && !*cert) {
+        while ((x = sk_X509_pop(ocerts)) != NULL) {
+                if (pkey != NULL && *pkey != NULL &&
+                    cert != NULL && *cert == NULL) {
                         ERR_set_mark();
                         if (X509_check_private_key(x, *pkey)) {
                                 *cert = x;
@@ -140,31 +139,31 @@ PKCS12_parse(PKCS12 *p12, const char *pass, EVP_PKEY **pkey, X509 **cert,
                         ERR_pop_to_mark();
                 }
 
-                if (ca && x) {
-                        if (!*ca)
+                if (ca != NULL && x != NULL) {
+                        if (*ca == NULL)
                                 *ca = sk_X509_new_null();
-                        if (!*ca)
+                        if (*ca == NULL)
                                 goto err;
                         if (!sk_X509_push(*ca, x))
                                 goto err;
                         x = NULL;
                 }
                 X509_free(x);
+                x = NULL;
         }
 
-        if (ocerts)
-                sk_X509_pop_free(ocerts, X509_free);
+        sk_X509_pop_free(ocerts, X509_free);
 
         return 1;
 
 err:
-        if (pkey && *pkey)
+        if (pkey != NULL)
                 EVP_PKEY_free(*pkey);
-        if (cert)
+        if (cert != NULL)
                 X509_free(*cert);
         X509_free(x);
-        if (ocerts)
-                sk_X509_pop_free(ocerts, X509_free);
+        sk_X509_pop_free(ocerts, X509_free);
+
         return 0;
 }