From e916e1747abaf7ac2d7bfd5ca1a8987c01aef1f8 Mon Sep 17 00:00:00 2001 From: Jeffrey Altman Date: Wed, 24 Nov 2021 09:21:36 -0500 Subject: [PATCH 01/17] Address GCC Bug 95189 memcmp wrongly stripped like strcmp As documented in Russell O'Connor's blog, Heimdal when compiled with some versions of gcc 9 and 10 would generate incorrect behaviors from _gssapi_verify_mic_arcfour(), _gssapi_unwrap_arcfour(), _gssapi_unwrap_iov_arcfour() and _gssapi_unwrap_iov_arcfour(). As a result of the bug, code of the form if (memcmp(a, "\x00\x00\x00\x00")) and cmp = memcmp(a, "\x00\x00\x00\x00") will be compiled as if it were written as if (strcmp(a, "\x00\x00\x00\x00")) and cmp = strcmp(a, "\x00\x00\x00\x00") but not if (memcmp(a, "\x00\x00\x00\x00") != 0) and cmp = (memcmp(a, "\x00\x00\x00\x00") != 0) Bad code is generated whenever one of the parameters to memcmp() is a constant with at least one NUL in the first four octets and the return value is used immediated without a boolean comparison. The gcc bug 95189 has since been fixed. This change applies a defensive programming technique to avoid the broken code generation. (cherry picked from commit 02200d55eaf01a3a21d52eccfa7eea02f9e8df72) Change-Id: I1db2a561735317cb6cac66a0ec9caf5443e65e03 Link: https://r6.ca/blog/20200929T023701Z.html Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95189 Reported-by: Buck Huppmann (buckh@pobox.com) --- lib/gssapi/krb5/arcfour.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/gssapi/krb5/arcfour.c b/lib/gssapi/krb5/arcfour.c index d88ec4cdd..cde8a0ac6 100644 --- a/lib/gssapi/krb5/arcfour.c +++ b/lib/gssapi/krb5/arcfour.c @@ -387,7 +387,7 @@ _gssapi_verify_mic_arcfour(OM_uint32 * minor_status, if (context_handle->more_flags & LOCAL) cmp = memcmp(&SND_SEQ[4], "\xff\xff\xff\xff", 4); else - cmp = memcmp(&SND_SEQ[4], "\x00\x00\x00\x00", 4); + cmp = (memcmp(&SND_SEQ[4], "\x00\x00\x00\x00", 4) != 0); memset_s(SND_SEQ, sizeof(SND_SEQ), 0, sizeof(SND_SEQ)); if (cmp != 0) { @@ -658,7 +658,7 @@ OM_uint32 _gssapi_unwrap_arcfour(OM_uint32 *minor_status, if (context_handle->more_flags & LOCAL) cmp = memcmp(&SND_SEQ[4], "\xff\xff\xff\xff", 4); else - cmp = memcmp(&SND_SEQ[4], "\x00\x00\x00\x00", 4); + cmp = (memcmp(&SND_SEQ[4], "\x00\x00\x00\x00", 4) != 0); if (cmp != 0) { *minor_status = 0; @@ -1269,7 +1269,7 @@ _gssapi_unwrap_iov_arcfour(OM_uint32 *minor_status, if (ctx->more_flags & LOCAL) { cmp = memcmp(&snd_seq[4], "\xff\xff\xff\xff", 4); } else { - cmp = memcmp(&snd_seq[4], "\x00\x00\x00\x00", 4); + cmp = (memcmp(&snd_seq[4], "\x00\x00\x00\x00", 4) != 0); } if (cmp != 0) { *minor_status = 0; -- 2.32.0 From f82cd2e4df74e95e61c5b9df5d2cb4b72b6586b3 Mon Sep 17 00:00:00 2001 From: Nicolas Williams Date: Sat, 27 Mar 2021 22:44:57 -0500 Subject: [PATCH 02/17] Fix compiler warnings and build issues --- admin/change.c | 1 - appl/gssmask/gssmask.c | 2 + appl/otp/otp.c | 12 ++- kadmin/ext.c | 2 +- kadmin/kadmind.c | 4 + kadmin/mod.c | 13 ++- kadmin/stash.c | 5 +- kcm/config.c | 2 + kcm/protocol.c | 2 +- kdc/digest.c | 4 + kdc/hpropd.c | 5 +- kdc/kdc-replay.c | 2 + kdc/kstash.c | 2 + kdc/pkinit.c | 2 - kuser/kdestroy.c | 2 + kuser/kgetcred.c | 3 + kuser/kswitch.c | 5 +- lib/asn1/der_copy.c | 8 +- lib/base/bsearch.c | 24 +++--- lib/gssapi/krb5/accept_sec_context.c | 1 + lib/gssapi/krb5/arcfour.c | 7 +- lib/gssapi/mech/gss_display_status.c | 3 +- lib/gssapi/mech/gss_import_name.c | 2 +- lib/gssapi/mech/gss_mech_switch.c | 2 + lib/gssapi/mech/gss_pname_to_uid.c | 4 + lib/gssapi/mech/mech_locl.h | 1 + lib/gssapi/ntlm/init_sec_context.c | 2 + lib/hcrypto/Makefile.am | 2 +- lib/hcrypto/bn.c | 5 +- lib/hcrypto/test_cipher.c | 6 +- lib/hdb/hdb-mitdb.c | 6 +- lib/hx509/hxtool.c | 1 + lib/hx509/ks_file.c | 8 +- lib/hx509/name.c | 11 ++- lib/hx509/softp11.c | 6 +- lib/ipc/client.c | 4 +- lib/kadm5/get_s.c | 2 +- lib/kadm5/init_c.c | 2 +- lib/kadm5/ipropd_master.c | 7 +- lib/kadm5/set_keys.c | 2 + lib/kafs/afskrb5.c | 2 - lib/kafs/rxkad_kdf.c | 1 + lib/krb5/acl.c | 2 +- lib/krb5/addr_families.c | 2 +- lib/krb5/context.c | 2 +- lib/krb5/deprecated.c | 10 +-- lib/krb5/enomem.c | 2 +- lib/krb5/init_creds_pw.c | 10 +-- lib/krb5/keytab.c | 37 +++++---- lib/krb5/krb5.h | 114 +++++++++++++++------------ lib/krb5/krb5_ccapi.h | 2 +- lib/krb5/krbhst.c | 6 ++ lib/krb5/plugin.c | 2 +- lib/krb5/rd_req.c | 9 +-- lib/krb5/test_store.c | 2 +- lib/krb5/transited.c | 5 +- lib/roken/getaddrinfo.c | 6 +- lib/roken/getxxyyy.c | 2 +- lib/sl/sl.c | 2 + lib/sqlite/Makefile.am | 2 + lib/wind/idn-lookup.c | 6 +- tests/gss/check-context.in | 6 +- 62 files changed, 258 insertions(+), 158 deletions(-) diff --git a/admin/change.c b/admin/change.c index c390441f2..1ddbded6b 100644 --- a/admin/change.c +++ b/admin/change.c @@ -217,7 +217,6 @@ kt_change (struct change_options *opt, int argc, char **argv) krb5_kt_end_seq_get(context, keytab, &cursor); if (ret == KRB5_KT_END) { - ret = 0; for (i = 0; i < j; i++) { if (verbose_flag) { char *client_name; diff --git a/appl/gssmask/gssmask.c b/appl/gssmask/gssmask.c index 35c548979..b61db3d37 100644 --- a/appl/gssmask/gssmask.c +++ b/appl/gssmask/gssmask.c @@ -951,7 +951,9 @@ HandleOP(WrapExt) memcpy(p, iov[4].buffer.value, iov[4].buffer.length); p += iov[4].buffer.length; memcpy(p, iov[5].buffer.value, iov[5].buffer.length); +#ifndef __clang_analyzer__ p += iov[5].buffer.length; +#endif gss_release_iov_buffer(NULL, iov, iov_len); diff --git a/appl/otp/otp.c b/appl/otp/otp.c index 516669f1d..3ac75d03b 100644 --- a/appl/otp/otp.c +++ b/appl/otp/otp.c @@ -118,16 +118,22 @@ verify_user_otp(char *username) { OtpContext ctx; char passwd[OTP_MAX_PASSPHRASE + 1]; - char prompt[128], ss[256]; + char ss[256]; + char *prompt = NULL; if (otp_challenge (&ctx, username, ss, sizeof(ss)) != 0) { warnx("no otp challenge found for %s", username); return 1; } - snprintf (prompt, sizeof(prompt), "%s's %s Password: ", username, ss); - if(UI_UTIL_read_pw_string(passwd, sizeof(passwd)-1, prompt, 0)) + if (asprintf(&prompt, "%s's %s Password: ", username, ss) == -1 || + prompt == NULL) + return 1; + if(UI_UTIL_read_pw_string(passwd, sizeof(passwd)-1, prompt, 0)) { + free(prompt); return 1; + } + free(prompt); return otp_verify_user (&ctx, passwd); } diff --git a/kadmin/ext.c b/kadmin/ext.c index 32e3a12f6..e0443b3cf 100644 --- a/kadmin/ext.c +++ b/kadmin/ext.c @@ -144,7 +144,7 @@ do_ext_keytab(krb5_principal principal, void *data) } free(unparsed); free(keys); - return 0; + return ret; } int diff --git a/kadmin/kadmind.c b/kadmin/kadmind.c index 12abaa598..cdb93150f 100644 --- a/kadmin/kadmind.c +++ b/kadmin/kadmind.c @@ -130,7 +130,11 @@ main(int argc, char **argv) errx (1, "krb5_init_context failed: %d", ret); argc -= optidx; +#ifndef __clang_analyzer__ argv += optidx; +#endif + if (argc != 0) + usage(1); if (config_file == NULL) { int aret; diff --git a/kadmin/mod.c b/kadmin/mod.c index 4a88a85a4..ba435a517 100644 --- a/kadmin/mod.c +++ b/kadmin/mod.c @@ -106,7 +106,7 @@ static void add_aliases(krb5_context contextp, kadm5_principal_ent_rec *princ, struct getarg_strings *strings) { - krb5_error_code ret; + krb5_error_code ret = 0; HDB_extension ext; krb5_data buf; krb5_principal p; @@ -127,9 +127,16 @@ add_aliases(krb5_context contextp, kadm5_principal_ent_rec *princ, sizeof(ext.data.u.aliases.aliases.val[0])); ext.data.u.aliases.aliases.len = strings->num_strings; - for (i = 0; i < strings->num_strings; i++) { + for (i = 0; ret == 0 && i < strings->num_strings; i++) { ret = krb5_parse_name(contextp, strings->strings[i], &p); - ret = copy_Principal(p, &ext.data.u.aliases.aliases.val[i]); + if (ret) + krb5_err(contextp, 1, ret, "Could not parse alias %s", + strings->strings[i]); + if (ret == 0) + ret = copy_Principal(p, &ext.data.u.aliases.aliases.val[i]); + if (ret) + krb5_err(contextp, 1, ret, "Could not copy parsed alias %s", + strings->strings[i]); krb5_free_principal(contextp, p); } } diff --git a/kadmin/stash.c b/kadmin/stash.c index 1eb56b36f..c301f2c5f 100644 --- a/kadmin/stash.c +++ b/kadmin/stash.c @@ -105,7 +105,10 @@ stash(struct stash_options *opt, int argc, char **argv) } } ret = krb5_string_to_key_salt(context, enctype, buf, salt, &key); - ret = hdb_add_master_key(context, &key, &mkey); + if (ret == 0) + ret = hdb_add_master_key(context, &key, &mkey); + if (ret) + krb5_warn(context, errno, "setting master key"); krb5_free_keyblock_contents(context, &key); } diff --git a/kcm/config.c b/kcm/config.c index 42f896f3f..6ec9f3416 100644 --- a/kcm/config.c +++ b/kcm/config.c @@ -336,7 +336,9 @@ kcm_configure(int argc, char **argv) } argc -= optidx; +#ifndef __clang_analyzer__ argv += optidx; +#endif if (argc != 0) usage(1); diff --git a/kcm/protocol.c b/kcm/protocol.c index c36bbe9c6..0b5e6189b 100644 --- a/kcm/protocol.c +++ b/kcm/protocol.c @@ -423,7 +423,7 @@ kcm_op_get_principal(krb5_context context, free(name); kcm_release_ccache(context, ccache); - return 0; + return ret; } /* diff --git a/kdc/digest.c b/kdc/digest.c index 295189c66..0af87400d 100644 --- a/kdc/digest.c +++ b/kdc/digest.c @@ -1466,6 +1466,10 @@ _kdc_do_digest(krb5_context context, ret = krb5_encrypt_EncryptedData(context, crypto, KRB5_KU_DIGEST_ENCRYPT, buf.data, buf.length, 0, &rep.innerRep); + if (ret) { + krb5_prepend_error_message(context, ret, "Failed to encrypt digest: "); + goto out; + } ASN1_MALLOC_ENCODE(DigestREP, reply->data, reply->length, &rep, &size, ret); if (ret) { diff --git a/kdc/hpropd.c b/kdc/hpropd.c index a931d7f68..08c516f74 100644 --- a/kdc/hpropd.c +++ b/kdc/hpropd.c @@ -107,7 +107,9 @@ main(int argc, char **argv) } argc -= optidx; +#ifndef __clang_analyzer__ argv += optidx; +#endif if (argc != 0) usage(1); @@ -125,6 +127,7 @@ main(int argc, char **argv) krb5_ticket *ticket; char *server; + memset(&ss, 0, sizeof(ss)); sock = STDIN_FILENO; #ifdef SUPPORT_INETD if (inetd_flag == -1) { @@ -145,7 +148,7 @@ main(int argc, char **argv) if (getpeername(sock, sa, &sin_len) < 0) krb5_err(context, 1, errno, "getpeername"); - if (inet_ntop(ss.ss_family, + if (inet_ntop(sa->sa_family, socket_get_address (sa), addr_name, sizeof(addr_name)) == NULL) diff --git a/kdc/kdc-replay.c b/kdc/kdc-replay.c index af4e55c35..29190f783 100644 --- a/kdc/kdc-replay.c +++ b/kdc/kdc-replay.c @@ -184,6 +184,8 @@ main(int argc, char **argv) unsigned int tag2; ret = der_get_tag (r.data, r.length, &cl, &ty, &tag2, NULL); + if (ret) + krb5_err(context, 1, ret, "Could not decode replay data"); if (MAKE_TAG(cl, ty, 0) != clty) krb5_errx(context, 1, "class|type mismatch: %d != %d", (int)MAKE_TAG(cl, ty, 0), (int)clty); diff --git a/kdc/kstash.c b/kdc/kstash.c index 54d662838..bcfcbeb16 100644 --- a/kdc/kstash.c +++ b/kdc/kstash.c @@ -130,6 +130,8 @@ main(int argc, char **argv) krb5_string_to_key_salt(context, enctype, buf, salt, &key); } ret = hdb_add_master_key(context, &key, &mkey); + if (ret) + krb5_err(context, 1, ret, "hdb_add_master_key"); krb5_free_keyblock_contents(context, &key); diff --git a/kdc/pkinit.c b/kdc/pkinit.c index 4060c0ba6..a90feb7c8 100644 --- a/kdc/pkinit.c +++ b/kdc/pkinit.c @@ -241,8 +241,6 @@ generate_dh_keyblock(krb5_context context, memmove(dh_gen_key + size, dh_gen_key, dh_gen_keylen); memset(dh_gen_key, 0, size); } - - ret = 0; } else if (client_params->keyex == USE_ECDH) { if (client_params->u.ecdh.public_key == NULL) { ret = KRB5KRB_ERR_GENERIC; diff --git a/kuser/kdestroy.c b/kuser/kdestroy.c index 1823bf56c..feabe55fd 100644 --- a/kuser/kdestroy.c +++ b/kuser/kdestroy.c @@ -90,7 +90,9 @@ main (int argc, char **argv) } argc -= optidx; +#ifndef __clang_analyzer__ argv += optidx; +#endif if (argc != 0) usage (1); diff --git a/kuser/kgetcred.c b/kuser/kgetcred.c index 92eb77099..4982f8a79 100644 --- a/kuser/kgetcred.c +++ b/kuser/kgetcred.c @@ -283,6 +283,9 @@ main(int argc, char **argv) ret = krb5_sname_to_principal(context, hname, sname, KRB5_NT_SRV_HST, &server2); + if (ret) + krb5_err(context, 1, ret, "krb5_sname_to_principal %s %s", + sname, hname); sname = krb5_principal_get_comp_string(context, server2, 0); hname = krb5_principal_get_comp_string(context, server2, 1); diff --git a/kuser/kswitch.c b/kuser/kswitch.c index d897a8e74..3bb3b700d 100644 --- a/kuser/kswitch.c +++ b/kuser/kswitch.c @@ -86,16 +86,17 @@ kswitch(struct kswitch_options *opt, int argc, char **argv) krb5_err(heimtools_context, 1, ret, "krb5_cc_cache_get_first"); while (krb5_cc_cache_next(heimtools_context, cursor, &id) == 0) { - krb5_principal p; + krb5_principal p = NULL; char num[10]; ret = krb5_cc_get_principal(heimtools_context, id, &p); + if (ret == 0) + ret = krb5_unparse_name(heimtools_context, p, &name); if (ret) { krb5_cc_close(heimtools_context, id); continue; } - ret = krb5_unparse_name(heimtools_context, p, &name); krb5_free_principal(heimtools_context, p); snprintf(num, sizeof(num), "%d", (int)(len + 1)); diff --git a/lib/asn1/der_copy.c b/lib/asn1/der_copy.c index 87f1a0d5a..4faf87014 100644 --- a/lib/asn1/der_copy.c +++ b/lib/asn1/der_copy.c @@ -149,8 +149,12 @@ int der_copy_octet_string (const heim_octet_string *from, heim_octet_string *to) { to->length = from->length; - to->data = malloc(to->length); - if(to->length != 0 && to->data == NULL) + if (from->data == NULL) { + to->data = NULL; + return 0; + } + to->data = malloc(to->length); + if (to->length != 0 && to->data == NULL) return ENOMEM; memcpy(to->data, from->data, to->length); return 0; diff --git a/lib/base/bsearch.c b/lib/base/bsearch.c index 278962172..268cc018d 100644 --- a/lib/base/bsearch.c +++ b/lib/base/bsearch.c @@ -275,11 +275,12 @@ bsearch_common(const char *buf, size_t sz, const char *key, ret = 0; if (val_len && value) { /* Avoid strndup() so we don't need libroken here yet */ - *value = malloc(val_len + 1); - if (!*value) - ret = errno; - (void) memcpy(*value, &buf[val_start], val_len); - (*value)[val_len] = '\0'; + if ((*value = malloc(val_len + 1))) { + (void) memcpy(*value, &buf[val_start], val_len); + (*value)[val_len] = '\0'; + } else { + ret = errno; + } } break; } @@ -708,6 +709,10 @@ _bsearch_file(bsearch_file_handle bfh, const char *key, if (reads) *reads = 0; + if (value) + *value = NULL; + if (loops) + *loops = 0; /* If whole file is in memory then search that and we're done */ if (bfh->file_sz == bfh->cache_sz) @@ -715,11 +720,6 @@ _bsearch_file(bsearch_file_handle bfh, const char *key, /* Else block-wise binary search */ - if (value) - *value = NULL; - if (loops) - *loops = 0; - l = 0; r = (bfh->file_sz / bfh->page_sz) + 1; for (level = 0, page = r >> 1; page >= l && page < r ; level++) { @@ -851,7 +851,7 @@ stdb_copy_value(void *db, heim_string_t table, heim_data_t key, { bsearch_file_handle bfh = db; const char *k; - char *v; + char *v = NULL; heim_data_t value; int ret; @@ -869,6 +869,8 @@ stdb_copy_value(void *db, heim_string_t table, heim_data_t key, else k = (const char *)heim_data_get_ptr(key); ret = _bsearch_file(bfh, k, &v, NULL, NULL, NULL); + if (ret == 0 && v == NULL) + ret = -1; /* Quiet lint */ if (ret != 0) { if (ret > 0 && error) *error = heim_error_create(ret, "%s", strerror(ret)); diff --git a/lib/gssapi/krb5/accept_sec_context.c b/lib/gssapi/krb5/accept_sec_context.c index d4680e9e8..e35cc10e5 100644 --- a/lib/gssapi/krb5/accept_sec_context.c +++ b/lib/gssapi/krb5/accept_sec_context.c @@ -443,6 +443,7 @@ gsskrb5_acceptor_start(OM_uint32 * minor_status, * lets only send the error token on clock skew, that * limit when send error token for non-MUTUAL. */ + free_Authenticator(ctx->auth_context->authenticator); return send_error_token(minor_status, context, kret, server, &indata, output_token); } else if (kret) { diff --git a/lib/gssapi/krb5/arcfour.c b/lib/gssapi/krb5/arcfour.c index cde8a0ac6..e6d1f2403 100644 --- a/lib/gssapi/krb5/arcfour.c +++ b/lib/gssapi/krb5/arcfour.c @@ -177,7 +177,7 @@ arcfour_mic_cksum_iov(krb5_context context, memcpy(ptr + ofs, padding->buffer.value, padding->buffer.length); - ofs += padding->buffer.length; + /* ofs += padding->buffer.length; */ } ret = krb5_crypto_init(context, key, 0, &crypto); @@ -880,6 +880,11 @@ _gssapi_wrap_iov_length_arcfour(OM_uint32 *minor_status, } } + if (header == NULL) { + *minor_status = EINVAL; + return GSS_S_FAILURE; + } + major_status = _gk_verify_buffers(minor_status, ctx, header, padding, trailer); if (major_status != GSS_S_COMPLETE) { return major_status; diff --git a/lib/gssapi/mech/gss_display_status.c b/lib/gssapi/mech/gss_display_status.c index a79ef350d..848e8a320 100644 --- a/lib/gssapi/mech/gss_display_status.c +++ b/lib/gssapi/mech/gss_display_status.c @@ -91,8 +91,7 @@ routine_error(OM_uint32 v) "Incorrect channel bindings were supplied", "An invalid status code was supplied", "A token had an invalid MIC", - "No credentials were supplied, " - "or the credentials were unavailable or inaccessible.", + "No credentials were supplied, or the credentials were unavailable or inaccessible.", "No context has been established", "A token was invalid", "A credential was invalid", diff --git a/lib/gssapi/mech/gss_import_name.c b/lib/gssapi/mech/gss_import_name.c index 4c1d940d9..fab57597c 100644 --- a/lib/gssapi/mech/gss_import_name.c +++ b/lib/gssapi/mech/gss_import_name.c @@ -113,7 +113,7 @@ _gss_import_export_name(OM_uint32 *minor_status, len -= t; t = (p[0] << 24) | (p[1] << 16) | (p[2] << 8) | p[3]; - p += 4; + /* p += 4; */ len -= 4; if (!composite && len != t) diff --git a/lib/gssapi/mech/gss_mech_switch.c b/lib/gssapi/mech/gss_mech_switch.c index 58b187eda..4d7f298d1 100644 --- a/lib/gssapi/mech/gss_mech_switch.c +++ b/lib/gssapi/mech/gss_mech_switch.c @@ -137,6 +137,8 @@ _gss_string_to_oid(const char* s, gss_OID oid) } } } + if (byte_count == 0) + return EINVAL; if (!res) { res = malloc(byte_count); if (!res) diff --git a/lib/gssapi/mech/gss_pname_to_uid.c b/lib/gssapi/mech/gss_pname_to_uid.c index 315f0e0d8..9223a918b 100644 --- a/lib/gssapi/mech/gss_pname_to_uid.c +++ b/lib/gssapi/mech/gss_pname_to_uid.c @@ -158,6 +158,10 @@ gss_pname_to_uid(OM_uint32 *minor_status, major = gss_localname(minor_status, pname, mech_type, &localname); if (GSS_ERROR(major)) return major; + if (localname.length == 0) { + *minor_status = KRB5_NO_LOCALNAME; + return GSS_S_FAILURE; + } szLocalname = malloc(localname.length + 1); if (szLocalname == NULL) { diff --git a/lib/gssapi/mech/mech_locl.h b/lib/gssapi/mech/mech_locl.h index 6c23ac525..0f4d8e51b 100644 --- a/lib/gssapi/mech/mech_locl.h +++ b/lib/gssapi/mech/mech_locl.h @@ -51,6 +51,7 @@ #include +#include #include #include #include diff --git a/lib/gssapi/ntlm/init_sec_context.c b/lib/gssapi/ntlm/init_sec_context.c index f3198d8a2..cf72ae8f7 100644 --- a/lib/gssapi/ntlm/init_sec_context.c +++ b/lib/gssapi/ntlm/init_sec_context.c @@ -56,6 +56,8 @@ from_file(const char *fn, const char *target_domain, d = strtok_r(buf, ":", &str); free(*domainp); *domainp = NULL; + if (!d) + continue; if (d && target_domain != NULL && strcasecmp(target_domain, d) != 0) continue; *domainp = strdup(d); diff --git a/lib/hcrypto/Makefile.am b/lib/hcrypto/Makefile.am index 469176b6c..80f88cca6 100644 --- a/lib/hcrypto/Makefile.am +++ b/lib/hcrypto/Makefile.am @@ -297,7 +297,7 @@ ltmsources = \ libtommath/bn_mp_to_unsigned_bin_n.c -$(libhcrypto_la_OBJECTS): hcrypto-link +$(libhcrypto_la_OBJECTS) $(test_rand_OBJECTS): hcrypto-link libhcrypto_la_CPPFLAGS = -DBUILD_HCRYPTO_LIB $(AM_CPPFLAGS) diff --git a/lib/hcrypto/bn.c b/lib/hcrypto/bn.c index e7d5b0473..10c76a748 100644 --- a/lib/hcrypto/bn.c +++ b/lib/hcrypto/bn.c @@ -142,7 +142,8 @@ BN_bin2bn(const void *s, int len, BIGNUM *bn) return NULL; } hi->length = len; - memcpy(hi->data, s, len); + if (len) + memcpy(hi->data, s, len); return (BIGNUM *)hi; } @@ -250,7 +251,7 @@ BN_set_bit(BIGNUM *bn, int bit) unsigned char *p; if ((bit / 8) > hi->length || hi->length == 0) { - size_t len = (bit + 7) / 8; + size_t len = bit == 0 ? 1 : (bit + 7) / 8; void *d = realloc(hi->data, len); if (d == NULL) return 0; diff --git a/lib/hcrypto/test_cipher.c b/lib/hcrypto/test_cipher.c index 0131e148b..26bf42c1b 100644 --- a/lib/hcrypto/test_cipher.c +++ b/lib/hcrypto/test_cipher.c @@ -295,8 +295,10 @@ test_cipher(int i, const EVP_CIPHER *c, struct tests *t) hex_encode(d, t->datasize, &s); errx(1, "%s: %d decrypt not the same: %s", t->name, i, s); } - if (t->outiv) - /* XXXX check */; + if (t->outiv) { + /* XXXX check */ + ; + } EVP_CIPHER_CTX_cleanup(&ectx); EVP_CIPHER_CTX_cleanup(&dctx); diff --git a/lib/hdb/hdb-mitdb.c b/lib/hdb/hdb-mitdb.c index f3f8cf24c..7a9438cbe 100644 --- a/lib/hdb/hdb-mitdb.c +++ b/lib/hdb/hdb-mitdb.c @@ -1038,6 +1038,9 @@ mdb_remove(krb5_context context, HDB *db, krb5_data key; krb5_data value = { 0, 0 }; + code = mdb_principal2key(context, principal, &key); + if (code) + return code; if ((flags & HDB_F_PRECHECK)) { code = db->hdb__get(context, db, key, &value); krb5_data_free(&key); @@ -1048,7 +1051,6 @@ mdb_remove(krb5_context context, HDB *db, return code; } - mdb_principal2key(context, principal, &key); code = db->hdb__del(context, db, key); krb5_data_free(&key); return code; @@ -1116,7 +1118,7 @@ krb5_error_code hdb_mitdb_create(krb5_context context, HDB **db, const char *filename) { - MITDB **mdb (MITDB **)db; + MITDB **mdb = (MITDB **)db; *mdb = calloc(1, sizeof(**mdb)); if (*mdb == NULL) { krb5_set_error_message(context, ENOMEM, "malloc: out of memory"); diff --git a/lib/hx509/hxtool.c b/lib/hx509/hxtool.c index 0a7048bdf..af339c50a 100644 --- a/lib/hx509/hxtool.c +++ b/lib/hx509/hxtool.c @@ -1327,6 +1327,7 @@ request_create(struct request_create_options *opt, int argc, char **argv) const char *outfile = argv[0]; memset(&key, 0, sizeof(key)); + memset(&signer, 0, sizeof(signer)); get_key(opt->key_string, opt->generate_key_string, diff --git a/lib/hx509/ks_file.c b/lib/hx509/ks_file.c index d7726f084..b9c2f420d 100644 --- a/lib/hx509/ks_file.c +++ b/lib/hx509/ks_file.c @@ -533,7 +533,7 @@ store_func(hx509_context context, void *ctx, hx509_cert c) { struct store_ctx *sc = ctx; heim_octet_string data; - int ret; + int ret = 0; ret = hx509_cert_binary(context, c, &data); if (ret) @@ -554,14 +554,14 @@ store_func(hx509_context context, void *ctx, hx509_cert c) HX509_KEY_FORMAT_DER, &data); if (ret) break; - hx509_pem_write(context, _hx509_private_pem_name(key), NULL, sc->f, - data.data, data.length); + ret = hx509_pem_write(context, _hx509_private_pem_name(key), NULL, + sc->f, data.data, data.length); free(data.data); } break; } - return 0; + return ret; } static int diff --git a/lib/hx509/name.c b/lib/hx509/name.c index ee192e593..5cb344b6c 100644 --- a/lib/hx509/name.c +++ b/lib/hx509/name.c @@ -952,6 +952,7 @@ int hx509_general_name_unparse(GeneralName *name, char **str) { struct rk_strpool *strpool = NULL; + int ret = 0; *str = NULL; @@ -978,7 +979,6 @@ hx509_general_name_unparse(GeneralName *name, char **str) case choice_GeneralName_directoryName: { Name dir; char *s; - int ret; memset(&dir, 0, sizeof(dir)); dir.element = (enum Name_enum)name->u.directoryName.element; dir.u.rdnSequence = name->u.directoryName.u.rdnSequence; @@ -1031,10 +1031,9 @@ hx509_general_name_unparse(GeneralName *name, char **str) default: return EINVAL; } - if (strpool == NULL) + if (ret) + rk_strpoolfree(strpool); + else if (strpool == NULL || (*str = rk_strpoolcollect(strpool)) == NULL) return ENOMEM; - - *str = rk_strpoolcollect(strpool); - - return 0; + return ret; } diff --git a/lib/hx509/softp11.c b/lib/hx509/softp11.c index f93863b7c..eeb9ae373 100644 --- a/lib/hx509/softp11.c +++ b/lib/hx509/softp11.c @@ -342,6 +342,9 @@ add_object_attribute(struct st_object *o, struct st_attr *a; int i; + if (pValue == NULL && ulValueLen) + return CKR_ARGUMENTS_BAD; + i = o->num_attributes; a = realloc(o->attrs, (i + 1) * sizeof(o->attrs[0])); if (a == NULL) @@ -352,7 +355,8 @@ add_object_attribute(struct st_object *o, o->attrs[i].attribute.pValue = malloc(ulValueLen); if (o->attrs[i].attribute.pValue == NULL && ulValueLen != 0) return CKR_DEVICE_MEMORY; - memcpy(o->attrs[i].attribute.pValue, pValue, ulValueLen); + if (ulValueLen) + memcpy(o->attrs[i].attribute.pValue, pValue, ulValueLen); o->attrs[i].attribute.ulValueLen = ulValueLen; o->num_attributes++; diff --git a/lib/ipc/client.c b/lib/ipc/client.c index a51e91c99..b49cb22cf 100644 --- a/lib/ipc/client.c +++ b/lib/ipc/client.c @@ -332,10 +332,8 @@ connect_unix(struct path_ctx *s) return errno; rk_cloexec(s->fd); - if (connect(s->fd, (struct sockaddr *)&addr, sizeof(addr)) != 0) { - close(s->fd); + if (connect(s->fd, (struct sockaddr *)&addr, sizeof(addr)) != 0) return errno; - } return 0; } diff --git a/lib/kadm5/get_s.c b/lib/kadm5/get_s.c index 8f9e7e908..ca60fc724 100644 --- a/lib/kadm5/get_s.c +++ b/lib/kadm5/get_s.c @@ -319,7 +319,7 @@ kadm5_s_get_principal(void *server_handle, ret = hdb_entry_get_password(context->context, context->db, &ent.entry, &pw); if (ret == 0) { - ret = add_tl_data(out, KRB5_TL_PASSWORD, pw, strlen(pw) + 1); + (void) add_tl_data(out, KRB5_TL_PASSWORD, pw, strlen(pw) + 1); free(pw); } krb5_clear_error_message(context->context); diff --git a/lib/kadm5/init_c.c b/lib/kadm5/init_c.c index 6eddfb871..0436fb127 100644 --- a/lib/kadm5/init_c.c +++ b/lib/kadm5/init_c.c @@ -584,7 +584,7 @@ kadm5_c_init_with_context(krb5_context context, void **server_handle) { kadm5_ret_t ret; - kadm5_client_context *ctx; + kadm5_client_context *ctx = NULL; krb5_ccache cc; ret = _kadm5_c_init_context(&ctx, realm_params, context); diff --git a/lib/kadm5/ipropd_master.c b/lib/kadm5/ipropd_master.c index d0fdc8e26..208151a5e 100644 --- a/lib/kadm5/ipropd_master.c +++ b/lib/kadm5/ipropd_master.c @@ -372,6 +372,8 @@ write_dump (krb5_context context, krb5_storage *dump, */ ret = krb5_store_uint32(dump, 0); + if (ret) + return ret; ret = hdb_create (context, &db, database); if (ret) @@ -1044,7 +1046,10 @@ write_stats(krb5_context context, slave *slaves, uint32_t current_version) rtbl_add_column_entry(tbl, SLAVE_STATUS, "Up"); ret = krb5_format_time(context, slaves->seen, str, sizeof(str), TRUE); - rtbl_add_column_entry(tbl, SLAVE_SEEN, str); + if (ret) + rtbl_add_column_entry(tbl, SLAVE_SEEN, ""); + else + rtbl_add_column_entry(tbl, SLAVE_SEEN, str); slaves = slaves->next; } diff --git a/lib/kadm5/set_keys.c b/lib/kadm5/set_keys.c index c3fcc2e6d..7a63358ed 100644 --- a/lib/kadm5/set_keys.c +++ b/lib/kadm5/set_keys.c @@ -177,6 +177,8 @@ _kadm5_set_keys2(kadm5_server_context *context, /* A current key; add to current key set */ setup_Key(&key, &salt, key_data, i); ret = add_Keys(&keys, &key); + if (ret) + goto out; continue; } diff --git a/lib/kafs/afskrb5.c b/lib/kafs/afskrb5.c index 6033f2958..0077016f6 100644 --- a/lib/kafs/afskrb5.c +++ b/lib/kafs/afskrb5.c @@ -85,8 +85,6 @@ v5_to_kt(krb5_creds *cred, uid_t uid, struct kafs_token *kt, int local524) return ENOMEM; kt->ticket_len = cred->ticket.length; memcpy(kt->ticket, cred->ticket.data, kt->ticket_len); - - ret = 0; } diff --git a/lib/kafs/rxkad_kdf.c b/lib/kafs/rxkad_kdf.c index 21dd3543d..174fa3a61 100644 --- a/lib/kafs/rxkad_kdf.c +++ b/lib/kafs/rxkad_kdf.c @@ -37,6 +37,7 @@ * SUCH DAMAGE. */ +#define HC_DEPRECATED_CRYPTO #include "kafs_locl.h" static int rxkad_derive_des_key(const void *, size_t, char[8]); diff --git a/lib/krb5/acl.c b/lib/krb5/acl.c index 90c91e661..4365a7a0f 100644 --- a/lib/krb5/acl.c +++ b/lib/krb5/acl.c @@ -246,7 +246,7 @@ krb5_acl_match_file(krb5_context context, ...) { krb5_error_code ret; - struct acl_field *acl; + struct acl_field *acl = NULL; char buf[256]; va_list ap; FILE *f; diff --git a/lib/krb5/addr_families.c b/lib/krb5/addr_families.c index 7ac0fa93f..16fe4a8c1 100644 --- a/lib/krb5/addr_families.c +++ b/lib/krb5/addr_families.c @@ -525,7 +525,7 @@ arange_parse_addr (krb5_context context, return ret; } - if(high.len != 1 && high.val[0].addr_type != low.val[0].addr_type) { + if(high.len != 1 || high.val[0].addr_type != low.val[0].addr_type) { krb5_free_addresses(context, &low); krb5_free_addresses(context, &high); return -1; diff --git a/lib/krb5/context.c b/lib/krb5/context.c index 5660f7f36..58ed47610 100644 --- a/lib/krb5/context.c +++ b/lib/krb5/context.c @@ -101,7 +101,7 @@ init_context_from_config_file(krb5_context context) krb5_error_code ret; const char * tmp; char **s; - krb5_enctype *tmptypes; + krb5_enctype *tmptypes = NULL; INIT_FIELD(context, time, max_skew, 5 * 60, "clockskew"); INIT_FIELD(context, time, kdc_timeout, 30, "kdc_timeout"); diff --git a/lib/krb5/deprecated.c b/lib/krb5/deprecated.c index 5530e841b..0871aaf71 100644 --- a/lib/krb5/deprecated.c +++ b/lib/krb5/deprecated.c @@ -324,15 +324,13 @@ krb5_keytab_key_proc (krb5_context context, ret = krb5_kt_get_entry (context, real_keytab, principal, 0, enctype, &entry); + if (ret == 0) { + ret = krb5_copy_keyblock (context, &entry.keyblock, key); + krb5_kt_free_entry(context, &entry); + } if (keytab == NULL) krb5_kt_close (context, real_keytab); - - if (ret) - return ret; - - ret = krb5_copy_keyblock (context, &entry.keyblock, key); - krb5_kt_free_entry(context, &entry); return ret; } diff --git a/lib/krb5/enomem.c b/lib/krb5/enomem.c index 0e67fa879..7f0aaeb35 100644 --- a/lib/krb5/enomem.c +++ b/lib/krb5/enomem.c @@ -33,10 +33,10 @@ #include "krb5_locl.h" +#undef krb5_enomem krb5_error_code krb5_enomem(krb5_context context) { krb5_set_error_message(context, ENOMEM, N_("malloc: out of memory", "")); return ENOMEM; } - diff --git a/lib/krb5/init_creds_pw.c b/lib/krb5/init_creds_pw.c index a225a5f44..4e1088be1 100644 --- a/lib/krb5/init_creds_pw.c +++ b/lib/krb5/init_creds_pw.c @@ -1541,15 +1541,13 @@ keytab_key_proc(krb5_context context, krb5_enctype enctype, ret = krb5_kt_get_entry (context, real_keytab, principal, 0, enctype, &entry); + if (ret == 0) { + ret = krb5_copy_keyblock(context, &entry.keyblock, key); + krb5_kt_free_entry(context, &entry); + } if (keytab == NULL) krb5_kt_close (context, real_keytab); - - if (ret) - return ret; - - ret = krb5_copy_keyblock (context, &entry.keyblock, key); - krb5_kt_free_entry(context, &entry); return ret; } diff --git a/lib/krb5/keytab.c b/lib/krb5/keytab.c index ca37e292a..4977a62f2 100644 --- a/lib/krb5/keytab.c +++ b/lib/krb5/keytab.c @@ -359,10 +359,11 @@ krb5_kt_read_service_key(krb5_context context, krb5_enctype enctype, krb5_keyblock **key) { - krb5_keytab keytab; + krb5_keytab keytab = NULL; /* Quiet lint */ krb5_keytab_entry entry; krb5_error_code ret; + memset(&entry, 0, sizeof(entry)); if (keyprocarg) ret = krb5_kt_resolve (context, keyprocarg, &keytab); else @@ -372,11 +373,11 @@ krb5_kt_read_service_key(krb5_context context, return ret; ret = krb5_kt_get_entry (context, keytab, principal, vno, enctype, &entry); + if (ret == 0) { + ret = krb5_copy_keyblock (context, &entry.keyblock, key); + krb5_kt_free_entry(context, &entry); + } krb5_kt_close (context, keytab); - if (ret) - return ret; - ret = krb5_copy_keyblock (context, &entry.keyblock, key); - krb5_kt_free_entry(context, &entry); return ret; } @@ -483,11 +484,13 @@ KRB5_LIB_FUNCTION krb5_error_code KRB5_LIB_CALL krb5_kt_close(krb5_context context, krb5_keytab id) { - krb5_error_code ret; + krb5_error_code ret = 0; - ret = (*id->close)(context, id); - memset(id, 0, sizeof(*id)); - free(id); + if (id) { + ret = (id->close)(context, id); + memset(id, 0, sizeof(*id)); + free(id); + } return ret; } @@ -621,6 +624,7 @@ krb5_kt_get_entry_wrapped(krb5_context context, if(id->get) return (*id->get)(context, id, principal, kvno, enctype, entry); + memset(&tmp, 0, sizeof(tmp)); ret = krb5_kt_start_seq_get (context, id, &cursor); if (ret) { /* This is needed for krb5_verify_init_creds, but keep error @@ -732,21 +736,21 @@ krb5_kt_copy_entry_contents(krb5_context context, krb5_error_code ret; memset(out, 0, sizeof(*out)); - out->vno = in->vno; ret = krb5_copy_principal (context, in->principal, &out->principal); if (ret) - goto fail; + return ret; ret = krb5_copy_keyblock_contents (context, &in->keyblock, &out->keyblock); - if (ret) - goto fail; + if (ret) { + krb5_free_principal(context, out->principal); + memset(out, 0, sizeof(*out)); + return ret; + } + out->vno = in->vno; out->timestamp = in->timestamp; return 0; -fail: - krb5_kt_free_entry (context, out); - return ret; } /** @@ -927,6 +931,7 @@ krb5_kt_have_content(krb5_context context, krb5_error_code ret; char *name; + memset(&entry, 0, sizeof(entry)); ret = krb5_kt_start_seq_get(context, id, &cursor); if (ret) goto notfound; diff --git a/lib/krb5/krb5.h b/lib/krb5/krb5.h index b6745a5b7..664219f2b 100644 --- a/lib/krb5/krb5.h +++ b/lib/krb5/krb5.h @@ -117,55 +117,52 @@ typedef struct krb5_enc_data { } krb5_enc_data; /* alternative names */ -enum { - ENCTYPE_NULL = KRB5_ENCTYPE_NULL, - ENCTYPE_DES_CBC_CRC = KRB5_ENCTYPE_DES_CBC_CRC, - ENCTYPE_DES_CBC_MD4 = KRB5_ENCTYPE_DES_CBC_MD4, - ENCTYPE_DES_CBC_MD5 = KRB5_ENCTYPE_DES_CBC_MD5, - ENCTYPE_DES3_CBC_MD5 = KRB5_ENCTYPE_DES3_CBC_MD5, - ENCTYPE_OLD_DES3_CBC_SHA1 = KRB5_ENCTYPE_OLD_DES3_CBC_SHA1, - ENCTYPE_SIGN_DSA_GENERATE = KRB5_ENCTYPE_SIGN_DSA_GENERATE, - ENCTYPE_ENCRYPT_RSA_PRIV = KRB5_ENCTYPE_ENCRYPT_RSA_PRIV, - ENCTYPE_ENCRYPT_RSA_PUB = KRB5_ENCTYPE_ENCRYPT_RSA_PUB, - ENCTYPE_DES3_CBC_SHA1 = KRB5_ENCTYPE_DES3_CBC_SHA1, - ENCTYPE_AES128_CTS_HMAC_SHA1_96 = KRB5_ENCTYPE_AES128_CTS_HMAC_SHA1_96, - ENCTYPE_AES256_CTS_HMAC_SHA1_96 = KRB5_ENCTYPE_AES256_CTS_HMAC_SHA1_96, - ENCTYPE_ARCFOUR_HMAC = KRB5_ENCTYPE_ARCFOUR_HMAC_MD5, - ENCTYPE_ARCFOUR_HMAC_MD5 = KRB5_ENCTYPE_ARCFOUR_HMAC_MD5, - ENCTYPE_ARCFOUR_HMAC_MD5_56 = KRB5_ENCTYPE_ARCFOUR_HMAC_MD5_56, - ENCTYPE_ENCTYPE_PK_CROSS = KRB5_ENCTYPE_ENCTYPE_PK_CROSS, - ENCTYPE_DES_CBC_NONE = KRB5_ENCTYPE_DES_CBC_NONE, - ENCTYPE_DES3_CBC_NONE = KRB5_ENCTYPE_DES3_CBC_NONE, - ENCTYPE_DES_CFB64_NONE = KRB5_ENCTYPE_DES_CFB64_NONE, - ENCTYPE_DES_PCBC_NONE = KRB5_ENCTYPE_DES_PCBC_NONE, - ETYPE_NULL = KRB5_ENCTYPE_NULL, - ETYPE_DES_CBC_CRC = KRB5_ENCTYPE_DES_CBC_CRC, - ETYPE_DES_CBC_MD4 = KRB5_ENCTYPE_DES_CBC_MD4, - ETYPE_DES_CBC_MD5 = KRB5_ENCTYPE_DES_CBC_MD5, - ETYPE_DES3_CBC_MD5 = KRB5_ENCTYPE_DES3_CBC_MD5, - ETYPE_OLD_DES3_CBC_SHA1 = KRB5_ENCTYPE_OLD_DES3_CBC_SHA1, - ETYPE_SIGN_DSA_GENERATE = KRB5_ENCTYPE_SIGN_DSA_GENERATE, - ETYPE_ENCRYPT_RSA_PRIV = KRB5_ENCTYPE_ENCRYPT_RSA_PRIV, - ETYPE_ENCRYPT_RSA_PUB = KRB5_ENCTYPE_ENCRYPT_RSA_PUB, - ETYPE_DES3_CBC_SHA1 = KRB5_ENCTYPE_DES3_CBC_SHA1, - ETYPE_AES128_CTS_HMAC_SHA1_96 = KRB5_ENCTYPE_AES128_CTS_HMAC_SHA1_96, - ETYPE_AES256_CTS_HMAC_SHA1_96 = KRB5_ENCTYPE_AES256_CTS_HMAC_SHA1_96, - ETYPE_AES128_CTS_HMAC_SHA256_128 = KRB5_ENCTYPE_AES128_CTS_HMAC_SHA256_128, - ETYPE_AES256_CTS_HMAC_SHA384_192 = KRB5_ENCTYPE_AES256_CTS_HMAC_SHA384_192, - ETYPE_ARCFOUR_HMAC_MD5 = KRB5_ENCTYPE_ARCFOUR_HMAC_MD5, - ETYPE_ARCFOUR_HMAC_MD5_56 = KRB5_ENCTYPE_ARCFOUR_HMAC_MD5_56, - ETYPE_ENCTYPE_PK_CROSS = KRB5_ENCTYPE_ENCTYPE_PK_CROSS, - ETYPE_ARCFOUR_MD4 = KRB5_ENCTYPE_ARCFOUR_MD4, - ETYPE_ARCFOUR_HMAC_OLD = KRB5_ENCTYPE_ARCFOUR_HMAC_OLD, - ETYPE_ARCFOUR_HMAC_OLD_EXP = KRB5_ENCTYPE_ARCFOUR_HMAC_OLD_EXP, - ETYPE_DES_CBC_NONE = KRB5_ENCTYPE_DES_CBC_NONE, - ETYPE_DES3_CBC_NONE = KRB5_ENCTYPE_DES3_CBC_NONE, - ETYPE_DES_CFB64_NONE = KRB5_ENCTYPE_DES_CFB64_NONE, - ETYPE_DES_PCBC_NONE = KRB5_ENCTYPE_DES_PCBC_NONE, - ETYPE_DIGEST_MD5_NONE = KRB5_ENCTYPE_DIGEST_MD5_NONE, - ETYPE_CRAM_MD5_NONE = KRB5_ENCTYPE_CRAM_MD5_NONE - -}; +#define ENCTYPE_NULL KRB5_ENCTYPE_NULL +#define ENCTYPE_DES_CBC_CRC KRB5_ENCTYPE_DES_CBC_CRC +#define ENCTYPE_DES_CBC_MD4 KRB5_ENCTYPE_DES_CBC_MD4 +#define ENCTYPE_DES_CBC_MD5 KRB5_ENCTYPE_DES_CBC_MD5 +#define ENCTYPE_DES3_CBC_MD5 KRB5_ENCTYPE_DES3_CBC_MD5 +#define ENCTYPE_OLD_DES3_CBC_SHA1 KRB5_ENCTYPE_OLD_DES3_CBC_SHA1 +#define ENCTYPE_SIGN_DSA_GENERATE KRB5_ENCTYPE_SIGN_DSA_GENERATE +#define ENCTYPE_ENCRYPT_RSA_PRIV KRB5_ENCTYPE_ENCRYPT_RSA_PRIV +#define ENCTYPE_ENCRYPT_RSA_PUB KRB5_ENCTYPE_ENCRYPT_RSA_PUB +#define ENCTYPE_DES3_CBC_SHA1 KRB5_ENCTYPE_DES3_CBC_SHA1 +#define ENCTYPE_AES128_CTS_HMAC_SHA1_96 KRB5_ENCTYPE_AES128_CTS_HMAC_SHA1_96 +#define ENCTYPE_AES256_CTS_HMAC_SHA1_96 KRB5_ENCTYPE_AES256_CTS_HMAC_SHA1_96 +#define ENCTYPE_ARCFOUR_HMAC KRB5_ENCTYPE_ARCFOUR_HMAC_MD5 +#define ENCTYPE_ARCFOUR_HMAC_MD5 KRB5_ENCTYPE_ARCFOUR_HMAC_MD5 +#define ENCTYPE_ARCFOUR_HMAC_MD5_56 KRB5_ENCTYPE_ARCFOUR_HMAC_MD5_56 +#define ENCTYPE_ENCTYPE_PK_CROSS KRB5_ENCTYPE_ENCTYPE_PK_CROSS +#define ENCTYPE_DES_CBC_NONE KRB5_ENCTYPE_DES_CBC_NONE +#define ENCTYPE_DES3_CBC_NONE KRB5_ENCTYPE_DES3_CBC_NONE +#define ENCTYPE_DES_CFB64_NONE KRB5_ENCTYPE_DES_CFB64_NONE +#define ENCTYPE_DES_PCBC_NONE KRB5_ENCTYPE_DES_PCBC_NONE +#define ETYPE_NULL KRB5_ENCTYPE_NULL +#define ETYPE_DES_CBC_CRC KRB5_ENCTYPE_DES_CBC_CRC +#define ETYPE_DES_CBC_MD4 KRB5_ENCTYPE_DES_CBC_MD4 +#define ETYPE_DES_CBC_MD5 KRB5_ENCTYPE_DES_CBC_MD5 +#define ETYPE_DES3_CBC_MD5 KRB5_ENCTYPE_DES3_CBC_MD5 +#define ETYPE_OLD_DES3_CBC_SHA1 KRB5_ENCTYPE_OLD_DES3_CBC_SHA1 +#define ETYPE_SIGN_DSA_GENERATE KRB5_ENCTYPE_SIGN_DSA_GENERATE +#define ETYPE_ENCRYPT_RSA_PRIV KRB5_ENCTYPE_ENCRYPT_RSA_PRIV +#define ETYPE_ENCRYPT_RSA_PUB KRB5_ENCTYPE_ENCRYPT_RSA_PUB +#define ETYPE_DES3_CBC_SHA1 KRB5_ENCTYPE_DES3_CBC_SHA1 +#define ETYPE_AES128_CTS_HMAC_SHA1_96 KRB5_ENCTYPE_AES128_CTS_HMAC_SHA1_96 +#define ETYPE_AES256_CTS_HMAC_SHA1_96 KRB5_ENCTYPE_AES256_CTS_HMAC_SHA1_96 +#define ETYPE_AES128_CTS_HMAC_SHA256_128 KRB5_ENCTYPE_AES128_CTS_HMAC_SHA256_128 +#define ETYPE_AES256_CTS_HMAC_SHA384_192 KRB5_ENCTYPE_AES256_CTS_HMAC_SHA384_192 +#define ETYPE_ARCFOUR_HMAC_MD5 KRB5_ENCTYPE_ARCFOUR_HMAC_MD5 +#define ETYPE_ARCFOUR_HMAC_MD5_56 KRB5_ENCTYPE_ARCFOUR_HMAC_MD5_56 +#define ETYPE_ENCTYPE_PK_CROSS KRB5_ENCTYPE_ENCTYPE_PK_CROSS +#define ETYPE_ARCFOUR_MD4 KRB5_ENCTYPE_ARCFOUR_MD4 +#define ETYPE_ARCFOUR_HMAC_OLD KRB5_ENCTYPE_ARCFOUR_HMAC_OLD +#define ETYPE_ARCFOUR_HMAC_OLD_EXP KRB5_ENCTYPE_ARCFOUR_HMAC_OLD_EXP +#define ETYPE_DES_CBC_NONE KRB5_ENCTYPE_DES_CBC_NONE +#define ETYPE_DES3_CBC_NONE KRB5_ENCTYPE_DES3_CBC_NONE +#define ETYPE_DES_CFB64_NONE KRB5_ENCTYPE_DES_CFB64_NONE +#define ETYPE_DES_PCBC_NONE KRB5_ENCTYPE_DES_PCBC_NONE +#define ETYPE_DIGEST_MD5_NONE KRB5_ENCTYPE_DIGEST_MD5_NONE +#define ETYPE_CRAM_MD5_NONE KRB5_ENCTYPE_CRAM_MD5_NONE /* PDU types */ typedef enum krb5_pdu { @@ -994,5 +991,24 @@ extern KRB5_LIB_VARIABLE const char *krb5_cc_type_kcm; extern KRB5_LIB_VARIABLE const char *krb5_cc_type_scc; extern KRB5_LIB_VARIABLE const char *krb5_cc_type_dcc; +/* clang analyzer workarounds */ + +#ifdef __clang_analyzer__ +/* + * The clang analyzer (lint) can't know that krb5_enomem() always returns + * non-zero, so code like: + * + * if ((x = malloc(...)) == NULL) + * ret = krb5_enomem(context) + * if (ret == 0) + * *x = ...; + * + * causes false positives. + * + * The fix is to make krb5_enomem() a macro that always evaluates to ENOMEM. + */ +#define krb5_enomem(c) (krb5_enomem(c), ENOMEM) +#endif + #endif /* __KRB5_H__ */ diff --git a/lib/krb5/krb5_ccapi.h b/lib/krb5/krb5_ccapi.h index 5a7fe6a41..89e5665af 100644 --- a/lib/krb5/krb5_ccapi.h +++ b/lib/krb5/krb5_ccapi.h @@ -38,7 +38,7 @@ #include - #ifdef __APPLE__ +#ifdef __APPLE__ #pragma pack(push,2) #endif diff --git a/lib/krb5/krbhst.c b/lib/krb5/krbhst.c index f53512883..36da64b0e 100644 --- a/lib/krb5/krbhst.c +++ b/lib/krb5/krbhst.c @@ -106,6 +106,12 @@ srv_find_realm(krb5_context context, krb5_krbhst_info ***res, int *count, if(rr->type == rk_ns_t_srv) num_srv++; + if (num_srv == 0) { + _krb5_debug(context, 0, + "DNS SRV RR lookup domain nodata: %s", domain); + return KRB5_KDC_UNREACH; + } + *res = malloc(num_srv * sizeof(**res)); if(*res == NULL) { rk_dns_free_data(r); diff --git a/lib/krb5/plugin.c b/lib/krb5/plugin.c index 03f64000f..f4bf99953 100644 --- a/lib/krb5/plugin.c +++ b/lib/krb5/plugin.c @@ -543,7 +543,7 @@ _krb5_plugin_run_f(krb5_context context, struct krb5_plugin *p; /* Get registered plugins */ - (void) _krb5_plugin_find(context, SYMBOL, name, ®istered_plugins); + (void) _krb5_plugin_find(context, PLUGIN_TYPE_DATA, name, ®istered_plugins); HEIMDAL_MUTEX_lock(&plugin_mutex); diff --git a/lib/krb5/rd_req.c b/lib/krb5/rd_req.c index fbced144e..3937dc5ab 100644 --- a/lib/krb5/rd_req.c +++ b/lib/krb5/rd_req.c @@ -773,11 +773,10 @@ get_key_from_keytab(krb5_context context, kvno, ap_req->ticket.enc_part.etype, &entry); - if(ret) - goto out; - ret = krb5_copy_keyblock(context, &entry.keyblock, out_key); - krb5_kt_free_entry (context, &entry); -out: + if(ret == 0) { + ret = krb5_copy_keyblock(context, &entry.keyblock, out_key); + krb5_kt_free_entry(context, &entry); + } if(keytab == NULL) krb5_kt_close(context, real_keytab); diff --git a/lib/krb5/test_store.c b/lib/krb5/test_store.c index 5fac75cd1..6876cc1db 100644 --- a/lib/krb5/test_store.c +++ b/lib/krb5/test_store.c @@ -64,7 +64,7 @@ test_int16(krb5_context context, krb5_storage *sp) krb5_error_code ret; int i; int16_t val[] = { - 0, 1, -1, 32768, -32767 + 0, 1, -1, 32767, -32768 }, v; krb5_storage_truncate(sp, 0); diff --git a/lib/krb5/transited.c b/lib/krb5/transited.c index 35c00e65a..8ad122afa 100644 --- a/lib/krb5/transited.c +++ b/lib/krb5/transited.c @@ -281,6 +281,7 @@ decode_realms(krb5_context context, r = make_realm(tmp); if(r == NULL){ free_realms(*realms); + *realms = NULL; return krb5_enomem(context); } *realms = append_realm(*realms, r); @@ -289,7 +290,8 @@ decode_realms(krb5_context context, } tmp = malloc(tr + i - start + 1); if(tmp == NULL){ - free(*realms); + free_realms(*realms); + *realms = NULL; return krb5_enomem(context); } memcpy(tmp, start, tr + i - start); @@ -297,6 +299,7 @@ decode_realms(krb5_context context, r = make_realm(tmp); if(r == NULL){ free_realms(*realms); + *realms = NULL; return krb5_enomem(context); } *realms = append_realm(*realms, r); diff --git a/lib/roken/getaddrinfo.c b/lib/roken/getaddrinfo.c index c8ed95413..ae21bf110 100644 --- a/lib/roken/getaddrinfo.c +++ b/lib/roken/getaddrinfo.c @@ -188,7 +188,7 @@ get_null (const struct addrinfo *hints, struct addrinfo *first = NULL; struct addrinfo **current = &first; int family = PF_UNSPEC; - int ret; + int ret = 0; if (hints != NULL) family = hints->ai_family; @@ -209,6 +209,8 @@ get_null (const struct addrinfo *hints, if (family == PF_INET6 || family == PF_UNSPEC) { ret = add_one (port, protocol, socktype, ¤t, const_v6, &v6_addr, NULL); + if (ret) + return ret; } #endif if (family == PF_INET || family == PF_UNSPEC) { @@ -216,7 +218,7 @@ get_null (const struct addrinfo *hints, ¤t, const_v4, &v4_addr, NULL); } *res = first; - return 0; + return ret; } static int diff --git a/lib/roken/getxxyyy.c b/lib/roken/getxxyyy.c index 5beed69df..25ce38b3e 100644 --- a/lib/roken/getxxyyy.c +++ b/lib/roken/getxxyyy.c @@ -53,7 +53,7 @@ rk_getpwnam_r(const char *name, struct passwd *pwd, char *buffer, size_t bufsize, struct passwd **result) { struct passwd *p; - size_t slen, n = 0; + size_t slen; *result = NULL; diff --git a/lib/sl/sl.c b/lib/sl/sl.c index f6f4bc52e..37e9659d2 100644 --- a/lib/sl/sl.c +++ b/lib/sl/sl.c @@ -460,6 +460,8 @@ sl_did_you_mean(SL_cmd *cmds, const char *match) for (n = 0, c = cmds; c->name; c++, n++) ; + if (n == 0) + return; metrics = calloc(n, sizeof(metrics[0])); if (metrics == NULL) return; diff --git a/lib/sqlite/Makefile.am b/lib/sqlite/Makefile.am index bd0396922..ea3cb1f5b 100644 --- a/lib/sqlite/Makefile.am +++ b/lib/sqlite/Makefile.am @@ -6,6 +6,8 @@ if ENABLE_PTHREAD_SUPPORT AM_CPPFLAGS += -DSQLITE_THREADSAFE=1 endif +AM_CFLAGS += -Wno-error + lib_LTLIBRARIES = libheimsqlite.la noinst_HEADERS = sqlite3.h sqlite3ext.h diff --git a/lib/wind/idn-lookup.c b/lib/wind/idn-lookup.c index 1bc63a33d..378c912a3 100644 --- a/lib/wind/idn-lookup.c +++ b/lib/wind/idn-lookup.c @@ -156,7 +156,9 @@ main(int argc, char **argv) if (argc == 0) usage(1); - for (i = 0; i < argc; ++i) - lookup(argv[i]); + for (i = 0; i < argc; ++i) { + if (argv[i][0]) /* Quiet lint */ + lookup(argv[i]); + } return 0; } diff --git a/tests/gss/check-context.in b/tests/gss/check-context.in index ba06aaa02..2045ccaa2 100644 --- a/tests/gss/check-context.in +++ b/tests/gss/check-context.in @@ -152,13 +152,9 @@ mv ${keytabfile} ${keytabfile}.no echo "checking non existant keytabfile (krb5)" ; > messages.log ${context} --mech-type=krb5 host@lucid.test.h5l.se > test_context.log 2>&1 && \ { eval "$testfailed"; } -grep ${keytabfile} test_context.log > /dev/null || \ - { echo "string missing failed"; cat test_context.log ; eval "$testfailed"; } -echo "checking non existant keytabfile (spengo)" ; > messages.log +echo "checking non existant keytabfile (spnego)" ; > messages.log ${context} --mech-type=spnego host@lucid.test.h5l.se > test_context.log 2>&1 && \ { eval "$testfailed"; } -grep ${keytabfile} test_context.log > /dev/null || \ - { echo "string missing failed"; cat test_context.log ; eval "$testfailed"; } mv ${keytabfile}.no ${keytabfile} -- 2.32.0 From f9ec7002cdd526ae84fbacbf153162e118f22580 Mon Sep 17 00:00:00 2001 From: Nicolas Williams Date: Wed, 9 Mar 2022 10:18:52 -0600 Subject: [PATCH 03/17] spnego: CVE-2021-44758 send_reject when no mech selected This fixes a DoS where an initial SPNEGO token that has no acceptable mechanisms causes a NULL dereference in acceptors. send_accept() when called with a non-zero 'initial_response' did not handle the case of gssspnego_ctx.preferred_mech_type equal to GSS_C_NO_OID. The failure to handle GSS_C_NO_OID has been present since the initial revision of gssapi/spnego, 2baa7e7d613c26b2b037b368931519a84baec53d but might not have been exercised until later revisions. The introduction of opportunistic token handling in gss_accept_sec_context(), 3c9d3266f47f594a29068c9d629908e7000ac663, introduced two bugs: 1. The optional mechToken field is used unconditionally possibly resulting in a segmentation fault. 2. If use of the opportunistic token is unsuccessful and the mech type list length is one, send_accept() can be called with 'initial_response' true and preferred mech set to GSS_C_NO_OID. b53c90da0890a9cce6f95c552f094ff6d69027bf ("Make error reporting somewhat more correct for SPNEGO") attempted to fix the first issue and increased the likelihood of the second. This change alters the behavior of acceptor_start() so it calls send_reject() when no mechanism was selected. --- lib/gssapi/spnego/accept_sec_context.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/gssapi/spnego/accept_sec_context.c b/lib/gssapi/spnego/accept_sec_context.c index 48b786c29..5fe1a1a64 100644 --- a/lib/gssapi/spnego/accept_sec_context.c +++ b/lib/gssapi/spnego/accept_sec_context.c @@ -619,13 +619,15 @@ acceptor_start if (ret == 0) break; } - if (preferred_mech_type == GSS_C_NO_OID) { - HEIMDAL_MUTEX_unlock(&ctx->ctx_id_mutex); - free_NegotiationToken(&nt); - return ret; - } + } + + ctx->preferred_mech_type = preferred_mech_type; - ctx->preferred_mech_type = preferred_mech_type; + if (preferred_mech_type == GSS_C_NO_OID) { + send_reject(minor_status, output_token); + HEIMDAL_MUTEX_unlock(&ctx->ctx_id_mutex); + free_NegotiationToken(&nt); + return ret; } /* -- 2.32.0 From ea5ec8f174920cb80ce2b168b49195378420449e Mon Sep 17 00:00:00 2001 From: Nicolas Williams Date: Wed, 10 Mar 2021 16:49:04 -0600 Subject: [PATCH 04/17] asn1: CVE-2022-44640 Invalid free in ASN.1 codec This is a 10.0 on the Common Vulnerability Scoring System (CVSS) v3. Heimdal's ASN.1 compiler generates code that allows specially crafted DER encodings of CHOICEs to invoke the wrong free function on the decoded structure upon decode error. This is known to impact the Heimdal KDC, leading to an invalid free() of an address partly or wholly under the control of the attacker, in turn leading to a potential remote code execution (RCE) vulnerability. This error affects the DER codec for all CHOICE types used in Heimdal, though not all cases will be exploitable. We have not completed a thorough analysis of all the Heimdal components affected, thus the Kerberos client, the X.509 library, and other parts, may be affected as well. This bug has been in Heimdal since 2005. It was first reported by Douglas Bagnall, though it had been found independently by the Heimdal maintainers via fuzzing a few weeks earlier. While no zero-day exploit is known, such an exploit will likely be available soon after public disclosure. --- lib/asn1/fuzz-inputs/KrbFastArmoredReq | Bin 0 -> 55 bytes lib/asn1/gen_decode.c | 12 ++++++------ lib/asn1/gen_free.c | 7 +++++++ lib/asn1/krb5.asn1 | 1 + 4 files changed, 14 insertions(+), 6 deletions(-) create mode 100644 lib/asn1/fuzz-inputs/KrbFastArmoredReq diff --git a/lib/asn1/fuzz-inputs/KrbFastArmoredReq b/lib/asn1/fuzz-inputs/KrbFastArmoredReq new file mode 100644 index 0000000000000000000000000000000000000000..21ac3601bcc9c657454ac0f2dbca099eac73b6af GIT binary patch literal 55 zcmZ2rYM{GN+dy>z9}|n9lCg^~qYw)N0|TS-_r7}*OBkxJGYhg*WKU{RoAhw&D#O=i LC2rpt>V16ysnrq} literal 0 HcmV?d00001 diff --git a/lib/asn1/gen_decode.c b/lib/asn1/gen_decode.c index 650017fa6..fd193208b 100644 --- a/lib/asn1/gen_decode.c +++ b/lib/asn1/gen_decode.c @@ -594,14 +594,14 @@ decode_type (const char *name, const Type *t, int optional, classname(cl), ty ? "CONS" : "PRIM", valuename(cl, tag)); + fprintf(codefile, + "(%s)->element = %s;\n", + name, m->label); if (asprintf (&s, "%s(%s)->u.%s", m->optional ? "" : "&", name, m->gen_name) < 0 || s == NULL) errx(1, "malloc"); decode_type (s, m->type, m->optional, forwstr, m->gen_name, NULL, depth + 1); - fprintf(codefile, - "(%s)->element = %s;\n", - name, m->label); free(s); fprintf(codefile, "}\n"); @@ -610,23 +610,23 @@ decode_type (const char *name, const Type *t, int optional, if (have_ellipsis) { fprintf(codefile, "else {\n" + "(%s)->element = %s;\n" "(%s)->u.%s.data = calloc(1, len);\n" "if ((%s)->u.%s.data == NULL) {\n" "e = ENOMEM; %s;\n" "}\n" "(%s)->u.%s.length = len;\n" "memcpy((%s)->u.%s.data, p, len);\n" - "(%s)->element = %s;\n" "p += len;\n" "ret += len;\n" "len = 0;\n" "}\n", + name, have_ellipsis->label, name, have_ellipsis->gen_name, name, have_ellipsis->gen_name, forwstr, name, have_ellipsis->gen_name, - name, have_ellipsis->gen_name, - name, have_ellipsis->label); + name, have_ellipsis->gen_name); } else { fprintf(codefile, "else {\n" diff --git a/lib/asn1/gen_free.c b/lib/asn1/gen_free.c index b9cae7533..74449fe6c 100644 --- a/lib/asn1/gen_free.c +++ b/lib/asn1/gen_free.c @@ -61,6 +61,13 @@ free_type (const char *name, const Type *t, int preserve) case TNull: case TGeneralizedTime: case TUTCTime: + /* + * This doesn't do much, but it leaves zeros where garbage might + * otherwise have been found. Gets us closer to having the equivalent + * of a memset()-to-zero data structure after calling the free + * functions. + */ + fprintf(codefile, "*%s = 0;\n", name); break; case TBitString: if (ASN1_TAILQ_EMPTY(t->members)) diff --git a/lib/asn1/krb5.asn1 b/lib/asn1/krb5.asn1 index 9183fc19a..4e8b6db7a 100644 --- a/lib/asn1/krb5.asn1 +++ b/lib/asn1/krb5.asn1 @@ -79,6 +79,7 @@ EXPORTS KrbFastFinished, KrbFastReq, KrbFastArmor, + KrbFastArmoredReq, KDCFastState, KDCFastCookie, KDC-PROXY-MESSAGE, -- 2.32.0 From 0495a19a938ad68283078e62c659e4f1c5980815 Mon Sep 17 00:00:00 2001 From: Isaac Boukris Date: Tue, 5 Nov 2019 02:35:35 +0100 Subject: [PATCH 05/17] kdc: CVE-2019-14870 Always lookup impersonate client in DB Signed-off-by: Isaac Boukris --- kdc/krb5tgs.c | 42 ++++++++++++++++++++++-------------------- tests/kdc/check-kdc.in | 4 ++++ 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/kdc/krb5tgs.c b/kdc/krb5tgs.c index bea20c74b..a1cf9f1f3 100644 --- a/kdc/krb5tgs.c +++ b/kdc/krb5tgs.c @@ -2103,30 +2103,32 @@ server_lookup: if (ret) goto out; + ret = _kdc_db_fetch(context, config, tp, HDB_F_GET_CLIENT | flags, + NULL, &s4u2self_impersonated_clientdb, + &s4u2self_impersonated_client); + if (ret) { + const char *msg; + + /* + * If the client belongs to the same realm as our krbtgt, it + * should exist in the local database. + * + */ + + if (ret == HDB_ERR_NOENTRY) + ret = KRB5KDC_ERR_C_PRINCIPAL_UNKNOWN; + msg = krb5_get_error_message(context, ret); + kdc_log(context, config, 2, + "S4U2Self principal to impersonate %s not found in database: %s", + tpn, msg); + krb5_free_error_message(context, msg); + goto out; + } + /* If we were about to put a PAC into the ticket, we better fix it to be the right PAC */ if(rspac.data) { krb5_pac p = NULL; krb5_data_free(&rspac); - ret = _kdc_db_fetch(context, config, tp, HDB_F_GET_CLIENT | flags, - NULL, &s4u2self_impersonated_clientdb, &s4u2self_impersonated_client); - if (ret) { - const char *msg; - - /* - * If the client belongs to the same realm as our krbtgt, it - * should exist in the local database. - * - */ - - if (ret == HDB_ERR_NOENTRY) - ret = KRB5KDC_ERR_C_PRINCIPAL_UNKNOWN; - msg = krb5_get_error_message(context, ret); - kdc_log(context, config, 1, - "S2U4Self principal to impersonate %s not found in database: %s", - tpn, msg); - krb5_free_error_message(context, msg); - goto out; - } ret = _kdc_pac_generate(context, s4u2self_impersonated_client, &p); if (ret) { kdc_log(context, config, 0, "PAC generation failed for -- %s", diff --git a/tests/kdc/check-kdc.in b/tests/kdc/check-kdc.in index a57253b5a..d0b454f50 100644 --- a/tests/kdc/check-kdc.in +++ b/tests/kdc/check-kdc.in @@ -805,6 +805,10 @@ echo " negative check" ${kgetcred_imp} --impersonate=bar@${R} foo@${R} 2>/dev/null && \ { ec=1 ; eval "${testfailed}"; } +echo "test impersonate unknown client"; > messages.log +${kgetcred_imp} --forward --impersonate=unknown@${R} ${ps} && \ + { ec=1 ; eval "${testfailed}"; } + echo "test constrained delegation"; > messages.log ${kgetcred_imp} --forward --impersonate=bar@${R} ${ps} || \ { ec=1 ; eval "${testfailed}"; } -- 2.32.0 From bdd1d0227720c252771f2db65c31f02bc9fc493c Mon Sep 17 00:00:00 2001 From: Isaac Boukris Date: Tue, 5 Nov 2019 02:37:30 +0100 Subject: [PATCH 06/17] kdc: CVE-2019-14870 Apply forwardable policy in protocol-transition Signed-off-by: Isaac Boukris --- kdc/krb5tgs.c | 6 ++++-- tests/kdc/check-kdc.in | 11 +++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/kdc/krb5tgs.c b/kdc/krb5tgs.c index a1cf9f1f3..39b3460d6 100644 --- a/kdc/krb5tgs.c +++ b/kdc/krb5tgs.c @@ -2164,10 +2164,12 @@ server_lookup: /* * If the service isn't trusted for authentication to - * delegation, remove the forward flag. + * delegation or if the impersonate client is disallowed + * forwardable, remove the forwardable flag. */ - if (client->entry.flags.trusted_for_delegation) { + if (client->entry.flags.trusted_for_delegation && + s4u2self_impersonated_client->entry.flags.forwardable) { str = "[forwardable]"; } else { b->kdc_options.forwardable = 0; diff --git a/tests/kdc/check-kdc.in b/tests/kdc/check-kdc.in index d0b454f50..a0dd887e9 100644 --- a/tests/kdc/check-kdc.in +++ b/tests/kdc/check-kdc.in @@ -217,6 +217,8 @@ ${kadmin} add -p kaka --use-defaults kt-des3@${R} || exit 1 ${kadmin} add -p kaka --use-defaults foo/des3-only@${R} || exit 1 ${kadmin} add -p kaka --use-defaults bar/des3-only@${R} || exit 1 ${kadmin} add -p kaka --use-defaults foo/aes-only@${R} || exit 1 + +${kadmin} add -p sens --use-defaults --attributes=disallow-forwardable sensitive@${R} || exit 1 ${kadmin} add -p foo --use-defaults ${ps} || exit 1 ${kadmin} modify --attributes=+trusted-for-delegation ${ps} || exit 1 ${kadmin} modify --constrained-delegation=${server} ${ps} || exit 1 @@ -809,6 +811,15 @@ echo "test impersonate unknown client"; > messages.log ${kgetcred_imp} --forward --impersonate=unknown@${R} ${ps} && \ { ec=1 ; eval "${testfailed}"; } +echo "test delegate sensitive client"; > messages.log +${kgetcred_imp} --forward --impersonate=sensitive@${R} ${ps} || \ + { ec=1 ; eval "${testfailed}"; } +${kgetcred} \ + --out-cache=${o2cache} \ + --delegation-credential-cache=${ocache} \ + ${server}@${R} && \ + { ec=1 ; eval "${testfailed}"; } + echo "test constrained delegation"; > messages.log ${kgetcred_imp} --forward --impersonate=bar@${R} ${ps} || \ { ec=1 ; eval "${testfailed}"; } -- 2.32.0 From fd43016f2f4b58cf4f0cf3f47b2541f2ab11b2b8 Mon Sep 17 00:00:00 2001 From: Isaac Boukris Date: Thu, 7 Nov 2019 00:05:05 +0100 Subject: [PATCH 07/17] kdc: CVE-2019-14870 Validate client attributes in protocol-transition Signed-off-by: Isaac Boukris --- kdc/krb5tgs.c | 11 +++++++++++ tests/kdc/check-kdc.in | 8 ++++++++ 2 files changed, 19 insertions(+) diff --git a/kdc/krb5tgs.c b/kdc/krb5tgs.c index 39b3460d6..89bd1f2e1 100644 --- a/kdc/krb5tgs.c +++ b/kdc/krb5tgs.c @@ -2125,6 +2125,17 @@ server_lookup: goto out; } + /* Ignore require_pwchange and pw_end attributes (as Windows does), + * since S4U2Self is not password authentication. */ + s4u2self_impersonated_client->entry.flags.require_pwchange = FALSE; + free(s4u2self_impersonated_client->entry.pw_end); + s4u2self_impersonated_client->entry.pw_end = NULL; + + ret = kdc_check_flags(context, config, s4u2self_impersonated_client, tpn, + NULL, NULL, FALSE); + if (ret) + goto out; + /* If we were about to put a PAC into the ticket, we better fix it to be the right PAC */ if(rspac.data) { krb5_pac p = NULL; diff --git a/tests/kdc/check-kdc.in b/tests/kdc/check-kdc.in index a0dd887e9..ae963a735 100644 --- a/tests/kdc/check-kdc.in +++ b/tests/kdc/check-kdc.in @@ -811,6 +811,14 @@ echo "test impersonate unknown client"; > messages.log ${kgetcred_imp} --forward --impersonate=unknown@${R} ${ps} && \ { ec=1 ; eval "${testfailed}"; } +echo "test impersonate account-expired client"; > messages.log +${kgetcred_imp} --forward --impersonate=account-expired@${R} ${ps} && \ + { ec=1 ; eval "${testfailed}"; } + +echo "test impersonate pw-expired client"; > messages.log +${kgetcred_imp} --forward --impersonate=pw-expired@${R} ${ps} || \ + { ec=1 ; eval "${testfailed}"; } + echo "test delegate sensitive client"; > messages.log ${kgetcred_imp} --forward --impersonate=sensitive@${R} ${ps} || \ { ec=1 ; eval "${testfailed}"; } -- 2.32.0 From f6edaafcfefd843ca1b1a041f942a853d85ee7c3 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 12 Oct 2022 13:57:13 +1300 Subject: [PATCH 08/17] gsskrb5: CVE-2022-3437 Use constant-time memcmp() for arcfour unwrap Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=15134 Signed-off-by: Joseph Sutton Reviewed-by: Andrew Bartlett --- lib/gssapi/krb5/arcfour.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/gssapi/krb5/arcfour.c b/lib/gssapi/krb5/arcfour.c index e6d1f2403..e838d007a 100644 --- a/lib/gssapi/krb5/arcfour.c +++ b/lib/gssapi/krb5/arcfour.c @@ -365,7 +365,7 @@ _gssapi_verify_mic_arcfour(OM_uint32 * minor_status, return GSS_S_FAILURE; } - cmp = ct_memcmp(cksum_data, p + 8, 8); + cmp = (ct_memcmp(cksum_data, p + 8, 8) == 0); if (cmp) { *minor_status = 0; return GSS_S_BAD_MIC; @@ -385,9 +385,9 @@ _gssapi_verify_mic_arcfour(OM_uint32 * minor_status, _gsskrb5_decode_be_om_uint32(SND_SEQ, &seq_number); if (context_handle->more_flags & LOCAL) - cmp = memcmp(&SND_SEQ[4], "\xff\xff\xff\xff", 4); + cmp = (ct_memcmp(&SND_SEQ[4], "\xff\xff\xff\xff", 4) != 0); else - cmp = (memcmp(&SND_SEQ[4], "\x00\x00\x00\x00", 4) != 0); + cmp = (ct_memcmp(&SND_SEQ[4], "\x00\x00\x00\x00", 4) != 0); memset_s(SND_SEQ, sizeof(SND_SEQ), 0, sizeof(SND_SEQ)); if (cmp != 0) { @@ -656,9 +656,9 @@ OM_uint32 _gssapi_unwrap_arcfour(OM_uint32 *minor_status, _gsskrb5_decode_be_om_uint32(SND_SEQ, &seq_number); if (context_handle->more_flags & LOCAL) - cmp = memcmp(&SND_SEQ[4], "\xff\xff\xff\xff", 4); + cmp = (ct_memcmp(&SND_SEQ[4], "\xff\xff\xff\xff", 4) != 0); else - cmp = (memcmp(&SND_SEQ[4], "\x00\x00\x00\x00", 4) != 0); + cmp = (ct_memcmp(&SND_SEQ[4], "\x00\x00\x00\x00", 4) != 0); if (cmp != 0) { *minor_status = 0; @@ -730,7 +730,7 @@ OM_uint32 _gssapi_unwrap_arcfour(OM_uint32 *minor_status, return GSS_S_FAILURE; } - cmp = ct_memcmp(cksum_data, p0 + 16, 8); /* SGN_CKSUM */ + cmp = (ct_memcmp(cksum_data, p0 + 16, 8) == 0); /* SGN_CKSUM */ if (cmp) { _gsskrb5_release_buffer(minor_status, output_message_buffer); *minor_status = 0; @@ -1272,9 +1272,9 @@ _gssapi_unwrap_iov_arcfour(OM_uint32 *minor_status, _gsskrb5_decode_be_om_uint32(snd_seq, &seq_number); if (ctx->more_flags & LOCAL) { - cmp = memcmp(&snd_seq[4], "\xff\xff\xff\xff", 4); + cmp = (ct_memcmp(&snd_seq[4], "\xff\xff\xff\xff", 4) != 0); } else { - cmp = (memcmp(&snd_seq[4], "\x00\x00\x00\x00", 4) != 0); + cmp = (ct_memcmp(&snd_seq[4], "\x00\x00\x00\x00", 4) != 0); } if (cmp != 0) { *minor_status = 0; -- 2.32.0 From c9cc34334bd64b08fe91a2f720262462e9f6bb49 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 12 Oct 2022 13:57:55 +1300 Subject: [PATCH 09/17] gsskrb5: CVE-2022-3437 Use constant-time memcmp() in unwrap_des3() The surrounding checks all use ct_memcmp(), so this one was presumably meant to as well. Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=15134 Signed-off-by: Joseph Sutton Reviewed-by: Andrew Bartlett --- lib/gssapi/krb5/unwrap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gssapi/krb5/unwrap.c b/lib/gssapi/krb5/unwrap.c index da939c052..61a341ee4 100644 --- a/lib/gssapi/krb5/unwrap.c +++ b/lib/gssapi/krb5/unwrap.c @@ -227,7 +227,7 @@ unwrap_des3 if (ret) return ret; - if (memcmp (p, "\x04\x00", 2) != 0) /* HMAC SHA1 DES3_KD */ + if (ct_memcmp (p, "\x04\x00", 2) != 0) /* HMAC SHA1 DES3_KD */ return GSS_S_BAD_SIG; p += 2; if (ct_memcmp (p, "\x02\x00", 2) == 0) { -- 2.32.0 From a587a4bcb28d5b9047f332573b1e7c8f89ca3edd Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 12 Oct 2022 13:57:42 +1300 Subject: [PATCH 10/17] gsskrb5: CVE-2022-3437 Don't pass NULL pointers to memcpy() in DES unwrap Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=15134 Signed-off-by: Joseph Sutton Reviewed-by: Andrew Bartlett --- lib/gssapi/krb5/unwrap.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/gssapi/krb5/unwrap.c b/lib/gssapi/krb5/unwrap.c index 61a341ee4..d3987240d 100644 --- a/lib/gssapi/krb5/unwrap.c +++ b/lib/gssapi/krb5/unwrap.c @@ -180,9 +180,10 @@ unwrap_des output_message_buffer->value = malloc(output_message_buffer->length); if(output_message_buffer->length != 0 && output_message_buffer->value == NULL) return GSS_S_FAILURE; - memcpy (output_message_buffer->value, - p + 24, - output_message_buffer->length); + if (output_message_buffer->value != NULL) + memcpy (output_message_buffer->value, + p + 24, + output_message_buffer->length); return GSS_S_COMPLETE; } #endif @@ -374,9 +375,10 @@ unwrap_des3 output_message_buffer->value = malloc(output_message_buffer->length); if(output_message_buffer->length != 0 && output_message_buffer->value == NULL) return GSS_S_FAILURE; - memcpy (output_message_buffer->value, - p + 36, - output_message_buffer->length); + if (output_message_buffer->value != NULL) + memcpy (output_message_buffer->value, + p + 36, + output_message_buffer->length); return GSS_S_COMPLETE; } -- 2.32.0 From c758910eaad3c0de2cfb68830a661c4739675a7d Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Mon, 15 Aug 2022 16:53:45 +1200 Subject: [PATCH 11/17] gsskrb5: CVE-2022-3437 Avoid undefined behaviour in _gssapi_verify_pad() By decrementing 'pad' only when we know it's safe, we ensure we can't stray backwards past the start of a buffer, which would be undefined behaviour. In the previous version of the loop, 'i' is the number of bytes left to check, and 'pad' is the current byte we're checking. 'pad' was decremented at the end of each loop iteration. If 'i' was 1 (so we checked the final byte), 'pad' could potentially be pointing to the first byte of the input buffer, and the decrement would put it one byte behind the buffer. That would be undefined behaviour. The patch changes it so that 'pad' is the byte we previously checked, which allows us to ensure that we only decrement it when we know we have a byte to check. Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=15134 Signed-off-by: Joseph Sutton Reviewed-by: Andrew Bartlett --- lib/gssapi/krb5/decapsulate.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/gssapi/krb5/decapsulate.c b/lib/gssapi/krb5/decapsulate.c index 86085f569..4e3fcd659 100644 --- a/lib/gssapi/krb5/decapsulate.c +++ b/lib/gssapi/krb5/decapsulate.c @@ -193,13 +193,13 @@ _gssapi_verify_pad(gss_buffer_t wrapped_token, if (wrapped_token->length < 1) return GSS_S_BAD_MECH; - pad = (u_char *)wrapped_token->value + wrapped_token->length - 1; - padlength = *pad; + pad = (u_char *)wrapped_token->value + wrapped_token->length; + padlength = pad[-1]; if (padlength > datalen) return GSS_S_BAD_MECH; - for (i = padlength; i > 0 && *pad == padlength; i--, pad--) + for (i = padlength; i > 0 && *--pad == padlength; i--) ; if (i != 0) return GSS_S_BAD_MIC; -- 2.32.0 From 414b2a77fd61c26d64562e3800dc5578d9d0f15d Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Mon, 15 Aug 2022 16:53:55 +1200 Subject: [PATCH 12/17] gsskrb5: CVE-2022-3437 Check the result of _gsskrb5_get_mech() We should make sure that the result of 'total_len - mech_len' won't overflow, and that we don't memcmp() past the end of the buffer. Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=15134 Signed-off-by: Joseph Sutton Reviewed-by: Andrew Bartlett --- lib/gssapi/krb5/decapsulate.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/gssapi/krb5/decapsulate.c b/lib/gssapi/krb5/decapsulate.c index 4e3fcd659..031a621ea 100644 --- a/lib/gssapi/krb5/decapsulate.c +++ b/lib/gssapi/krb5/decapsulate.c @@ -80,6 +80,10 @@ _gssapi_verify_mech_header(u_char **str, if (mech_len != mech->length) return GSS_S_BAD_MECH; + if (mech_len > total_len) + return GSS_S_BAD_MECH; + if (p - *str > total_len - mech_len) + return GSS_S_BAD_MECH; if (ct_memcmp(p, mech->elements, mech->length) != 0) -- 2.32.0 From be9bbd93ed8f204b4bc1b92d1bc3c16aac194696 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Mon, 15 Aug 2022 16:54:23 +1200 Subject: [PATCH 13/17] gsskrb5: CVE-2022-3437 Check buffer length against overflow for DES{,3} unwrap Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=15134 Signed-off-by: Joseph Sutton Reviewed-by: Andrew Bartlett --- lib/gssapi/krb5/unwrap.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/lib/gssapi/krb5/unwrap.c b/lib/gssapi/krb5/unwrap.c index d3987240d..fddb64bc5 100644 --- a/lib/gssapi/krb5/unwrap.c +++ b/lib/gssapi/krb5/unwrap.c @@ -64,6 +64,8 @@ unwrap_des if (IS_DCE_STYLE(context_handle)) { token_len = 22 + 8 + 15; /* 45 */ + if (input_message_buffer->length < token_len) + return GSS_S_BAD_MECH; } else { token_len = input_message_buffer->length; } @@ -76,6 +78,11 @@ unwrap_des if (ret) return ret; + len = (p - (u_char *)input_message_buffer->value) + + 22 + 8; + if (input_message_buffer->length < len) + return GSS_S_BAD_MECH; + if (memcmp (p, "\x00\x00", 2) != 0) return GSS_S_BAD_SIG; p += 2; @@ -216,6 +223,8 @@ unwrap_des3 if (IS_DCE_STYLE(context_handle)) { token_len = 34 + 8 + 15; /* 57 */ + if (input_message_buffer->length < token_len) + return GSS_S_BAD_MECH; } else { token_len = input_message_buffer->length; } @@ -228,6 +237,11 @@ unwrap_des3 if (ret) return ret; + len = (p - (u_char *)input_message_buffer->value) + + 34 + 8; + if (input_message_buffer->length < len) + return GSS_S_BAD_MECH; + if (ct_memcmp (p, "\x04\x00", 2) != 0) /* HMAC SHA1 DES3_KD */ return GSS_S_BAD_SIG; p += 2; -- 2.32.0 From c8407ca079294d76a5ed140ba5b546f870d23ed2 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Mon, 10 Oct 2022 20:33:09 +1300 Subject: [PATCH 14/17] gsskrb5: CVE-2022-3437 Check for overflow in _gsskrb5_get_mech() If len_len is equal to total_len - 1 (i.e. the input consists only of a 0x60 byte and a length), the expression 'total_len - 1 - len_len - 1', used as the 'len' parameter to der_get_length(), will overflow to SIZE_MAX. Then der_get_length() will proceed to read, unconstrained, whatever data follows in memory. Add a check to ensure that doesn't happen. Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=15134 Signed-off-by: Joseph Sutton Reviewed-by: Andrew Bartlett --- lib/gssapi/krb5/decapsulate.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/gssapi/krb5/decapsulate.c b/lib/gssapi/krb5/decapsulate.c index 031a621ea..d7b75a642 100644 --- a/lib/gssapi/krb5/decapsulate.c +++ b/lib/gssapi/krb5/decapsulate.c @@ -54,6 +54,8 @@ _gsskrb5_get_mech (const u_char *ptr, e = der_get_length (p, total_len - 1, &len, &len_len); if (e || 1 + len_len + len != total_len) return -1; + if (total_len < 1 + len_len + 1) + return -1; p += len_len; if (*p++ != 0x06) return -1; -- 2.32.0 From 8fb508a25a6a47289c73e3f4339352a73a396eef Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 12 Oct 2022 13:57:33 +1300 Subject: [PATCH 15/17] gsskrb5: CVE-2022-3437 Pass correct length to _gssapi_verify_pad() We later subtract 8 when calculating the length of the output message buffer. If padlength is excessively high, this calculation can underflow and result in a very large positive value. Now we properly constrain the value of padlength so underflow shouldn't be possible. Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=15134 Signed-off-by: Joseph Sutton Reviewed-by: Andrew Bartlett --- lib/gssapi/krb5/unwrap.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/gssapi/krb5/unwrap.c b/lib/gssapi/krb5/unwrap.c index fddb64bc5..bab30f450 100644 --- a/lib/gssapi/krb5/unwrap.c +++ b/lib/gssapi/krb5/unwrap.c @@ -124,7 +124,7 @@ unwrap_des } else { /* check pad */ ret = _gssapi_verify_pad(input_message_buffer, - input_message_buffer->length - len, + input_message_buffer->length - len - 8, &padlength); if (ret) return ret; @@ -289,7 +289,7 @@ unwrap_des3 } else { /* check pad */ ret = _gssapi_verify_pad(input_message_buffer, - input_message_buffer->length - len, + input_message_buffer->length - len - 8, &padlength); if (ret) return ret; -- 2.32.0 From 1a7a659ba7763c4dd1f428a6284790728f67104d Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 25 Oct 2022 16:58:55 -0500 Subject: [PATCH 16/17] krb5: CVE-2022-42898 PAC parse integer overflows Catch overflows that result from adding PAC_INFO_BUFFER_SIZE. Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=15203 Signed-off-by: Nico Williams --- lib/krb5/pac.c | 178 +++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 150 insertions(+), 28 deletions(-) diff --git a/lib/krb5/pac.c b/lib/krb5/pac.c index c26201be9..240845f72 100644 --- a/lib/krb5/pac.c +++ b/lib/krb5/pac.c @@ -112,6 +112,56 @@ HMAC_MD5_any_checksum(krb5_context context, } +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; +} + +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; +} + /* * */ @@ -153,8 +203,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; @@ -163,7 +217,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; @@ -292,37 +345,65 @@ 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 unaligned_len, num_buffers, len, offset, header_end, old_end; uint32_t i; - len = p->pac->numbuffers; + if (data->length > UINT32_MAX) { + ret = EINVAL; + krb5_set_error_message(context, ret, "integer overrun"); + return ret; + } + + num_buffers = p->pac->numbuffers; + + if (num_buffers >= UINT32_MAX) { + ret = EINVAL; + krb5_set_error_message(context, ret, "integer overrun"); + return ret; + } + ret = pac_header_size(context, num_buffers + 1, &header_end); + if (ret) { + return ret; + } - ptr = realloc(p->pac, - sizeof(*p->pac) + (sizeof(p->pac->buffers[0]) * len)); + ptr = realloc(p->pac, header_end); if (ptr == NULL) return krb5_enomem(context); 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"); + 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; - 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; - 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; } + unaligned_len = offset + data->length; - /* align to PAC_ALIGNMENT */ - len = ((len + PAC_ALIGNMENT - 1) / PAC_ALIGNMENT) * PAC_ALIGNMENT; + ret = pac_aligned_size(context, unaligned_len, &len); + if (ret) + return ret; ret = krb5_data_realloc(&p->data, len); if (ret) { @@ -333,7 +414,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); @@ -346,7 +427,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; @@ -375,8 +456,8 @@ krb5_pac_get_buffer(krb5_context context, krb5_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; @@ -981,8 +1062,8 @@ _krb5_pac_sign(krb5_context context, size_t server_size, priv_size; uint32_t server_offset = 0, priv_offset = 0; uint32_t server_cksumtype = 0, priv_cksumtype = 0; - int num = 0; - size_t i; + uint32_t num = 0; + uint32_t i; krb5_data logon, d; krb5_data_zero(&logon); @@ -1030,8 +1111,18 @@ _krb5_pac_sign(krb5_context context, if (num) { void *ptr; - - ptr = realloc(p->pac, sizeof(*p->pac) + (sizeof(p->pac->buffers[0]) * (p->pac->numbuffers + num - 1))); + 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; + + ptr = realloc(p->pac, len); if (ptr == NULL) return krb5_enomem(context); @@ -1084,7 +1175,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; @@ -1094,11 +1187,31 @@ _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; + } + if (end > UINT32_MAX - 4) { + ret = EINVAL; + krb5_set_error_message(context, ret, "integer overrun"); + goto out; + } len = server_size + 4; 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; + } + if (end > UINT32_MAX - 4) { + ret = EINVAL; + krb5_set_error_message(context, ret, "integer overrun"); + goto out; + } len = priv_size + 4; priv_offset = end + 4; CHECK(ret, krb5_store_uint32(spdata, priv_cksumtype), out); @@ -1129,11 +1242,20 @@ _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) { + + ret = pac_aligned_size(context, end, &e); + if (ret) + goto out; + + if (end != e) { CHECK(ret, fill_zeros(context, spdata, e - end), out); } end = e; -- 2.32.0 From aa3739c932f209e824da6849e91d2829fc7a7ea8 Mon Sep 17 00:00:00 2001 From: Jeffrey Altman Date: Wed, 17 Nov 2021 20:00:29 -0500 Subject: [PATCH 17/17] lib/wind: find_normalize read past end of array find_normalize() can under some circumstances read one element beyond the input array. The contents are discarded immediately without further use. This change prevents the unintended read. (cherry picked from commit 357a38fc7fb582ae73f4b7f4a90a4b0b871b149e) Change-Id: Ia2759a5632d64f7fa6553f879b5bbbf43ba3513e --- lib/wind/normalize.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/wind/normalize.c b/lib/wind/normalize.c index 20e8a4a04..8f3991d10 100644 --- a/lib/wind/normalize.c +++ b/lib/wind/normalize.c @@ -227,9 +227,9 @@ find_composition(const uint32_t *in, unsigned in_len) unsigned i; if (n % 5 == 0) { - cur = *in++; if (in_len-- == 0) return c->val; + cur = *in++; } i = cur >> 16; -- 2.32.0