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 863aa6a..2cb5a00 100644 (file)
--- 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 071febb..15c298e 100644 (file)
--- 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 863aa6a..2cb5a00 100644 (file)
--- 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 071febb..15c298e 100644 (file)
--- 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++;