From 03b04ddac162c7b7fa3c57eadccc5a583a00d291 Mon Sep 17 00:00:00 2001

From: Emilia Kasper <emilia@openssl.org>

Date: Wed, 2 Jul 2014 19:02:33 +0200

Subject: [PATCH] Fix OID handling:

- Upon parsing, reject OIDs with invalid base-128 encoding.

- Always NUL-terminate the destination buffer in OBJ_obj2txt printing function.

CVE-2014-3508

Reviewed-by: Dr. Stephen Henson <steve@openssl.org>

Reviewed-by: Kurt Roeckx <kurt@openssl.org>

Reviewed-by: Tim Hudson <tjh@openssl.org>

---

 crypto/asn1/a_object.c   | 30 +++++++++++++++++++++---------

 crypto/objects/obj_dat.c | 16 +++++++++-------

 2 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/crypto/asn1/a_object.c b/crypto/asn1/a_object.c

index 3978c91..77b2768 100644

--- a/crypto/asn1/a_object.c

+++ b/crypto/asn1/a_object.c

@@ -283,17 +283,29 @@ err:

 	ASN1err(ASN1_F_D2I_ASN1_OBJECT,i);

 	return(NULL);

 }

+

 ASN1_OBJECT *c2i_ASN1_OBJECT(ASN1_OBJECT **a, const unsigned char **pp,

 	     long len)

 	{

 	ASN1_OBJECT *ret=NULL;

 	const unsigned char *p;

 	unsigned char *data;

-	int i;

-	/* Sanity check OID encoding: can't have leading 0x80 in

-	 * subidentifiers, see: X.690 8.19.2

+	int i, length;

+

+	/* Sanity check OID encoding.

+	 * Need at least one content octet.

+	 * MSB must be clear in the last octet.

+	 * can't have leading 0x80 in subidentifiers, see: X.690 8.19.2

 	 */

-	for (i = 0, p = *pp; i < len; i++, p++)

+	if (len <= 0 || len > INT_MAX || pp == NULL || (p = *pp) == NULL ||

+	    p[len - 1] & 0x80)

+		{

+		ASN1err(ASN1_F_C2I_ASN1_OBJECT,ASN1_R_INVALID_OBJECT_ENCODING);

+		return NULL;

+		}

+	/* Now 0 < len <= INT_MAX, so the cast is safe. */

+	length = (int)len;

+	for (i = 0; i < length; i++, p++)

 		{

 		if (*p == 0x80 && (!i || !(p[-1] & 0x80)))

 			{

@@ -316,23 +328,23 @@ ASN1_OBJECT *c2i_ASN1_OBJECT(ASN1_OBJECT **a, const unsigned char **pp,

 	data = (unsigned char *)ret->data;

 	ret->data = NULL;

 	/* once detached we can change it */

-	if ((data == NULL) || (ret->length < len))

+	if ((data == NULL) || (ret->length < length))

 		{

 		ret->length=0;

 		if (data != NULL) OPENSSL_free(data);

-		data=(unsigned char *)OPENSSL_malloc(len ? (int)len : 1);

+		data=(unsigned char *)OPENSSL_malloc(length);

 		if (data == NULL)

 			{ i=ERR_R_MALLOC_FAILURE; goto err; }

 		ret->flags|=ASN1_OBJECT_FLAG_DYNAMIC_DATA;

 		}

-	memcpy(data,p,(int)len);

+	memcpy(data,p,length);

 	/* reattach data to object, after which it remains const */

 	ret->data  =data;

-	ret->length=(int)len;

+	ret->length=length;

 	ret->sn=NULL;

 	ret->ln=NULL;

 	/* ret->flags=ASN1_OBJECT_FLAG_DYNAMIC; we know it is dynamic */

-	p+=len;

+	p+=length;

 	if (a != NULL) (*a)=ret;

 	*pp=p;

diff --git a/crypto/objects/obj_dat.c b/crypto/objects/obj_dat.c

index 8a342ba..0b2f442 100644

--- a/crypto/objects/obj_dat.c

+++ b/crypto/objects/obj_dat.c

@@ -471,11 +471,12 @@ int OBJ_obj2txt(char *buf, int buf_len, const ASN1_OBJECT *a, int no_name)

 	const unsigned char *p;

 	char tbuf[DECIMAL_SIZE(i)+DECIMAL_SIZE(l)+2];

-	if ((a == NULL) || (a->data == NULL)) {

-		buf[0]='\0';

-		return(0);

-	}

+	/* Ensure that, at every state, |buf| is NUL-terminated. */

+	if (buf && buf_len > 0)

+		buf[0] = '\0';

+	if ((a == NULL) || (a->data == NULL))

+		return(0);

 	if (!no_name && (nid=OBJ_obj2nid(a)) != NID_undef)

 		{

@@ -554,9 +555,10 @@ int OBJ_obj2txt(char *buf, int buf_len, const ASN1_OBJECT *a, int no_name)

 				i=(int)(l/40);

 				l-=(long)(i*40);

 				}

-			if (buf && (buf_len > 0))

+			if (buf && (buf_len > 1))

 				{

 				*buf++ = i + '0';

+				*buf = '\0';

 				buf_len--;

 				}

 			n++;

@@ -571,9 +573,10 @@ int OBJ_obj2txt(char *buf, int buf_len, const ASN1_OBJECT *a, int no_name)

 			i = strlen(bndec);

 			if (buf)

 				{

-				if (buf_len > 0)

+				if (buf_len > 1)

 					{

 					*buf++ = '.';

+					*buf = '\0';

 					buf_len--;

 					}

 				BUF_strlcpy(buf,bndec,buf_len);

@@ -807,4 +810,3 @@ err:

 	OPENSSL_free(buf);

 	return(ok);

 	}

-

-- 

1.8.3.1