The branch, master has been updated via 434f461e9e5 CVE-2022-42898 third_party/heimdal: PAC parse integer overflows from 15696da0151 gp: Fix startup scripts add not always set runonce
https://git.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit 434f461e9e5a914d4e5a9141324f1705e5e50cf9 Author: Joseph Sutton <josephsut...@catalyst.net.nz> Date: Fri Oct 14 16:45:37 2022 +1300 CVE-2022-42898 third_party/heimdal: PAC parse integer overflows Catch overflows that result from adding PAC_INFO_BUFFER_SIZE. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15203 Heavily edited by committer Nico Williams <n...@twosigma.com>, original by Joseph Sutton <josephsut...@catalyst.net.nz>. Signed-off-by: Nico Williams <n...@twosigma.com> [jsut...@samba.org Zero-initialised header_size in krb5_pac_parse() to avoid a maybe-uninitialized error; added a missing check for ret == 0] Autobuild-User(master): Jule Anger <jan...@samba.org> Autobuild-Date(master): Tue Nov 15 17:02:52 UTC 2022 on sn-devel-184 ----------------------------------------------------------------------- Summary of changes: third_party/heimdal/lib/krb5/pac.c | 614 +++++++++++++++++++++----------- third_party/heimdal/lib/krb5/test_pac.c | 48 ++- 2 files changed, 444 insertions(+), 218 deletions(-) Changeset truncated at 500 lines: diff --git a/third_party/heimdal/lib/krb5/pac.c b/third_party/heimdal/lib/krb5/pac.c index b923981908d..e6dfe2aef3d 100644 --- a/third_party/heimdal/lib/krb5/pac.c +++ b/third_party/heimdal/lib/krb5/pac.c @@ -37,19 +37,34 @@ #include <wind.h> #include <assert.h> +/* + * https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-pac/3341cfa2-6ef5-42e0-b7bc-4544884bf399 + */ struct PAC_INFO_BUFFER { - uint32_t type; - uint32_t buffersize; - uint32_t offset_hi; - uint32_t offset_lo; + uint32_t type; /* ULONG ulType in the original */ + uint32_t buffersize; /* ULONG cbBufferSize in the original */ + uint64_t offset; /* ULONG64 Offset in the original + * this being the offset from the beginning of the + * struct PACTYPE to the beginning of the buffer + * containing data of type ulType + */ }; +/* + * https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-pac/6655b92f-ab06-490b-845d-037e6987275f + */ struct PACTYPE { - uint32_t numbuffers; - uint32_t version; - struct PAC_INFO_BUFFER buffers[1]; + uint32_t numbuffers; /* named cBuffers of type ULONG in the original */ + uint32_t version; /* Named Version of type ULONG in the original */ + struct PAC_INFO_BUFFER buffers[1]; /* an ellipsis (...) in the original */ }; +/* + * A PAC starts with a PACTYPE header structure that is followed by an array of + * numbuffers PAC_INFO_BUFFER structures, each of which points to a buffer + * beyond the last PAC_INFO_BUFFER structures. + */ + struct krb5_pac_data { struct PACTYPE *pac; krb5_data data; @@ -133,6 +148,60 @@ struct heim_type_data pac_object = { NULL }; +/* + * Returns the size of the PACTYPE header + the PAC_INFO_BUFFER array. This is + * also the end of the whole thing, and any offsets to buffers from + * thePAC_INFO_BUFFER[] entries have to be beyond it. + */ +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 */ + if (num_buffers > UINT32_MAX / PAC_INFO_BUFFER_SIZE) { + ret = EOVERFLOW; + 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 */ + if (header_size > UINT32_MAX - PACTYPE_SIZE) { + ret = EOVERFLOW; + krb5_set_error_message(context, ret, "PAC has too many buffers"); + return ret; + } + header_size += PACTYPE_SIZE; + + *result = header_size; + + return 0; +} + +/* Output `size' + `addend' + padding for alignment if it doesn't overflow */ +static krb5_error_code +pac_aligned_size(krb5_context context, + uint32_t size, + uint32_t addend, + uint32_t *aligned_size) +{ + krb5_error_code ret; + + if (size > UINT32_MAX - addend || + (size + addend) > UINT32_MAX - (PAC_ALIGNMENT - 1)) { + ret = EOVERFLOW; + krb5_set_error_message(context, ret, "integer overrun"); + return ret; + } + size += addend; + size += PAC_ALIGNMENT - 1; + size &= ~(PAC_ALIGNMENT - 1); + *aligned_size = size; + return 0; +} + /* * HMAC-MD5 checksum over any key (needed for the PAC routines) */ @@ -184,165 +253,164 @@ KRB5_LIB_FUNCTION krb5_error_code KRB5_LIB_CALL krb5_pac_parse(krb5_context context, const void *ptr, size_t len, krb5_pac *pac) { - krb5_error_code ret; + krb5_error_code ret = 0; krb5_pac p; krb5_storage *sp = NULL; - uint32_t i, tmp, tmp2, header_end; + uint32_t i, num_buffers, version, header_size = 0; + uint32_t prev_start = 0; + uint32_t prev_end = 0; + *pac = NULL; p = _heim_alloc_object(&pac_object, sizeof(*p)); - if (p == NULL) { - ret = krb5_enomem(context); - goto out; - } - - sp = krb5_storage_from_readonly_mem(ptr, len); - if (sp == NULL) { + if (p) + sp = krb5_storage_from_readonly_mem(ptr, len); + if (sp == NULL) ret = krb5_enomem(context); - goto out; - } - krb5_storage_set_flags(sp, KRB5_STORAGE_BYTEORDER_LE); - - CHECK(ret, krb5_ret_uint32(sp, &tmp), out); - CHECK(ret, krb5_ret_uint32(sp, &tmp2), out); - if (tmp < 1) { - ret = EINVAL; /* Too few buffers */ - krb5_set_error_message(context, ret, N_("PAC has too few buffers", "")); - goto out; + if (ret == 0) { + krb5_storage_set_flags(sp, KRB5_STORAGE_BYTEORDER_LE); + ret = krb5_ret_uint32(sp, &num_buffers); } - if (tmp2 != 0) { - ret = EINVAL; /* Wrong version */ - krb5_set_error_message(context, ret, + if (ret == 0) + ret = krb5_ret_uint32(sp, &version); + if (ret == 0 && num_buffers < 1) + krb5_set_error_message(context, ret = EINVAL, + N_("PAC has too few buffers", "")); + if (ret == 0 && num_buffers > 1000) + krb5_set_error_message(context, ret = EINVAL, + N_("PAC has too many buffers", "")); + if (ret == 0 && version != 0) + krb5_set_error_message(context, ret = EINVAL, N_("PAC has wrong version %d", ""), - (int)tmp2); - goto out; - } - - p->pac = calloc(1, - sizeof(*p->pac) + (sizeof(p->pac->buffers[0]) * (tmp - 1))); - if (p->pac == NULL) { + (int)version); + if (ret == 0) + ret = pac_header_size(context, num_buffers, &header_size); + if (ret == 0 && header_size > len) + krb5_set_error_message(context, ret = EOVERFLOW, + N_("PAC encoding invalid, would overflow buffers", "")); + if (ret == 0) + p->pac = calloc(1, header_size); + if (ret == 0 && p->pac == NULL) ret = krb5_enomem(context); - goto out; - } - - 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; + if (ret == 0) { + p->pac->numbuffers = num_buffers; + p->pac->version = version; } - for (i = 0; i < p->pac->numbuffers; i++) { - CHECK(ret, krb5_ret_uint32(sp, &p->pac->buffers[i].type), out); - CHECK(ret, krb5_ret_uint32(sp, &p->pac->buffers[i].buffersize), out); - CHECK(ret, krb5_ret_uint32(sp, &p->pac->buffers[i].offset_lo), out); - CHECK(ret, krb5_ret_uint32(sp, &p->pac->buffers[i].offset_hi), out); + for (i = 0; ret == 0 && i < p->pac->numbuffers; i++) { + ret = krb5_ret_uint32(sp, &p->pac->buffers[i].type); + if (ret == 0) + ret = krb5_ret_uint32(sp, &p->pac->buffers[i].buffersize); + if (ret == 0) + ret = krb5_ret_uint64(sp, &p->pac->buffers[i].offset); + if (ret) + break; - /* consistency checks */ - if (p->pac->buffers[i].offset_lo & (PAC_ALIGNMENT - 1)) { - ret = EINVAL; - krb5_set_error_message(context, ret, + /* Consistency checks (we don't check for wasted space) */ + if (p->pac->buffers[i].offset & (PAC_ALIGNMENT - 1)) { + krb5_set_error_message(context, ret = EINVAL, N_("PAC out of alignment", "")); - goto out; + break; } - if (p->pac->buffers[i].offset_hi) { - ret = EINVAL; - krb5_set_error_message(context, ret, - N_("PAC high offset set", "")); - goto out; + if (p->pac->buffers[i].offset > len || + p->pac->buffers[i].buffersize > len || + len - p->pac->buffers[i].offset < p->pac->buffers[i].buffersize) { + krb5_set_error_message(context, ret = EOVERFLOW, + N_("PAC buffer overflow", "")); + break; } - if (p->pac->buffers[i].offset_lo > len) { - ret = EINVAL; - krb5_set_error_message(context, ret, - N_("PAC offset overflow", "")); - goto out; - } - if (p->pac->buffers[i].offset_lo < header_end) { - ret = EINVAL; - krb5_set_error_message(context, ret, + if (p->pac->buffers[i].offset < header_size) { + krb5_set_error_message(context, ret = EINVAL, N_("PAC offset inside header: %lu %lu", ""), - (unsigned long)p->pac->buffers[i].offset_lo, - (unsigned long)header_end); - goto out; - } - if (p->pac->buffers[i].buffersize > len - p->pac->buffers[i].offset_lo){ - ret = EINVAL; - krb5_set_error_message(context, ret, N_("PAC length overflow", "")); - goto out; + (unsigned long)p->pac->buffers[i].offset, + (unsigned long)header_size); + break; } - /* let save pointer to data we need later */ - if (p->pac->buffers[i].type == PAC_SERVER_CHECKSUM) { - if (p->server_checksum) { - ret = EINVAL; - krb5_set_error_message(context, ret, + /* + * We'd like to check for non-overlapping of buffers, but the buffers + * need not be in the same order as the PAC_INFO_BUFFER[] entries + * pointing to them! To fully check for overlap we'd have to have an + * O(N^2) loop after we parse all the PAC_INFO_BUFFER[]. + * + * But we can check that each buffer does not overlap the previous + * buffer. + */ + if (prev_start) { + if (p->pac->buffers[i].offset >= prev_start && + p->pac->buffers[i].offset < prev_end) { + krb5_set_error_message(context, ret = EINVAL, + N_("PAC overlap", "")); + break; + } + if (p->pac->buffers[i].offset < prev_start && + p->pac->buffers[i].offset + + p->pac->buffers[i].buffersize > prev_start) { + krb5_set_error_message(context, ret = EINVAL, + N_("PAC overlap", "")); + break; + } + } + prev_start = p->pac->buffers[i].offset; + prev_end = p->pac->buffers[i].offset + p->pac->buffers[i].buffersize; + + /* Let's save pointers to buffers we'll need later */ + switch (p->pac->buffers[i].type) { + case PAC_SERVER_CHECKSUM: + if (p->server_checksum) + krb5_set_error_message(context, ret = EINVAL, N_("PAC has multiple server checksums", "")); - goto out; - } - p->server_checksum = &p->pac->buffers[i]; - } else if (p->pac->buffers[i].type == PAC_PRIVSVR_CHECKSUM) { - if (p->privsvr_checksum) { - ret = EINVAL; - krb5_set_error_message(context, ret, - N_("PAC has multiple KDC checksums", "")); - goto out; - } - p->privsvr_checksum = &p->pac->buffers[i]; - } else if (p->pac->buffers[i].type == PAC_LOGON_NAME) { - if (p->logon_name) { - ret = EINVAL; - krb5_set_error_message(context, ret, - N_("PAC has multiple logon names", "")); - goto out; - } - p->logon_name = &p->pac->buffers[i]; - } else if (p->pac->buffers[i].type == PAC_UPN_DNS_INFO) { - if (p->upn_dns_info) { - ret = EINVAL; - krb5_set_error_message(context, ret, + else + p->server_checksum = &p->pac->buffers[i]; + break; + case PAC_PRIVSVR_CHECKSUM: + if (p->privsvr_checksum) + krb5_set_error_message(context, ret = EINVAL, + N_("PAC has multiple KDC checksums", "")); + else + p->privsvr_checksum = &p->pac->buffers[i]; + break; + case PAC_LOGON_NAME: + if (p->logon_name) + krb5_set_error_message(context, ret = EINVAL, + N_("PAC has multiple logon names", "")); + else + p->logon_name = &p->pac->buffers[i]; + break; + case PAC_UPN_DNS_INFO: + if (p->upn_dns_info) + krb5_set_error_message(context, ret = EINVAL, N_("PAC has multiple UPN DNS info buffers", "")); - goto out; - } - p->upn_dns_info = &p->pac->buffers[i]; - } else if (p->pac->buffers[i].type == PAC_TICKET_CHECKSUM) { - if (p->ticket_checksum) { - ret = EINVAL; - krb5_set_error_message(context, ret, + else + p->upn_dns_info = &p->pac->buffers[i]; + break; + case PAC_TICKET_CHECKSUM: + if (p->ticket_checksum) + krb5_set_error_message(context, ret = EINVAL, N_("PAC has multiple ticket checksums", "")); - goto out; - } - p->ticket_checksum = &p->pac->buffers[i]; - } else if (p->pac->buffers[i].type == PAC_ATTRIBUTES_INFO) { - if (p->attributes_info) { - ret = EINVAL; - krb5_set_error_message(context, ret, + else + p->ticket_checksum = &p->pac->buffers[i]; + break; + case PAC_ATTRIBUTES_INFO: + if (p->attributes_info) + krb5_set_error_message(context, ret = EINVAL, N_("PAC has multiple attributes info buffers", "")); - goto out; - } - p->attributes_info = &p->pac->buffers[i]; - } + else + p->attributes_info = &p->pac->buffers[i]; + break; + default: break; + } } - ret = krb5_data_copy(&p->data, ptr, len); - if (ret) - goto out; - - krb5_storage_free(sp); - - *pac = p; - return 0; - -out: - if (sp) - krb5_storage_free(sp); - if (p) { - if (p->pac) - free(p->pac); - krb5_pac_free(context, p); + if (ret == 0) + ret = krb5_data_copy(&p->data, ptr, len); + if (ret == 0) { + *pac = p; + p = NULL; } - *pac = NULL; - + if (sp) + krb5_storage_free(sp); + krb5_pac_free(context, p); return ret; } @@ -369,77 +437,111 @@ krb5_pac_init(krb5_context context, krb5_pac *pac) krb5_pac_free(context, p); return krb5_enomem(context); } + memset(p->data.data, 0, p->data.length); *pac = p; return 0; } +/** + * Add a PAC buffer `nd' of type `type' to the pac `p'. + * + * @param context + * @param p + * @param type + * @param nd + * + * @return 0 on success or a Kerberos or system error. + */ KRB5_LIB_FUNCTION krb5_error_code KRB5_LIB_CALL krb5_pac_add_buffer(krb5_context context, krb5_pac p, - uint32_t type, const krb5_data *data) + uint32_t type, const krb5_data *nd) { krb5_error_code ret; void *ptr; - size_t len, offset, header_end, old_end; + size_t old_len = p->data.length; + uint32_t len, offset, header_size; uint32_t i; + uint32_t num_buffers; - assert(data->data != NULL); + assert(nd->data != NULL); - len = p->pac->numbuffers; + num_buffers = p->pac->numbuffers; + ret = pac_header_size(context, num_buffers + 1, &header_size); + if (ret) + return ret; - ptr = realloc(p->pac, - sizeof(*p->pac) + (sizeof(p->pac->buffers[0]) * len)); + ptr = realloc(p->pac, header_size); if (ptr == NULL) return krb5_enomem(context); p->pac = ptr; + p->pac->buffers[num_buffers].type = 0; + p->pac->buffers[num_buffers].buffersize = 0; + p->pac->buffers[num_buffers].offset = 0; - for (i = 0; i < len; i++) - p->pac->buffers[i].offset_lo += PAC_INFO_BUFFER_SIZE; - - 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; - - old_end = p->data.length; - len = p->data.length + data->length + PAC_INFO_BUFFER_SIZE; - if (len < p->data.length) { - krb5_set_error_message(context, EINVAL, "integer overrun"); - return EINVAL; + /* + * Check that we can adjust all the buffer offsets in the existing + * PAC_INFO_BUFFERs, since changing the size of PAC_INFO_BUFFER[] means + * changing the offsets of buffers following that array. + * + * We don't adjust them until we can't fail. + */ + for (i = 0; i < num_buffers; i++) { + if (p->pac->buffers[i].offset > UINT32_MAX - PAC_INFO_BUFFER_SIZE) { + krb5_set_error_message(context, ret = EOVERFLOW, + "too many / too large PAC buffers"); + return ret; + } } - /* align to PAC_ALIGNMENT */ - len = ((len + PAC_ALIGNMENT - 1) / PAC_ALIGNMENT) * PAC_ALIGNMENT; + /* + * The new buffer's offset must be past the end of the buffers we have + * (p->data), which is the sum of the header and p->data.length. + */ + /* Set offset = p->data.length + PAC_INFO_BUFFER_SIZE + alignment */ + ret = pac_aligned_size(context, p->data.length, PAC_INFO_BUFFER_SIZE, &offset); + if (ret == 0) + /* Set the new length = offset + nd->length + alignment */ -- Samba Shared Repository