From 0042fb5fd1c9d257d713b15a1f45da05cf5c1c87 Mon Sep 17 00:00:00 2001 From: Emilia Kasper Date: Wed, 2 Jul 2014 19:02:33 +0200 Subject: [PATCH] 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 Reviewed-by: Kurt Roeckx Reviewed-by: Tim Hudson --- crypto/asn1/a_object.c | 30 +++++++++++++++++++++--------- crypto/objects/obj_dat.c | 16 +++++++++------- 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/crypto/asn1/a_object.c b/crypto/asn1/a_object.c index ca73a281f5..84db02551e 100644 --- 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 e515160d72..bf89bc7b16 100644 --- 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); } - -- 2.25.1