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@

1 files changed, 21 insertions, 10 deletions

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;