Add error checking to i2v_AUTHORITY_KEYID(), i2v_GENERAL_NAME()

and i2v_GENERAL_NAMES().  This fixes a couple of leaks and other
ugliness.

tweaks & ok jsing

2 files changed, 80 insertions, 32 deletions

diff --git a/lib/libcrypto/x509v3/v3_akey.c b/lib/libcrypto/x509v3/v3_akey.c
index 3b57fd21f70..65184b19b60 100644
--- a/lib/libcrypto/x509v3/v3_akey.c
+++ b/lib/libcrypto/x509v3/v3_akey.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: v3_akey.c,v 1.20 2019/04/21 08:07:47 tb Exp $ */
+/* $OpenBSD: v3_akey.c,v 1.21 2019/04/21 16:50:34 tb Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project 1999.
  */
@@ -91,22 +91,41 @@ static STACK_OF(CONF_VALUE) *
 i2v_AUTHORITY_KEYID(X509V3_EXT_METHOD *method, AUTHORITY_KEYID *akeyid,
     STACK_OF(CONF_VALUE) *extlist)
 {
-	char *tmp;
+	char *tmpstr = NULL;
 
-	if (akeyid->keyid) {
-		tmp = hex_to_string(akeyid->keyid->data, akeyid->keyid->length);
-		X509V3_add_value("keyid", tmp, &extlist);
-		free(tmp);
+	if (akeyid->keyid != NULL) {
+		if ((tmpstr = hex_to_string(akeyid->keyid->data,
+		    akeyid->keyid->length)) == NULL)
+			goto err;
+		if (!X509V3_add_value("keyid", tmpstr, &extlist))
+			goto err;
+		free(tmpstr);
+		tmpstr = NULL;
 	}
-	if (akeyid->issuer)
-		extlist = i2v_GENERAL_NAMES(NULL, akeyid->issuer, extlist);
-	if (akeyid->serial) {
-		tmp = hex_to_string(akeyid->serial->data,
-		    akeyid->serial->length);
-		X509V3_add_value("serial", tmp, &extlist);
-		free(tmp);
+
+	if (akeyid->issuer != NULL) {
+		if ((extlist = i2v_GENERAL_NAMES(NULL, akeyid->issuer,
+		    extlist)) == NULL)
+			goto err;
 	}
+
+	if (akeyid->serial != NULL) {
+		if ((tmpstr = hex_to_string(akeyid->serial->data,
+		    akeyid->serial->length)) == NULL)
+			goto err;
+		if (!X509V3_add_value("serial", tmpstr, &extlist))
+			goto err;
+		free(tmpstr);
+		tmpstr = NULL;
+	}
+
 	return extlist;
+
+ err:
+	free(tmpstr);
+	sk_CONF_VALUE_pop_free(extlist, X509V3_conf_free);
+
+	return NULL;
 }
 
 /*
diff --git a/lib/libcrypto/x509v3/v3_alt.c b/lib/libcrypto/x509v3/v3_alt.c
index 08063d191b0..2dc07b4025f 100644
--- a/lib/libcrypto/x509v3/v3_alt.c
+++ b/lib/libcrypto/x509v3/v3_alt.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: v3_alt.c,v 1.28 2018/05/18 19:34:37 tb Exp $ */
+/* $OpenBSD: v3_alt.c,v 1.29 2019/04/21 16:50:34 tb Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project.
  */
@@ -127,16 +127,27 @@ STACK_OF(CONF_VALUE) *
 i2v_GENERAL_NAMES(X509V3_EXT_METHOD *method, GENERAL_NAMES *gens,
     STACK_OF(CONF_VALUE) *ret)
 {
-	int i;
 	GENERAL_NAME *gen;
+	int i;
+
+	if (ret == NULL) {
+		if ((ret = sk_CONF_VALUE_new_null()) == NULL)
+			return NULL;
+	}
 
 	for (i = 0; i < sk_GENERAL_NAME_num(gens); i++) {
-		gen = sk_GENERAL_NAME_value(gens, i);
-		ret = i2v_GENERAL_NAME(method, gen, ret);
+		if ((gen = sk_GENERAL_NAME_value(gens, i)) == NULL)
+			goto err;
+		if ((ret = i2v_GENERAL_NAME(method, gen, ret)) == NULL)
+			goto err;
 	}
-	if (!ret)
-		return sk_CONF_VALUE_new_null();
+
 	return ret;
+
+ err:
+	sk_CONF_VALUE_pop_free(ret, X509V3_conf_free);
+
+	return NULL;
 }
 
 STACK_OF(CONF_VALUE) *
@@ -149,35 +160,43 @@ i2v_GENERAL_NAME(X509V3_EXT_METHOD *method, GENERAL_NAME *gen,
 
 	switch (gen->type) {
 	case GEN_OTHERNAME:
-		X509V3_add_value("othername", "<unsupported>", &ret);
+		if (!X509V3_add_value("othername", "<unsupported>", &ret))
+			goto err;
 		break;
 
 	case GEN_X400:
-		X509V3_add_value("X400Name", "<unsupported>", &ret);
+		if (!X509V3_add_value("X400Name", "<unsupported>", &ret))
+			goto err;
 		break;
 
 	case GEN_EDIPARTY:
-		X509V3_add_value("EdiPartyName", "<unsupported>", &ret);
+		if (!X509V3_add_value("EdiPartyName", "<unsupported>", &ret))
+			goto err;
 		break;
 
 	case GEN_EMAIL:
-		X509V3_add_value_uchar("email", gen->d.ia5->data, &ret);
+		if (!X509V3_add_value_uchar("email", gen->d.ia5->data, &ret))
+			goto err;
 		break;
 
 	case GEN_DNS:
-		X509V3_add_value_uchar("DNS", gen->d.ia5->data, &ret);
+		if (!X509V3_add_value_uchar("DNS", gen->d.ia5->data, &ret))
+			goto err;
 		break;
 
 	case GEN_URI:
-		X509V3_add_value_uchar("URI", gen->d.ia5->data, &ret);
+		if (!X509V3_add_value_uchar("URI", gen->d.ia5->data, &ret))
+			goto err;
 		break;
 
 	case GEN_DIRNAME:
-		X509_NAME_oneline(gen->d.dirn, oline, 256);
-		X509V3_add_value("DirName", oline, &ret);
+		if (X509_NAME_oneline(gen->d.dirn, oline, 256) == NULL)
+			goto err;
+		if (!X509V3_add_value("DirName", oline, &ret))
+			goto err;
 		break;
 
-	case GEN_IPADD:
+	case GEN_IPADD: /* XXX */
 		p = gen->d.ip->data;
 		if (gen->d.ip->length == 4)
 			(void) snprintf(oline, sizeof oline,
@@ -193,18 +212,28 @@ i2v_GENERAL_NAME(X509V3_EXT_METHOD *method, GENERAL_NAME *gen,
 					strlcat(oline, ":", sizeof(oline));
 			}
 		} else {
-			X509V3_add_value("IP Address", "<invalid>", &ret);
+			if (!X509V3_add_value("IP Address", "<invalid>", &ret))
+				goto err;
 			break;
 		}
-		X509V3_add_value("IP Address", oline, &ret);
+		if (!X509V3_add_value("IP Address", oline, &ret))
+			goto err;
 		break;
 
 	case GEN_RID:
-		i2t_ASN1_OBJECT(oline, 256, gen->d.rid);
-		X509V3_add_value("Registered ID", oline, &ret);
+		if (!i2t_ASN1_OBJECT(oline, 256, gen->d.rid))
+			goto err;
+		if (!X509V3_add_value("Registered ID", oline, &ret))
+			goto err;
 		break;
 	}
+
 	return ret;
+
+ err:
+	sk_CONF_VALUE_pop_free(ret, X509V3_conf_free);
+
+	return NULL;
 }
 
 int