From 85218251e0b479212b2e2c267e504d095975edae Mon Sep 17 00:00:00 2001
From: tb <>
Date: Fri, 29 Mar 2024 04:50:11 +0000
Subject: Make X509_VERIFY_PARAM_set1_policies() less bad

If any OBJ_dup() fails along the way, a partially copied policy stack
would remain on the params object. This makes no sense. Implement and
use an sk_ASN1_OBJECT_deep_copy(), that copies the full stack or else
returns NULL.

Remove unnecessary NULL check and streamline some other logic.

ok jsing
---
 src/lib/libcrypto/x509/x509_vpm.c | 56 ++++++++++++++++++++++++---------------
 1 file changed, 34 insertions(+), 22 deletions(-)

diff --git a/src/lib/libcrypto/x509/x509_vpm.c b/src/lib/libcrypto/x509/x509_vpm.c
index 62d9215b4c..927c79716e 100644
--- a/src/lib/libcrypto/x509/x509_vpm.c
+++ b/src/lib/libcrypto/x509/x509_vpm.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509_vpm.c,v 1.44 2024/03/29 04:45:15 tb Exp $ */
+/* $OpenBSD: x509_vpm.c,v 1.45 2024/03/29 04:50:11 tb Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project 2004.
  */
@@ -474,37 +474,49 @@ X509_VERIFY_PARAM_add0_policy(X509_VERIFY_PARAM *param, ASN1_OBJECT *policy)
 }
 LCRYPTO_ALIAS(X509_VERIFY_PARAM_add0_policy);
 
+static STACK_OF(ASN1_OBJECT) *
+sk_ASN1_OBJECT_deep_copy(const STACK_OF(ASN1_OBJECT) *sk)
+{
+	STACK_OF(ASN1_OBJECT) *objs;
+	ASN1_OBJECT *obj = NULL;
+	int i;
+
+	if ((objs = sk_ASN1_OBJECT_new_null()) == NULL)
+		goto err;
+
+	for (i = 0; i < sk_ASN1_OBJECT_num(sk); i++) {
+		if ((obj = OBJ_dup(sk_ASN1_OBJECT_value(sk, i))) == NULL)
+			goto err;
+		if (sk_ASN1_OBJECT_push(objs, obj) <= 0)
+			goto err;
+		obj = NULL;
+	}
+
+	return objs;
+
+ err:
+	sk_ASN1_OBJECT_pop_free(objs, ASN1_OBJECT_free);
+	ASN1_OBJECT_free(obj);
+
+	return NULL;
+}
+
 int
 X509_VERIFY_PARAM_set1_policies(X509_VERIFY_PARAM *param,
     STACK_OF(ASN1_OBJECT) *policies)
 {
-	int i;
-	ASN1_OBJECT *oid, *doid;
-
-	if (!param)
+	if (param == NULL)
 		return 0;
-	if (param->policies)
-		sk_ASN1_OBJECT_pop_free(param->policies, ASN1_OBJECT_free);
 
-	if (!policies) {
-		param->policies = NULL;
+	sk_ASN1_OBJECT_pop_free(param->policies, ASN1_OBJECT_free);
+	param->policies = NULL;
+
+	if (policies == NULL)
 		return 1;
-	}
 
-	param->policies = sk_ASN1_OBJECT_new_null();
-	if (!param->policies)
+	if ((param->policies = sk_ASN1_OBJECT_deep_copy(policies)) == NULL)
 		return 0;
 
-	for (i = 0; i < sk_ASN1_OBJECT_num(policies); i++) {
-		oid = sk_ASN1_OBJECT_value(policies, i);
-		doid = OBJ_dup(oid);
-		if (!doid)
-			return 0;
-		if (!sk_ASN1_OBJECT_push(param->policies, doid)) {
-			ASN1_OBJECT_free(doid);
-			return 0;
-		}
-	}
 	return 1;
 }
 LCRYPTO_ALIAS(X509_VERIFY_PARAM_set1_policies);
-- 
cgit v1.2.3-55-g6feb