From: tb <tb@openbsd.org>
Date: Wed, 11 Oct 2023 13:20:18 +0000 (+0000)
Subject: Rewrite X509_ALGOR_set0()
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=97fce2b4783e9e53a0d63d7f4b49c2bc54e5491f;p=openbsd

Rewrite X509_ALGOR_set0()

The current implementation is a complete mess. There are three cases:
1) ptype == V_ASN1_UNDEF: parameter must be freed and set to NULL.
2) ptype == 0: existing non-NULL parameters are left untouched, NULL
   parameters are replaced with ASN1_TYPE_new()'s wacky defaults.
3) otherwise allocate new parameters if needed and set them to ptype/pval.
In all three cases free the algorithm and set it to aobj.

The challenge now is to implement this using nine if statements and one
else clause... We can do better. This preserves existing behavior. There
would be cleaner implementations possible, but they would change behavior.

There are many callers in the ecosystem that do not error check
X509_ALGOR_set0() since OpenSSL failed to do so. So this was carefully
rewritten to leave alg in a consisten state so that unchecking callers
don't encounter corrupted algs.

ok jsing
---

diff --git a/lib/libcrypto/asn1/x_algor.c b/lib/libcrypto/asn1/x_algor.c
index 08742c5f1c9..74d123535b0 100644
--- a/lib/libcrypto/asn1/x_algor.c
+++ b/lib/libcrypto/asn1/x_algor.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: x_algor.c,v 1.29 2023/10/11 13:12:46 tb Exp $ */
+/* $OpenBSD: x_algor.c,v 1.30 2023/10/11 13:20:18 tb Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project 2000.
  */
@@ -150,28 +150,24 @@ X509_ALGOR_dup(X509_ALGOR *x)
 int
 X509_ALGOR_set0(X509_ALGOR *alg, ASN1_OBJECT *aobj, int ptype, void *pval)
 {
-	if (!alg)
+	if (alg == NULL)
 		return 0;
-	if (ptype != V_ASN1_UNDEF) {
+
+	if (ptype == V_ASN1_UNDEF) {
+		ASN1_TYPE_free(alg->parameter);
+		alg->parameter = NULL;
+	} else {
 		if (alg->parameter == NULL)
 			alg->parameter = ASN1_TYPE_new();
 		if (alg->parameter == NULL)
 			return 0;
+		if (ptype != 0)
+			ASN1_TYPE_set(alg->parameter, ptype, pval);
 	}
-	if (alg) {
-		if (alg->algorithm)
-			ASN1_OBJECT_free(alg->algorithm);
-		alg->algorithm = aobj;
-	}
-	if (ptype == 0)
-		return 1;
-	if (ptype == V_ASN1_UNDEF) {
-		if (alg->parameter) {
-			ASN1_TYPE_free(alg->parameter);
-			alg->parameter = NULL;
-		}
-	} else
-		ASN1_TYPE_set(alg->parameter, ptype, pval);
+
+	ASN1_OBJECT_free(alg->algorithm);
+	alg->algorithm = aobj;
+
 	return 1;
 }