Rewrite & fix X509V3_add_value()

X509V3_add_value() helpfully allocates a STACK_OF(CONF_VALUE) if it
receives a pointer to a NULL pointer.  If anything fails along the way,
it is however the caller's responsibility to free it.  This can easily
be fixed by freeing *extlist in the error path and zeroing it to avoid
a double free if there happens to be a caller out there that avoids
the leak.

Polish a few things so the function conforms a bit better to our usual
style.

tweak & ok jsing

1 files changed, 24 insertions, 17 deletions

diff --git a/src/lib/libcrypto/x509v3/v3_utl.c b/src/lib/libcrypto/x509v3/v3_utl.c
index c01edf4e3c..a051baae62 100644
--- a/src/lib/libcrypto/x509v3/v3_utl.c
+++ b/src/lib/libcrypto/x509v3/v3_utl.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: v3_utl.c,v 1.36 2019/04/16 19:34:15 tb Exp $ */
+/* $OpenBSD: v3_utl.c,v 1.37 2019/04/16 19:42:20 tb Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project.
  */
@@ -79,35 +79,42 @@ static int ipv6_from_asc(unsigned char *v6, const char *in);
 static int ipv6_cb(const char *elem, int len, void *usr);
 static int ipv6_hex(unsigned char *out, const char *in, int inlen);
 
-/* Add a CONF_VALUE name value pair to stack */
-
+/* Add a CONF_VALUE name-value pair to stack. */
 int
 X509V3_add_value(const char *name, const char *value,
     STACK_OF(CONF_VALUE) **extlist)
 {
         CONF_VALUE *vtmp = NULL;
-        char *tname = NULL, *tvalue = NULL;
+        STACK_OF(CONF_VALUE) *free_exts = NULL;
 
-        if (name && !(tname = strdup(name)))
-                goto err;
-        if (value && !(tvalue = strdup(value)))
-                goto err;
-        if (!(vtmp = malloc(sizeof(CONF_VALUE))))
-                goto err;
-        if (!*extlist && !(*extlist = sk_CONF_VALUE_new_null()))
+        if ((vtmp = calloc(1, sizeof(CONF_VALUE))) == NULL)
                 goto err;
-        vtmp->section = NULL;
-        vtmp->name = tname;
-        vtmp->value = tvalue;
+        if (name != NULL) {
+                if ((vtmp->name = strdup(name)) == NULL)
+                        goto err;
+        }
+        if (value != NULL) {
+                if ((vtmp->value = strdup(value)) == NULL)
+                        goto err;
+        }
+
+        if (*extlist == NULL) {
+                if ((free_exts = *extlist = sk_CONF_VALUE_new_null()) == NULL)
+                        goto err;
+        }
+
         if (!sk_CONF_VALUE_push(*extlist, vtmp))
                 goto err;
+
         return 1;
 
  err:
         X509V3error(ERR_R_MALLOC_FAILURE);
-        free(vtmp);
-        free(tname);
-        free(tvalue);
+        X509V3_conf_free(vtmp);
+        if (free_exts != NULL) {
+                sk_CONF_VALUE_free(*extlist);
+                *extlist = NULL;
+        }
         return 0;
 }