From e0d29ce4ca3a66fb79a3bdb9e13b5c6ea1e19604 Mon Sep 17 00:00:00 2001
From: doug <>
Date: Sat, 6 Dec 2014 19:26:37 +0000
Subject: Avoid modifying input on failure in X509_(TRUST|PURPOSE)_add.

If X509_TRUST_add() or X509_PURPOSE_add() fail, they will leave the
object in an inconsistent state since the name is already freed.
This commit avoids changing the original name unless the *_add() call
will succeed.

Based on BoringSSL's commit: ab2815eaff6219ef57aedca2f7b1b72333c27fd0

ok miod@
---
 src/lib/libcrypto/x509/x509_trs.c          | 14 ++++++++------
 src/lib/libcrypto/x509v3/v3_purp.c         | 24 +++++++++++++-----------
 src/lib/libssl/src/crypto/x509/x509_trs.c  | 14 ++++++++------
 src/lib/libssl/src/crypto/x509v3/v3_purp.c | 24 +++++++++++++-----------
 4 files changed, 42 insertions(+), 34 deletions(-)

(limited to 'src/lib')

diff --git a/src/lib/libcrypto/x509/x509_trs.c b/src/lib/libcrypto/x509/x509_trs.c
index 95fb568c68..4fa9f81ee7 100644
--- a/src/lib/libcrypto/x509/x509_trs.c
+++ b/src/lib/libcrypto/x509/x509_trs.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509_trs.c,v 1.18 2014/11/18 03:28:05 tedu Exp $ */
+/* $OpenBSD: x509_trs.c,v 1.19 2014/12/06 19:26:37 doug Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project 1999.
  */
@@ -177,6 +177,7 @@ X509_TRUST_add(int id, int flags, int (*ck)(X509_TRUST *, X509 *, int),
 {
 	int idx;
 	X509_TRUST *trtmp;
+	char *name_dup;
 
 	/* This is set according to what we change: application can't set it */
 	flags &= ~X509_TRUST_DYNAMIC;
@@ -199,12 +200,14 @@ X509_TRUST_add(int id, int flags, int (*ck)(X509_TRUST *, X509 *, int),
 		}
 	}
 
+	if ((name_dup = strdup(name)) == NULL)
+		goto err;
+
 	/* free existing name if dynamic */
 	if (trtmp->flags & X509_TRUST_DYNAMIC_NAME)
 		free(trtmp->name);
 	/* dup supplied name */
-	if ((trtmp->name = strdup(name)) == NULL)
-		goto err;
+	trtmp->name = name_dup;
 	/* Keep the dynamic flag of existing entry */
 	trtmp->flags &= X509_TRUST_DYNAMIC;
 	/* Set all other flags */
@@ -226,10 +229,9 @@ X509_TRUST_add(int id, int flags, int (*ck)(X509_TRUST *, X509 *, int),
 	return 1;
 
 err:
-	if (idx == -1) {
-		free(trtmp->name);
+	free(name_dup);
+	if (idx == -1)
 		free(trtmp);
-	}
 	X509err(X509_F_X509_TRUST_ADD, ERR_R_MALLOC_FAILURE);
 	return 0;
 }
diff --git a/src/lib/libcrypto/x509v3/v3_purp.c b/src/lib/libcrypto/x509v3/v3_purp.c
index 1a073e368e..b020f87a0f 100644
--- a/src/lib/libcrypto/x509v3/v3_purp.c
+++ b/src/lib/libcrypto/x509v3/v3_purp.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: v3_purp.c,v 1.23 2014/10/05 18:33:57 miod Exp $ */
+/* $OpenBSD: v3_purp.c,v 1.24 2014/12/06 19:26:37 doug Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project 2001.
  */
@@ -203,6 +203,9 @@ X509_PURPOSE_add(int id, int trust, int flags,
 {
 	int idx;
 	X509_PURPOSE *ptmp;
+	char *name_dup, *sname_dup;
+
+	name_dup = sname_dup = NULL;
 
 	if (name == NULL || sname == NULL) {
 		X509V3err(X509V3_F_X509_PURPOSE_ADD,
@@ -227,16 +230,19 @@ X509_PURPOSE_add(int id, int trust, int flags,
 	} else
 		ptmp = X509_PURPOSE_get0(idx);
 
+	if ((name_dup = strdup(name)) == NULL)
+		goto err;
+	if ((sname_dup = strdup(sname)) == NULL)
+		goto err;
+
 	/* free existing name if dynamic */
 	if (ptmp->flags & X509_PURPOSE_DYNAMIC_NAME) {
 		free(ptmp->name);
 		free(ptmp->sname);
 	}
 	/* dup supplied name */
-	ptmp->name = strdup(name);
-	ptmp->sname = strdup(sname);
-	if (ptmp->name == NULL || ptmp->sname == NULL)
-		goto err;
+	ptmp->name = name_dup;
+	ptmp->sname = sname_dup;
 	/* Keep the dynamic flag of existing entry */
 	ptmp->flags &= X509_PURPOSE_DYNAMIC;
 	/* Set all other flags */
@@ -258,14 +264,10 @@ X509_PURPOSE_add(int id, int trust, int flags,
 	return 1;
 
 err:
-	free(ptmp->name);
-	free(ptmp->sname);
+	free(name_dup);
+	free(sname_dup);
 	if (idx == -1)
 		free(ptmp);
-	else {
-		ptmp->name = NULL;
-		ptmp->sname = NULL;
-	}
 	X509V3err(X509V3_F_X509_PURPOSE_ADD, ERR_R_MALLOC_FAILURE);
 	return 0;
 }
diff --git a/src/lib/libssl/src/crypto/x509/x509_trs.c b/src/lib/libssl/src/crypto/x509/x509_trs.c
index 95fb568c68..4fa9f81ee7 100644
--- a/src/lib/libssl/src/crypto/x509/x509_trs.c
+++ b/src/lib/libssl/src/crypto/x509/x509_trs.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509_trs.c,v 1.18 2014/11/18 03:28:05 tedu Exp $ */
+/* $OpenBSD: x509_trs.c,v 1.19 2014/12/06 19:26:37 doug Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project 1999.
  */
@@ -177,6 +177,7 @@ X509_TRUST_add(int id, int flags, int (*ck)(X509_TRUST *, X509 *, int),
 {
 	int idx;
 	X509_TRUST *trtmp;
+	char *name_dup;
 
 	/* This is set according to what we change: application can't set it */
 	flags &= ~X509_TRUST_DYNAMIC;
@@ -199,12 +200,14 @@ X509_TRUST_add(int id, int flags, int (*ck)(X509_TRUST *, X509 *, int),
 		}
 	}
 
+	if ((name_dup = strdup(name)) == NULL)
+		goto err;
+
 	/* free existing name if dynamic */
 	if (trtmp->flags & X509_TRUST_DYNAMIC_NAME)
 		free(trtmp->name);
 	/* dup supplied name */
-	if ((trtmp->name = strdup(name)) == NULL)
-		goto err;
+	trtmp->name = name_dup;
 	/* Keep the dynamic flag of existing entry */
 	trtmp->flags &= X509_TRUST_DYNAMIC;
 	/* Set all other flags */
@@ -226,10 +229,9 @@ X509_TRUST_add(int id, int flags, int (*ck)(X509_TRUST *, X509 *, int),
 	return 1;
 
 err:
-	if (idx == -1) {
-		free(trtmp->name);
+	free(name_dup);
+	if (idx == -1)
 		free(trtmp);
-	}
 	X509err(X509_F_X509_TRUST_ADD, ERR_R_MALLOC_FAILURE);
 	return 0;
 }
diff --git a/src/lib/libssl/src/crypto/x509v3/v3_purp.c b/src/lib/libssl/src/crypto/x509v3/v3_purp.c
index 1a073e368e..b020f87a0f 100644
--- a/src/lib/libssl/src/crypto/x509v3/v3_purp.c
+++ b/src/lib/libssl/src/crypto/x509v3/v3_purp.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: v3_purp.c,v 1.23 2014/10/05 18:33:57 miod Exp $ */
+/* $OpenBSD: v3_purp.c,v 1.24 2014/12/06 19:26:37 doug Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project 2001.
  */
@@ -203,6 +203,9 @@ X509_PURPOSE_add(int id, int trust, int flags,
 {
 	int idx;
 	X509_PURPOSE *ptmp;
+	char *name_dup, *sname_dup;
+
+	name_dup = sname_dup = NULL;
 
 	if (name == NULL || sname == NULL) {
 		X509V3err(X509V3_F_X509_PURPOSE_ADD,
@@ -227,16 +230,19 @@ X509_PURPOSE_add(int id, int trust, int flags,
 	} else
 		ptmp = X509_PURPOSE_get0(idx);
 
+	if ((name_dup = strdup(name)) == NULL)
+		goto err;
+	if ((sname_dup = strdup(sname)) == NULL)
+		goto err;
+
 	/* free existing name if dynamic */
 	if (ptmp->flags & X509_PURPOSE_DYNAMIC_NAME) {
 		free(ptmp->name);
 		free(ptmp->sname);
 	}
 	/* dup supplied name */
-	ptmp->name = strdup(name);
-	ptmp->sname = strdup(sname);
-	if (ptmp->name == NULL || ptmp->sname == NULL)
-		goto err;
+	ptmp->name = name_dup;
+	ptmp->sname = sname_dup;
 	/* Keep the dynamic flag of existing entry */
 	ptmp->flags &= X509_PURPOSE_DYNAMIC;
 	/* Set all other flags */
@@ -258,14 +264,10 @@ X509_PURPOSE_add(int id, int trust, int flags,
 	return 1;
 
 err:
-	free(ptmp->name);
-	free(ptmp->sname);
+	free(name_dup);
+	free(sname_dup);
 	if (idx == -1)
 		free(ptmp);
-	else {
-		ptmp->name = NULL;
-		ptmp->sname = NULL;
-	}
 	X509V3err(X509V3_F_X509_PURPOSE_ADD, ERR_R_MALLOC_FAILURE);
 	return 0;
 }
-- 
cgit v1.2.3-55-g6feb