From fca39bf77338b32d5264edaa28a2d4ffd38b333f Mon Sep 17 00:00:00 2001 From: guenther Date: Fri, 8 Aug 2014 04:53:43 +0000 Subject: [PATCH] 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@ --- lib/libcrypto/asn1/a_object.c | 31 +++++++++++++++++-------- lib/libcrypto/objects/obj_dat.c | 9 +++++-- lib/libssl/src/crypto/asn1/a_object.c | 31 +++++++++++++++++-------- lib/libssl/src/crypto/objects/obj_dat.c | 9 +++++-- 4 files changed, 56 insertions(+), 24 deletions(-) diff --git a/lib/libcrypto/asn1/a_object.c b/lib/libcrypto/asn1/a_object.c index 863aa6a226a..2cb5a0097b0 100644 --- 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 071febba524..15c298e3330 100644 --- 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 863aa6a226a..2cb5a0097b0 100644 --- 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 071febba524..15c298e3330 100644 --- 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++; -- 2.20.1