Reorganise -subj option code, fix buffer overrun.

3 files changed, 32 insertions, 133 deletions

diff --git a/apps/apps.h b/apps/apps.h
index a05ba712be..5b3836ab22 100644
--- a/apps/apps.h
+++ b/apps/apps.h
@@ -253,6 +253,8 @@ int unpack_revinfo(ASN1_TIME **prevtm, int *preason, ASN1_OBJECT **phold,
 			ASN1_GENERALIZEDTIME **pinvtm, char *str);
 int make_serial_index(TXT_DB *db);
 
+X509_NAME *do_subject(char *str, long chtype);
+
 #define FORMAT_UNDEF    0
 #define FORMAT_ASN1     1
 #define FORMAT_TEXT     2
diff --git a/apps/ca.c b/apps/ca.c
index 297e3a2dfd..0b3c8a4397 100644
--- a/apps/ca.c
+++ b/apps/ca.c
@@ -238,7 +238,6 @@ static int do_body(X509 **xret, EVP_PKEY *pkey, X509 *x509, const EVP_MD *dgst,
        	int verbose, X509_REQ *req, char *ext_sect, CONF *conf,
 	unsigned long certopt, unsigned long nameopt, int default_op,
 	int ext_copy);
-static X509_NAME *do_subject(char *subject);
 static int do_revoke(X509 *x509, TXT_DB *db, int ext, char *extval);
 static int get_certificate_status(const char *ser_status, TXT_DB *db);
 static int do_updatedb(TXT_DB *db);
@@ -1879,7 +1878,7 @@ static int do_body(X509 **xret, EVP_PKEY *pkey, X509 *x509, const EVP_MD *dgst,
 
 	if (subj)
 		{
-		X509_NAME *n = do_subject(subj);
+		X509_NAME *n = do_subject(subj, MBSTRING_ASC);
 
 		if (!n)
 			{
@@ -3027,13 +3026,13 @@ int make_revoked(X509_REVOKED *rev, char *str)
  * subject is expected to be in the format /type0=value0/type1=value1/type2=...
  * where characters may be escaped by \
  */
-static X509_NAME *do_subject(char *subject)
+X509_NAME *do_subject(char *subject, long chtype)
 	{
-	size_t buflen = strlen (subject)+1; /* to copy the types and values into. due to escaping, the copy can only become shorter */
-	char *buf = malloc (buflen);
+	size_t buflen = strlen(subject)+1; /* to copy the types and values into. due to escaping, the copy can only become shorter */
+	char *buf = OPENSSL_malloc(buflen);
 	size_t max_ne = buflen / 2 + 1; /* maximum number of name elements */
-	char **ne_types = malloc (max_ne * sizeof (char *));
-	char **ne_values = malloc (max_ne * sizeof (char *));
+	char **ne_types = OPENSSL_malloc(max_ne * sizeof (char *));
+	char **ne_values = OPENSSL_malloc(max_ne * sizeof (char *));
 
 	char *sp = subject, *bp = buf;
 	int i, ne_num = 0;
@@ -3044,13 +3043,13 @@ static X509_NAME *do_subject(char *subject)
 	if (!buf || !ne_types || !ne_values)
 	{
 		BIO_printf(bio_err, "malloc error\n");
-		goto error0;
+		goto error;
 	}
 
 	if (*subject != '/')
 	{
 		BIO_printf(bio_err, "Subject does not start with '/'.\n");
-		goto error0;
+		goto error;
 	}
 	sp++; /* skip leading / */
 
@@ -3066,7 +3065,7 @@ static X509_NAME *do_subject(char *subject)
 				else
 				{
 					BIO_printf(bio_err, "escape character at end of string\n");
-					goto error0;
+					goto error;
 				}
 			else if (*sp == '=')
 			{
@@ -3080,7 +3079,7 @@ static X509_NAME *do_subject(char *subject)
 		if (!*sp)
 		{
 			BIO_printf(bio_err, "end of string encountered while processing type of subject name element #%d\n", ne_num);
-			goto error0;
+			goto error;
 		}
 		ne_values[ne_num] = bp;
 		while (*sp)
@@ -3091,12 +3090,11 @@ static X509_NAME *do_subject(char *subject)
 				else
 				{
 					BIO_printf(bio_err, "escape character at end of string\n");
-					goto error0;
+					goto error;
 				}
 			else if (*sp == '/')
 			{
 				sp++;
-				*bp++ = '\0';
 				break;
 			}
 			else
@@ -3107,7 +3105,7 @@ static X509_NAME *do_subject(char *subject)
 	}
 
 	if (!(n = X509_NAME_new()))
-		goto error0;
+		goto error;
 
 	for (i = 0; i < ne_num; i++)
 		{
@@ -3123,25 +3121,26 @@ static X509_NAME *do_subject(char *subject)
 			continue;
 			}
 
-		if (!X509_NAME_add_entry_by_NID(n, nid, MBSTRING_ASC, (unsigned char*)ne_values[i], -1,-1,0))
-			goto error1;
+		if (!X509_NAME_add_entry_by_NID(n, nid, chtype, (unsigned char*)ne_values[i], -1,-1,0))
+			goto error;
 		}
 
-	free (ne_values);
-	free (ne_types);
-	free (buf);
+	OPENSSL_free(ne_values);
+	OPENSSL_free(ne_types);
+	OPENSSL_free(buf);
 	return n;
 
-error1:
+error:
 	X509_NAME_free(n);
-error0:
-	free (ne_values);
-	free (ne_types);
-	free (buf);
+	if (ne_values)
+		OPENSSL_free(ne_values);
+	if (ne_types)
+		OPENSSL_free(ne_types);
+	if (buf)
+		OPENSSL_free(buf);
 	return NULL;
 }
 
-
 int old_entry_print(BIO *bp, ASN1_OBJECT *obj, ASN1_STRING *str)
 	{
 	char buf[25],*pbuf, *p;
diff --git a/apps/req.c b/apps/req.c
index db3dcb80e6..c1575fe5e9 100644
--- a/apps/req.c
+++ b/apps/req.c
@@ -1217,120 +1217,18 @@ err:
  */
 static int build_subject(X509_REQ *req, char *subject, unsigned long chtype)
 	{
-	size_t buflen = strlen (subject)+1; /* to copy the types and values into. due to escaping, the copy can only become shorter */
-	char *buf = malloc (buflen);
-	size_t max_ne = buflen / 2 + 1; /* maximum number of name elements */
-	char **ne_types = malloc (max_ne * sizeof (char *));
-	char **ne_values = malloc (max_ne * sizeof (char *));
+	X509_NAME *n;
 
-	char *sp = subject, *bp = buf;
-	int i, ne_num = 0;
-
-	X509_NAME *n = NULL;
-	int nid;
-
-	if (!buf || !ne_types || !ne_values)
-	{
-		BIO_printf(bio_err, "malloc error\n");
-		goto error0;
-	}
-
-	if (*subject != '/')
-	{
-		BIO_printf(bio_err, "Subject does not start with '/'.\n");
-		goto error0;
-	}
-	sp++; /* skip leading / */
-
-	while (*sp)
-	{
-		/* collect type */
-		ne_types[ne_num] = bp;
-		while (*sp)
-		{
-			if (*sp == '\\') /* is there anything to escape in the type...? */
-				if (*++sp)
-					*bp++ = *sp++;
-				else
-				{
-					BIO_printf(bio_err, "escape character at end of string\n");
-					goto error0;
-				}
-			else if (*sp == '=')
-			{
-				sp++;
-				*bp++ = '\0';
-				break;
-			}
-			else
-				*bp++ = *sp++;
-		}
-		if (!*sp)
-		{
-			BIO_printf(bio_err, "end of string encountered while processing type of subject name element #%d\n", ne_num);
-			goto error0;
-		}
-		ne_values[ne_num] = bp;
-		while (*sp)
-		{
-			if (*sp == '\\')
-				if (*++sp)
-					*bp++ = *sp++;
-				else
-				{
-					BIO_printf(bio_err, "escape character at end of string\n");
-					goto error0;
-				}
-			else if (*sp == '/')
-			{
-				sp++;
-				*bp++ = '\0';
-				break;
-			}
-			else
-				*bp++ = *sp++;
-		}
-		*bp++ = '\0';
-		ne_num++;
-	}
-
-	if (!(n = X509_NAME_new()))
-		goto error0;
+	if (!(n = do_subject(subject, chtype)))
+		return 0;
 
-	for(i = 0; i < ne_num; i++)
+	if (!X509_REQ_set_subject_name(req, n))
 		{
-		if ((nid=OBJ_txt2nid(ne_types[i])) == NID_undef)
-			{
-			BIO_printf(bio_err, "Subject Attribute %s has no known NID, skipped\n", ne_types[i]);
-			continue;
-			}
-
-		if (!*ne_values[i])
-			{
-			BIO_printf(bio_err, "No value provided for Subject Attribute %s, skipped\n", ne_types[i]);
-			continue;
-			}
-
-		if (!X509_NAME_add_entry_by_NID(n, nid, chtype, (unsigned char*)ne_values[i], -1,-1,0))
-			goto error1;
-
+		X509_NAME_free(n);
+		return 0;
 		}
-
-	if (!X509_REQ_set_subject_name(req, n))
-		goto error1;
 	X509_NAME_free(n);
-	free (ne_values);
-	free (ne_types);
-	free (buf);
 	return 1;
-
-error1:
-	X509_NAME_free(n);
-error0:
-	free (ne_values);
-	free (ne_types);
-	free (buf);
-	return 0;
 }