Skip to content

Commit 0042fb5

Browse files
ekaspermattcaswell
authored andcommitted
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>
1 parent 1716003 commit 0042fb5

File tree

2 files changed

+30
-16
lines changed

2 files changed

+30
-16
lines changed

Diff for: crypto/asn1/a_object.c

+21-9
Original file line numberDiff line numberDiff line change
@@ -287,17 +287,29 @@ ASN1_OBJECT *d2i_ASN1_OBJECT(ASN1_OBJECT **a, const unsigned char **pp,
287287
ASN1err(ASN1_F_D2I_ASN1_OBJECT,i);
288288
return(NULL);
289289
}
290+
290291
ASN1_OBJECT *c2i_ASN1_OBJECT(ASN1_OBJECT **a, const unsigned char **pp,
291292
long len)
292293
{
293294
ASN1_OBJECT *ret=NULL;
294295
const unsigned char *p;
295296
unsigned char *data;
296-
int i;
297-
/* Sanity check OID encoding: can't have leading 0x80 in
298-
* subidentifiers, see: X.690 8.19.2
297+
int i, length;
298+
299+
/* Sanity check OID encoding.
300+
* Need at least one content octet.
301+
* MSB must be clear in the last octet.
302+
* can't have leading 0x80 in subidentifiers, see: X.690 8.19.2
299303
*/
300-
for (i = 0, p = *pp; i < len; i++, p++)
304+
if (len <= 0 || len > INT_MAX || pp == NULL || (p = *pp) == NULL ||
305+
p[len - 1] & 0x80)
306+
{
307+
ASN1err(ASN1_F_C2I_ASN1_OBJECT,ASN1_R_INVALID_OBJECT_ENCODING);
308+
return NULL;
309+
}
310+
/* Now 0 < len <= INT_MAX, so the cast is safe. */
311+
length = (int)len;
312+
for (i = 0; i < length; i++, p++)
301313
{
302314
if (*p == 0x80 && (!i || !(p[-1] & 0x80)))
303315
{
@@ -320,23 +332,23 @@ ASN1_OBJECT *c2i_ASN1_OBJECT(ASN1_OBJECT **a, const unsigned char **pp,
320332
data = (unsigned char *)ret->data;
321333
ret->data = NULL;
322334
/* once detached we can change it */
323-
if ((data == NULL) || (ret->length < len))
335+
if ((data == NULL) || (ret->length < length))
324336
{
325337
ret->length=0;
326338
if (data != NULL) OPENSSL_free(data);
327-
data=(unsigned char *)OPENSSL_malloc(len ? (int)len : 1);
339+
data=(unsigned char *)OPENSSL_malloc(length);
328340
if (data == NULL)
329341
{ i=ERR_R_MALLOC_FAILURE; goto err; }
330342
ret->flags|=ASN1_OBJECT_FLAG_DYNAMIC_DATA;
331343
}
332-
memcpy(data,p,(int)len);
344+
memcpy(data,p,length);
333345
/* reattach data to object, after which it remains const */
334346
ret->data =data;
335-
ret->length=(int)len;
347+
ret->length=length;
336348
ret->sn=NULL;
337349
ret->ln=NULL;
338350
/* ret->flags=ASN1_OBJECT_FLAG_DYNAMIC; we know it is dynamic */
339-
p+=len;
351+
p+=length;
340352

341353
if (a != NULL) (*a)=ret;
342354
*pp=p;

Diff for: crypto/objects/obj_dat.c

+9-7
Original file line numberDiff line numberDiff line change
@@ -471,11 +471,12 @@ int OBJ_obj2txt(char *buf, int buf_len, const ASN1_OBJECT *a, int no_name)
471471
const unsigned char *p;
472472
char tbuf[DECIMAL_SIZE(i)+DECIMAL_SIZE(l)+2];
473473

474-
if ((a == NULL) || (a->data == NULL)) {
475-
buf[0]='\0';
476-
return(0);
477-
}
474+
/* Ensure that, at every state, |buf| is NUL-terminated. */
475+
if (buf && buf_len > 0)
476+
buf[0] = '\0';
478477

478+
if ((a == NULL) || (a->data == NULL))
479+
return(0);
479480

480481
if (!no_name && (nid=OBJ_obj2nid(a)) != NID_undef)
481482
{
@@ -554,9 +555,10 @@ int OBJ_obj2txt(char *buf, int buf_len, const ASN1_OBJECT *a, int no_name)
554555
i=(int)(l/40);
555556
l-=(long)(i*40);
556557
}
557-
if (buf && (buf_len > 0))
558+
if (buf && (buf_len > 1))
558559
{
559560
*buf++ = i + '0';
561+
*buf = '\0';
560562
buf_len--;
561563
}
562564
n++;
@@ -571,9 +573,10 @@ int OBJ_obj2txt(char *buf, int buf_len, const ASN1_OBJECT *a, int no_name)
571573
i = strlen(bndec);
572574
if (buf)
573575
{
574-
if (buf_len > 0)
576+
if (buf_len > 1)
575577
{
576578
*buf++ = '.';
579+
*buf = '\0';
577580
buf_len--;
578581
}
579582
BUF_strlcpy(buf,bndec,buf_len);
@@ -807,4 +810,3 @@ int OBJ_create(const char *oid, const char *sn, const char *ln)
807810
OPENSSL_free(buf);
808811
return(ok);
809812
}
810-

0 commit comments

Comments
 (0)