From: tb <tb@openbsd.org>
Date: Sun, 24 Jul 2022 18:51:16 +0000 (+0000)
Subject: Minor fixes in PKCS12_parse()
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=fc53f9bee7b68eb849097ed33c2d5ec6f97ee517;p=openbsd

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
---

diff --git a/lib/libcrypto/pkcs12/p12_kiss.c b/lib/libcrypto/pkcs12/p12_kiss.c
index 1e221f4436c..6bbfa2aeefc 100644
--- a/lib/libcrypto/pkcs12/p12_kiss.c
+++ b/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;
 }