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>

diff --git a/crypto/asn1/a_object.c b/crypto/asn1/a_object.c
index ca73a281f5579b224e1dfab15b7161bcc525c94a..84db02551e2d50fd3df25725a397d40d79ba95ef 100644 (file)
--- a/crypto/asn1/a_object.c
+++ b/crypto/asn1/a_object.c
@@ -287,17 +287,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)))
                        {
@@ -320,23 +332,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 e515160d72e1807169c2e5f3f54b41872babbb88..bf89bc7b161f5e4cef69fcb6d131d7460042e113 100644 (file)
--- 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);
        }
-