From 86d2018c2e3e51ab867799e1591a4740c7d7f7cd Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Fri, 14 Oct 2022 16:45:37 +1300 Subject: [PATCH 1/8] CVE-2022-42898 third_party/heimdal: Avoid potential overflows in krb5_pac_add_buffer() Catch overflows that result from adding PAC_INFO_BUFFER_SIZE. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15203 Signed-off-by: Joseph Sutton --- third_party/heimdal/lib/krb5/pac.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/third_party/heimdal/lib/krb5/pac.c b/third_party/heimdal/lib/krb5/pac.c index c8f355c8179..d4a0719daf2 100644 --- a/third_party/heimdal/lib/krb5/pac.c +++ b/third_party/heimdal/lib/krb5/pac.c @@ -394,9 +394,21 @@ krb5_pac_add_buffer(krb5_context context, krb5_pac p, p->pac = ptr; - for (i = 0; i < len; i++) - p->pac->buffers[i].offset_lo += PAC_INFO_BUFFER_SIZE; + for (i = 0; i < len; i++) { + if (p->pac->buffers[i].offset_lo > UINT32_MAX - PAC_INFO_BUFFER_SIZE) { + ret = EINVAL; + krb5_set_error_message(context, ret, "integer overrun"); + return ret; + } + p->pac->buffers[i].offset_lo += PAC_INFO_BUFFER_SIZE; + } + + if (p->data.length > UINT32_MAX - PAC_INFO_BUFFER_SIZE) { + ret = EINVAL; + krb5_set_error_message(context, ret, "integer overrun"); + return ret; + } offset = p->data.length + PAC_INFO_BUFFER_SIZE; p->pac->buffers[len].type = type; -- 2.35.0 From 1a1c5c26dc722d75360907687b23451a8b9606bd Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Mon, 17 Oct 2022 19:07:42 +1300 Subject: [PATCH 2/8] CVE-2022-42898 third_party/heimdal: Resize offset and size variables We don't support PACs with offsets larger than 32 bits (4 GiB). This can be made clearer by restricting ourselves to 32-bit types. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15203 Signed-off-by: Joseph Sutton --- third_party/heimdal/lib/krb5/pac.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/third_party/heimdal/lib/krb5/pac.c b/third_party/heimdal/lib/krb5/pac.c index d4a0719daf2..9b84f0f84b9 100644 --- a/third_party/heimdal/lib/krb5/pac.c +++ b/third_party/heimdal/lib/krb5/pac.c @@ -380,7 +380,7 @@ krb5_pac_add_buffer(krb5_context context, krb5_pac p, { krb5_error_code ret; void *ptr; - size_t len, offset, header_end, old_end; + uint32_t len, offset, header_end, old_end; uint32_t i; assert(data->data != NULL); @@ -477,8 +477,8 @@ krb5_pac_get_buffer(krb5_context context, krb5_const_pac p, uint32_t i; for (i = 0; i < p->pac->numbuffers; i++) { - const size_t len = p->pac->buffers[i].buffersize; - const size_t offset = p->pac->buffers[i].offset_lo; + const uint32_t len = p->pac->buffers[i].buffersize; + const uint32_t offset = p->pac->buffers[i].offset_lo; if (p->pac->buffers[i].type != type) continue; @@ -1295,8 +1295,8 @@ _krb5_pac_sign(krb5_context context, size_t server_size, priv_size; uint32_t server_offset = 0, priv_offset = 0, ticket_offset = 0; uint32_t server_cksumtype = 0, priv_cksumtype = 0; - int num = 0; - size_t i, sz; + uint32_t num = 0; + uint32_t i, sz; krb5_data logon, d; krb5_data_zero(&d); -- 2.35.0 From 475161110b8ef35012c00e8e385e07291802d9a5 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Mon, 17 Oct 2022 19:05:15 +1300 Subject: [PATCH 3/8] CVE-2022-42898 third_party/heimdal: Avoid integer overflow when calculating PAC header size On 32-bit systems, or after converting integer types to uint32_t, the multiplication of buffer count by size can overflow. Introduce a helper function that prevents this from occurring. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15203 Signed-off-by: Joseph Sutton --- third_party/heimdal/lib/krb5/pac.c | 63 ++++++++++++++++++++++++++---- 1 file changed, 55 insertions(+), 8 deletions(-) diff --git a/third_party/heimdal/lib/krb5/pac.c b/third_party/heimdal/lib/krb5/pac.c index 9b84f0f84b9..2452ee797e4 100644 --- a/third_party/heimdal/lib/krb5/pac.c +++ b/third_party/heimdal/lib/krb5/pac.c @@ -133,6 +133,34 @@ struct heim_type_data pac_object = { NULL }; +static krb5_error_code pac_header_size(krb5_context context, + uint32_t num_buffers, + uint32_t *result) +{ + krb5_error_code ret; + uint32_t header_size; + + /* Guard against integer overflow on 32-bit systems. */ + if (num_buffers > UINT32_MAX / PAC_INFO_BUFFER_SIZE) { + ret = EINVAL; + krb5_set_error_message(context, ret, "PAC has too many buffers"); + return ret; + } + header_size = PAC_INFO_BUFFER_SIZE * num_buffers; + + /* Guard against integer overflow on 32-bit systems. */ + if (header_size > UINT32_MAX - PACTYPE_SIZE) { + ret = EINVAL; + krb5_set_error_message(context, ret, "PAC has too many buffers"); + return ret; + } + header_size += PACTYPE_SIZE; + + *result = header_size; + + return 0; +} + /* * HMAC-MD5 checksum over any key (needed for the PAC routines) */ @@ -217,8 +245,12 @@ krb5_pac_parse(krb5_context context, const void *ptr, size_t len, goto out; } - p->pac = calloc(1, - sizeof(*p->pac) + (sizeof(p->pac->buffers[0]) * (tmp - 1))); + ret = pac_header_size(context, tmp, &header_end); + if (ret) { + return ret; + } + + p->pac = calloc(1, header_end); if (p->pac == NULL) { ret = krb5_enomem(context); goto out; @@ -227,7 +259,6 @@ krb5_pac_parse(krb5_context context, const void *ptr, size_t len, p->pac->numbuffers = tmp; p->pac->version = tmp2; - header_end = PACTYPE_SIZE + (PAC_INFO_BUFFER_SIZE * p->pac->numbuffers); if (header_end > len) { ret = EINVAL; goto out; @@ -387,8 +418,17 @@ krb5_pac_add_buffer(krb5_context context, krb5_pac p, len = p->pac->numbuffers; - ptr = realloc(p->pac, - sizeof(*p->pac) + (sizeof(p->pac->buffers[0]) * len)); + if (len >= UINT32_MAX) { + ret = EINVAL; + krb5_set_error_message(context, ret, "integer overrun"); + return ret; + } + ret = pac_header_size(context, len + 1, &header_end); + if (ret) { + return ret; + } + + ptr = realloc(p->pac, header_end); if (ptr == NULL) return krb5_enomem(context); @@ -435,7 +475,7 @@ krb5_pac_add_buffer(krb5_context context, krb5_pac p, /* * make place for new PAC INFO BUFFER header */ - header_end = PACTYPE_SIZE + (PAC_INFO_BUFFER_SIZE * p->pac->numbuffers); + header_end -= PAC_INFO_BUFFER_SIZE; memmove((unsigned char *)p->data.data + header_end + PAC_INFO_BUFFER_SIZE, (unsigned char *)p->data.data + header_end , old_end - header_end); @@ -1377,8 +1417,13 @@ _krb5_pac_sign(krb5_context context, if (num) { void *ptr; + uint32_t len; - ptr = realloc(p->pac, sizeof(*p->pac) + (sizeof(p->pac->buffers[0]) * (p->pac->numbuffers + num - 1))); + ret = pac_header_size(context, p->pac->numbuffers + num, &len); + if (ret) + goto out; + + ptr = realloc(p->pac, len); if (ptr == NULL) { ret = krb5_enomem(context); goto out; @@ -1440,7 +1485,9 @@ _krb5_pac_sign(krb5_context context, CHECK(ret, krb5_store_uint32(sp, p->pac->numbuffers), out); CHECK(ret, krb5_store_uint32(sp, p->pac->version), out); - end = PACTYPE_SIZE + (PAC_INFO_BUFFER_SIZE * p->pac->numbuffers); + ret = pac_header_size(context, p->pac->numbuffers, &end); + if (ret) + goto out; for (i = 0; i < p->pac->numbuffers; i++) { uint32_t len; -- 2.35.0 From 0c6d64b576aee7fc52d383a36f5e662dd3e54e95 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Mon, 17 Oct 2022 19:04:04 +1300 Subject: [PATCH 4/8] CVE-2022-42898 third_party/heimdal: Limit PAC buffers to (2^32) - 1 bytes in length BUG: https://bugzilla.samba.org/show_bug.cgi?id=15203 Signed-off-by: Joseph Sutton --- third_party/heimdal/lib/krb5/pac.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/third_party/heimdal/lib/krb5/pac.c b/third_party/heimdal/lib/krb5/pac.c index 2452ee797e4..809b142525f 100644 --- a/third_party/heimdal/lib/krb5/pac.c +++ b/third_party/heimdal/lib/krb5/pac.c @@ -416,6 +416,12 @@ krb5_pac_add_buffer(krb5_context context, krb5_pac p, assert(data->data != NULL); + if (data->length > UINT32_MAX) { + ret = EINVAL; + krb5_set_error_message(context, ret, "integer overrun"); + return ret; + } + len = p->pac->numbuffers; if (len >= UINT32_MAX) { -- 2.35.0 From 1ceee39f36f8bbe3fb5a5313829439e620e83a0a Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Mon, 17 Oct 2022 18:40:12 +1300 Subject: [PATCH 5/8] CVE-2022-42898 third_party/heimdal: Avoid integer overflows in _krb5_pac_sign() BUG: https://bugzilla.samba.org/show_bug.cgi?id=15203 Signed-off-by: Joseph Sutton --- third_party/heimdal/lib/krb5/pac.c | 54 ++++++++++++++++++++++++++++-- 1 file changed, 52 insertions(+), 2 deletions(-) diff --git a/third_party/heimdal/lib/krb5/pac.c b/third_party/heimdal/lib/krb5/pac.c index 809b142525f..2689c97b2df 100644 --- a/third_party/heimdal/lib/krb5/pac.c +++ b/third_party/heimdal/lib/krb5/pac.c @@ -1425,6 +1425,11 @@ _krb5_pac_sign(krb5_context context, void *ptr; uint32_t len; + if (p->pac->numbuffers > UINT32_MAX - num) { + ret = EINVAL; + krb5_set_error_message(context, ret, "integer overrun"); + goto out; + } ret = pac_header_size(context, p->pac->numbuffers + num, &len); if (ret) goto out; @@ -1503,26 +1508,66 @@ _krb5_pac_sign(krb5_context context, /* store data */ if (p->pac->buffers[i].type == PAC_SERVER_CHECKSUM) { + if (server_size > UINT32_MAX - 4) { + ret = EINVAL; + krb5_set_error_message(context, ret, "integer overrun"); + goto out; + } len = server_size + 4; + if (end > UINT32_MAX - 4) { + ret = EINVAL; + krb5_set_error_message(context, ret, "integer overrun"); + goto out; + } server_offset = end + 4; CHECK(ret, krb5_store_uint32(spdata, server_cksumtype), out); CHECK(ret, fill_zeros(context, spdata, server_size), out); } else if (p->pac->buffers[i].type == PAC_PRIVSVR_CHECKSUM) { + if (priv_size > UINT32_MAX - 4) { + ret = EINVAL; + krb5_set_error_message(context, ret, "integer overrun"); + goto out; + } len = priv_size + 4; + if (end > UINT32_MAX - 4) { + ret = EINVAL; + krb5_set_error_message(context, ret, "integer overrun"); + goto out; + } priv_offset = end + 4; CHECK(ret, krb5_store_uint32(spdata, priv_cksumtype), out); CHECK(ret, fill_zeros(context, spdata, priv_size), out); if (rodc_id != 0) { + if (len > UINT32_MAX - sizeof(rodc_id)) { + ret = EINVAL; + krb5_set_error_message(context, ret, "integer overrun"); + goto out; + } len += sizeof(rodc_id); CHECK(ret, fill_zeros(context, spdata, sizeof(rodc_id)), out); } } else if (p->ticket_sign_data.length != 0 && p->pac->buffers[i].type == PAC_TICKET_CHECKSUM) { + if (priv_size > UINT32_MAX - 4) { + ret = EINVAL; + krb5_set_error_message(context, ret, "integer overrun"); + goto out; + } len = priv_size + 4; + if (end > UINT32_MAX - 4) { + ret = EINVAL; + krb5_set_error_message(context, ret, "integer overrun"); + goto out; + } ticket_offset = end + 4; CHECK(ret, krb5_store_uint32(spdata, priv_cksumtype), out); CHECK(ret, fill_zeros(context, spdata, priv_size), out); if (rodc_id != 0) { + if (len > UINT32_MAX - sizeof(rodc_id)) { + ret = EINVAL; + krb5_set_error_message(context, ret, "integer overrun"); + goto out; + } len += sizeof(rodc_id); CHECK(ret, krb5_store_uint16(spdata, rodc_id), out); } @@ -1552,11 +1597,16 @@ _krb5_pac_sign(krb5_context context, /* advance data endpointer and align */ { - int32_t e; + uint32_t e; + if (end > UINT32_MAX - len) { + ret = EINVAL; + krb5_set_error_message(context, ret, "integer overrun"); + goto out; + } end += len; e = ((end + PAC_ALIGNMENT - 1) / PAC_ALIGNMENT) * PAC_ALIGNMENT; - if ((int32_t)end != e) { + if (end != e) { CHECK(ret, fill_zeros(context, spdata, e - end), out); } end = e; -- 2.35.0 From 37e24e85f4fd9d81eadd124f27b3edf747b26c69 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Mon, 17 Oct 2022 18:43:47 +1300 Subject: [PATCH 6/8] CVE-2022-42898 third_party/heimdal: Avoid integer overflow when aligning PAC buffers BUG: https://bugzilla.samba.org/show_bug.cgi?id=15203 Signed-off-by: Joseph Sutton --- third_party/heimdal/lib/krb5/pac.c | 37 +++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/third_party/heimdal/lib/krb5/pac.c b/third_party/heimdal/lib/krb5/pac.c index 2689c97b2df..3160a320de4 100644 --- a/third_party/heimdal/lib/krb5/pac.c +++ b/third_party/heimdal/lib/krb5/pac.c @@ -161,6 +161,28 @@ static krb5_error_code pac_header_size(krb5_context context, return 0; } +static krb5_error_code pac_aligned_size(krb5_context context, + uint32_t size, + uint32_t *aligned_size) +{ + krb5_error_code ret; + + /* Guard against integer overflow on 32-bit systems. */ + if (size > UINT32_MAX - (PAC_ALIGNMENT - 1)) { + ret = EINVAL; + krb5_set_error_message(context, ret, "integer overrun"); + return ret; + } + size += PAC_ALIGNMENT - 1; + + /* align to PAC_ALIGNMENT */ + size = (size / PAC_ALIGNMENT) * PAC_ALIGNMENT; + + *aligned_size = size; + + return 0; +} + /* * HMAC-MD5 checksum over any key (needed for the PAC routines) */ @@ -463,14 +485,13 @@ krb5_pac_add_buffer(krb5_context context, krb5_pac p, p->pac->buffers[len].offset_hi = 0; old_end = p->data.length; - len = p->data.length + data->length + PAC_INFO_BUFFER_SIZE; - if (len < p->data.length) { + if (offset > UINT32_MAX - data->length) { krb5_set_error_message(context, EINVAL, "integer overrun"); return EINVAL; } - - /* align to PAC_ALIGNMENT */ - len = ((len + PAC_ALIGNMENT - 1) / PAC_ALIGNMENT) * PAC_ALIGNMENT; + ret = pac_aligned_size(context, offset + data->length, &len); + if (ret) + return ret; ret = krb5_data_realloc(&p->data, len); if (ret) { @@ -1605,7 +1626,11 @@ _krb5_pac_sign(krb5_context context, goto out; } end += len; - e = ((end + PAC_ALIGNMENT - 1) / PAC_ALIGNMENT) * PAC_ALIGNMENT; + + ret = pac_aligned_size(context, end, &e); + if (ret) + goto out; + if (end != e) { CHECK(ret, fill_zeros(context, spdata, e - end), out); } -- 2.35.0 From c783365a7bda48ebf96dc278c75dc504fa0d1907 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Mon, 17 Oct 2022 18:27:16 +1300 Subject: [PATCH 7/8] CVE-2022-42898 third_party/heimdal: Introduce unaligned_len variable This is intended to clarify the code. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15203 Signed-off-by: Joseph Sutton --- third_party/heimdal/lib/krb5/pac.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/third_party/heimdal/lib/krb5/pac.c b/third_party/heimdal/lib/krb5/pac.c index 3160a320de4..0114790dbeb 100644 --- a/third_party/heimdal/lib/krb5/pac.c +++ b/third_party/heimdal/lib/krb5/pac.c @@ -435,6 +435,7 @@ krb5_pac_add_buffer(krb5_context context, krb5_pac p, void *ptr; uint32_t len, offset, header_end, old_end; uint32_t i; + uint32_t unaligned_len; assert(data->data != NULL); @@ -489,7 +490,9 @@ krb5_pac_add_buffer(krb5_context context, krb5_pac p, krb5_set_error_message(context, EINVAL, "integer overrun"); return EINVAL; } - ret = pac_aligned_size(context, offset + data->length, &len); + unaligned_len = offset + data->length; + + ret = pac_aligned_size(context, unaligned_len, &len); if (ret) return ret; @@ -515,7 +518,7 @@ krb5_pac_add_buffer(krb5_context context, krb5_pac p, memcpy((unsigned char *)p->data.data + offset, data->data, data->length); memset((unsigned char *)p->data.data + offset + data->length, - 0, p->data.length - offset - data->length); + 0, p->data.length - unaligned_len); p->pac->numbuffers += 1; -- 2.35.0 From 1742484cd41181b9430c8e902607d40e07b8f99b Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Mon, 17 Oct 2022 18:23:38 +1300 Subject: [PATCH 8/8] CVE-2022-42898 third_party/heimdal: Introduce num_buffers variable Reusing 'len' for two unrelated quantities is confusing. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15203 Signed-off-by: Joseph Sutton --- third_party/heimdal/lib/krb5/pac.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/third_party/heimdal/lib/krb5/pac.c b/third_party/heimdal/lib/krb5/pac.c index 0114790dbeb..7e1d22b7eaf 100644 --- a/third_party/heimdal/lib/krb5/pac.c +++ b/third_party/heimdal/lib/krb5/pac.c @@ -436,6 +436,7 @@ krb5_pac_add_buffer(krb5_context context, krb5_pac p, uint32_t len, offset, header_end, old_end; uint32_t i; uint32_t unaligned_len; + uint32_t num_buffers; assert(data->data != NULL); @@ -445,14 +446,14 @@ krb5_pac_add_buffer(krb5_context context, krb5_pac p, return ret; } - len = p->pac->numbuffers; + num_buffers = p->pac->numbuffers; - if (len >= UINT32_MAX) { + if (num_buffers >= UINT32_MAX) { ret = EINVAL; krb5_set_error_message(context, ret, "integer overrun"); return ret; } - ret = pac_header_size(context, len + 1, &header_end); + ret = pac_header_size(context, num_buffers + 1, &header_end); if (ret) { return ret; } @@ -463,7 +464,7 @@ krb5_pac_add_buffer(krb5_context context, krb5_pac p, p->pac = ptr; - for (i = 0; i < len; i++) { + for (i = 0; i < num_buffers; i++) { if (p->pac->buffers[i].offset_lo > UINT32_MAX - PAC_INFO_BUFFER_SIZE) { ret = EINVAL; krb5_set_error_message(context, ret, "integer overrun"); @@ -480,10 +481,10 @@ krb5_pac_add_buffer(krb5_context context, krb5_pac p, } offset = p->data.length + PAC_INFO_BUFFER_SIZE; - p->pac->buffers[len].type = type; - p->pac->buffers[len].buffersize = data->length; - p->pac->buffers[len].offset_lo = offset; - p->pac->buffers[len].offset_hi = 0; + p->pac->buffers[num_buffers].type = type; + p->pac->buffers[num_buffers].buffersize = data->length; + p->pac->buffers[num_buffers].offset_lo = offset; + p->pac->buffers[num_buffers].offset_hi = 0; old_end = p->data.length; if (offset > UINT32_MAX - data->length) { -- 2.35.0