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@

4 files changed, 56 insertions, 24 deletions

diff --git a/src/lib/libcrypto/asn1/a_object.c b/src/lib/libcrypto/asn1/a_object.c
index 863aa6a226..2cb5a0097b 100644
--- a/src/lib/libcrypto/asn1/a_object.c
+++ b/src/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/src/lib/libcrypto/objects/obj_dat.c b/src/lib/libcrypto/objects/obj_dat.c
index 071febba52..15c298e333 100644
--- a/src/lib/libcrypto/objects/obj_dat.c
+++ b/src/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/src/lib/libssl/src/crypto/asn1/a_object.c b/src/lib/libssl/src/crypto/asn1/a_object.c
index 863aa6a226..2cb5a0097b 100644
--- a/src/lib/libssl/src/crypto/asn1/a_object.c
+++ b/src/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/src/lib/libssl/src/crypto/objects/obj_dat.c b/src/lib/libssl/src/crypto/objects/obj_dat.c
index 071febba52..15c298e333 100644
--- a/src/lib/libssl/src/crypto/objects/obj_dat.c
+++ b/src/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++;