From: guenther <guenther@openbsd.org>
Date: Fri, 8 Aug 2014 04:53:43 +0000 (+0000)
Subject: Fix CVE-2014-3508, pretty printing and OID validation:
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=fca39bf77338b32d5264edaa28a2d4ffd38b333f;p=openbsd

Fix CVE-2014-3508, pretty printing and OID validation:
 - make sure the output buffer is always NUL terminated if buf_len
   was initially greater than zero.
 - reject OIDs that are too long, too short, or not in proper base-127

Based on
https://git.openssl.org/gitweb/?p=openssl.git;a=commit;h=0042fb5fd1c9d257d713b15a1f45da05cf5c1c87

ok bcook@
---

diff --git a/lib/libcrypto/asn1/a_object.c b/lib/libcrypto/asn1/a_object.c
index 863aa6a226a..2cb5a0097b0 100644
--- a/lib/libcrypto/asn1/a_object.c
+++ b/lib/libcrypto/asn1/a_object.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: a_object.c,v 1.22 2014/07/12 16:03:36 miod Exp $ */
+/* $OpenBSD: a_object.c,v 1.23 2014/08/08 04:53:43 guenther Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -281,12 +281,23 @@ 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;
+	int i, length;
 
-	/* Sanity check OID encoding: can't have leading 0x80 in
-	 * subidentifiers, see: X.690 8.19.2
+	/*
+	 * 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))) {
 			ASN1err(ASN1_F_C2I_ASN1_OBJECT,
 			    ASN1_R_INVALID_OBJECT_ENCODING);
@@ -308,24 +319,24 @@ c2i_ASN1_OBJECT(ASN1_OBJECT **a, const unsigned char **pp, long len)
 	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;
 		free(data);
-		data = malloc(len ? len : 1);
+		data = malloc(length);
 		if (data == NULL) {
 			i = ERR_R_MALLOC_FAILURE;
 			goto err;
 		}
 		ret->flags |= ASN1_OBJECT_FLAG_DYNAMIC_DATA;
 	}
-	memcpy(data, p, 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;
diff --git a/lib/libcrypto/objects/obj_dat.c b/lib/libcrypto/objects/obj_dat.c
index 071febba524..15c298e3330 100644
--- a/lib/libcrypto/objects/obj_dat.c
+++ b/lib/libcrypto/objects/obj_dat.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: obj_dat.c,v 1.30 2014/07/11 08:44:49 jsing Exp $ */
+/* $OpenBSD: obj_dat.c,v 1.31 2014/08/08 04:53:43 guenther Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -495,6 +495,10 @@ OBJ_obj2txt(char *buf, int buf_len, const ASN1_OBJECT *a, int no_name)
 	unsigned long l;
 	const unsigned char *p;
 
+	/* Ensure that, at every state, |buf| is NUL-terminated. */
+	if (buf_len > 0)
+		buf[0] = '\0';
+
 	if ((a == NULL) || (a->data == NULL))
 		goto err;
 
@@ -554,8 +558,9 @@ OBJ_obj2txt(char *buf, int buf_len, const ASN1_OBJECT *a, int no_name)
 				i = (int)(l / 40);
 				l -= (long)(i * 40);
 			}
-			if (buf_len > 0) {
+			if (buf_len > 1) {
 				*buf++ = i + '0';
+				*buf = '\0';
 				buf_len--;
 			}
 			ret++;
diff --git a/lib/libssl/src/crypto/asn1/a_object.c b/lib/libssl/src/crypto/asn1/a_object.c
index 863aa6a226a..2cb5a0097b0 100644
--- a/lib/libssl/src/crypto/asn1/a_object.c
+++ b/lib/libssl/src/crypto/asn1/a_object.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: a_object.c,v 1.22 2014/07/12 16:03:36 miod Exp $ */
+/* $OpenBSD: a_object.c,v 1.23 2014/08/08 04:53:43 guenther Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -281,12 +281,23 @@ 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;
+	int i, length;
 
-	/* Sanity check OID encoding: can't have leading 0x80 in
-	 * subidentifiers, see: X.690 8.19.2
+	/*
+	 * 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))) {
 			ASN1err(ASN1_F_C2I_ASN1_OBJECT,
 			    ASN1_R_INVALID_OBJECT_ENCODING);
@@ -308,24 +319,24 @@ c2i_ASN1_OBJECT(ASN1_OBJECT **a, const unsigned char **pp, long len)
 	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;
 		free(data);
-		data = malloc(len ? len : 1);
+		data = malloc(length);
 		if (data == NULL) {
 			i = ERR_R_MALLOC_FAILURE;
 			goto err;
 		}
 		ret->flags |= ASN1_OBJECT_FLAG_DYNAMIC_DATA;
 	}
-	memcpy(data, p, 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;
diff --git a/lib/libssl/src/crypto/objects/obj_dat.c b/lib/libssl/src/crypto/objects/obj_dat.c
index 071febba524..15c298e3330 100644
--- a/lib/libssl/src/crypto/objects/obj_dat.c
+++ b/lib/libssl/src/crypto/objects/obj_dat.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: obj_dat.c,v 1.30 2014/07/11 08:44:49 jsing Exp $ */
+/* $OpenBSD: obj_dat.c,v 1.31 2014/08/08 04:53:43 guenther Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -495,6 +495,10 @@ OBJ_obj2txt(char *buf, int buf_len, const ASN1_OBJECT *a, int no_name)
 	unsigned long l;
 	const unsigned char *p;
 
+	/* Ensure that, at every state, |buf| is NUL-terminated. */
+	if (buf_len > 0)
+		buf[0] = '\0';
+
 	if ((a == NULL) || (a->data == NULL))
 		goto err;
 
@@ -554,8 +558,9 @@ OBJ_obj2txt(char *buf, int buf_len, const ASN1_OBJECT *a, int no_name)
 				i = (int)(l / 40);
 				l -= (long)(i * 40);
 			}
-			if (buf_len > 0) {
+			if (buf_len > 1) {
 				*buf++ = i + '0';
+				*buf = '\0';
 				buf_len--;
 			}
 			ret++;