From ed549cb0c53f8438c52593ce811f6fcc812248e9 Mon Sep 17 00:00:00 2001 From: Cy Schubert Date: Tue, 8 Nov 2022 00:53:29 -0800 Subject: [PATCH] heimdal: Fix multiple security vulnerabilities The following issues are patched: - CVE-2022-42898 PAC parse integer overflows - CVE-2022-3437 Overflows and non-constant time leaks in DES{,3} and arcfour - CVE-2021-44758 NULL dereference DoS in SPNEGO acceptors - CVE-2022-44640 Heimdal KDC: invalid free in ASN.1 codec Note that CVE-2022-44640 is a severe vulnerability, possibly a 10.0 on the Common Vulnerability Scoring System (CVSS) v3, as we believe it should be possible to get an RCE on a KDC, which means that credentials can be compromised that can be used to impersonate anyone in a realm or forest of realms. 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 extensible 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's ASN.1 compiler since 2005, but it may only affect Heimdal 1.6 and up. 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. - CVE-2019-14870: Validate client attributes in protocol-transition - CVE-2019-14870: Apply forwardable policy in protocol-transition - CVE-2019-14870: Always lookup impersonate client in DB Sponsored by: so (philip) Obtained from: so (philip) Tested by: philip, cy MFC after: immediately --- crypto/heimdal/admin/change.c | 1 - crypto/heimdal/appl/gssmask/gssmask.c | 2 + crypto/heimdal/kadmin/kadmind.c | 4 + crypto/heimdal/kadmin/mod.c | 13 +- crypto/heimdal/kadmin/stash.c | 5 +- crypto/heimdal/kcm/protocol.c | 2 +- crypto/heimdal/kdc/digest.c | 4 + crypto/heimdal/kdc/hpropd.c | 3 + crypto/heimdal/kdc/kdc-replay.c | 2 + crypto/heimdal/kdc/krb5tgs.c | 56 +++--- crypto/heimdal/kdc/kstash.c | 2 + crypto/heimdal/kdc/pkinit.c | 1 - crypto/heimdal/kuser/kdestroy.c | 2 + crypto/heimdal/kuser/kswitch.c | 5 +- crypto/heimdal/lib/asn1/der_copy.c | 8 +- crypto/heimdal/lib/asn1/gen_decode.c | 12 +- crypto/heimdal/lib/asn1/gen_free.c | 7 + .../lib/gssapi/krb5/accept_sec_context.c | 1 + crypto/heimdal/lib/gssapi/krb5/arcfour.c | 12 +- crypto/heimdal/lib/gssapi/krb5/decapsulate.c | 12 +- crypto/heimdal/lib/gssapi/krb5/unwrap.c | 34 +++- .../lib/gssapi/mech/gss_display_status.c | 3 +- .../heimdal/lib/gssapi/mech/gss_import_name.c | 2 +- .../heimdal/lib/gssapi/mech/gss_mech_switch.c | 2 + crypto/heimdal/lib/gssapi/mech/mech_locl.h | 1 + .../lib/gssapi/ntlm/init_sec_context.c | 2 + .../lib/gssapi/spnego/accept_sec_context.c | 14 +- crypto/heimdal/lib/hdb/hdb-mitdb.c | 1 - crypto/heimdal/lib/hx509/hxtool.c | 1 + crypto/heimdal/lib/hx509/ks_file.c | 8 +- crypto/heimdal/lib/hx509/name.c | 11 +- crypto/heimdal/lib/hx509/softp11.c | 6 +- crypto/heimdal/lib/ipc/client.c | 4 +- crypto/heimdal/lib/kadm5/get_s.c | 2 +- crypto/heimdal/lib/kadm5/init_c.c | 2 +- crypto/heimdal/lib/kadm5/ipropd_master.c | 5 +- crypto/heimdal/lib/kafs/afskrb5.c | 2 - crypto/heimdal/lib/krb5/acl.c | 2 +- crypto/heimdal/lib/krb5/addr_families.c | 2 +- crypto/heimdal/lib/krb5/context.c | 2 +- crypto/heimdal/lib/krb5/deprecated.c | 10 +- crypto/heimdal/lib/krb5/init_creds_pw.c | 10 +- crypto/heimdal/lib/krb5/keytab.c | 37 ++-- crypto/heimdal/lib/krb5/krb5.h | 19 ++ crypto/heimdal/lib/krb5/krb5_ccapi.h | 2 +- crypto/heimdal/lib/krb5/krbhst.c | 6 + crypto/heimdal/lib/krb5/pac.c | 178 +++++++++++++++--- crypto/heimdal/lib/krb5/rd_req.c | 9 +- crypto/heimdal/lib/krb5/test_store.c | 2 +- crypto/heimdal/lib/krb5/transited.c | 5 +- crypto/heimdal/lib/roken/getaddrinfo.c | 6 +- crypto/heimdal/lib/wind/idn-lookup.c | 6 +- crypto/heimdal/lib/wind/normalize.c | 2 +- 53 files changed, 394 insertions(+), 158 deletions(-) diff --git a/crypto/heimdal/admin/change.c b/crypto/heimdal/admin/change.c index c390441f23d..1ddbded6bf7 100644 --- a/crypto/heimdal/admin/change.c +++ b/crypto/heimdal/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/crypto/heimdal/appl/gssmask/gssmask.c b/crypto/heimdal/appl/gssmask/gssmask.c index 916837b42de..6eafc9391f5 100644 --- a/crypto/heimdal/appl/gssmask/gssmask.c +++ b/crypto/heimdal/appl/gssmask/gssmask.c @@ -949,7 +949,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/crypto/heimdal/kadmin/kadmind.c b/crypto/heimdal/kadmin/kadmind.c index f99f9572334..e52a836b9ad 100644 --- a/crypto/heimdal/kadmin/kadmind.c +++ b/crypto/heimdal/kadmin/kadmind.c @@ -116,7 +116,11 @@ main(int argc, char **argv) } argc -= optidx; +#ifndef __clang_analyzer__ argv += optidx; +#endif + if (argc != 0) + usage(1); if (config_file == NULL) { asprintf(&config_file, "%s/kdc.conf", hdb_db_dir(context)); diff --git a/crypto/heimdal/kadmin/mod.c b/crypto/heimdal/kadmin/mod.c index 940425f2a54..39b48c2e09a 100644 --- a/crypto/heimdal/kadmin/mod.c +++ b/crypto/heimdal/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/crypto/heimdal/kadmin/stash.c b/crypto/heimdal/kadmin/stash.c index f9b940ac5b7..0ca5b487a69 100644 --- a/crypto/heimdal/kadmin/stash.c +++ b/crypto/heimdal/kadmin/stash.c @@ -103,7 +103,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/crypto/heimdal/kcm/protocol.c b/crypto/heimdal/kcm/protocol.c index 0cf7157b7a7..c57a4c0b5f4 100644 --- a/crypto/heimdal/kcm/protocol.c +++ b/crypto/heimdal/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/crypto/heimdal/kdc/digest.c b/crypto/heimdal/kdc/digest.c index 9398803f04d..f6b2342f031 100644 --- a/crypto/heimdal/kdc/digest.c +++ b/crypto/heimdal/kdc/digest.c @@ -1467,6 +1467,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/crypto/heimdal/kdc/hpropd.c b/crypto/heimdal/kdc/hpropd.c index 75b26a15f50..1cfc688b2a6 100644 --- a/crypto/heimdal/kdc/hpropd.c +++ b/crypto/heimdal/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) { diff --git a/crypto/heimdal/kdc/kdc-replay.c b/crypto/heimdal/kdc/kdc-replay.c index b0510f40892..90dcf33048a 100644 --- a/crypto/heimdal/kdc/kdc-replay.c +++ b/crypto/heimdal/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/crypto/heimdal/kdc/krb5tgs.c b/crypto/heimdal/kdc/krb5tgs.c index 87e33930be1..19d66979883 100644 --- a/crypto/heimdal/kdc/krb5tgs.c +++ b/crypto/heimdal/kdc/krb5tgs.c @@ -1928,30 +1928,40 @@ tgs_build_reply(krb5_context context, 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; + } + + 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; 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", @@ -1987,10 +1997,12 @@ tgs_build_reply(krb5_context context, /* * 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/crypto/heimdal/kdc/kstash.c b/crypto/heimdal/kdc/kstash.c index 0b75fb8d84a..9a5217b27cc 100644 --- a/crypto/heimdal/kdc/kstash.c +++ b/crypto/heimdal/kdc/kstash.c @@ -126,6 +126,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/crypto/heimdal/kdc/pkinit.c b/crypto/heimdal/kdc/pkinit.c index 75edda464f4..c3955aaf66a 100644 --- a/crypto/heimdal/kdc/pkinit.c +++ b/crypto/heimdal/kdc/pkinit.c @@ -249,7 +249,6 @@ generate_dh_keyblock(krb5_context context, memset(dh_gen_key, 0, size); } - ret = 0; #ifdef HAVE_OPENSSL } else if (client_params->keyex == USE_ECDH) { diff --git a/crypto/heimdal/kuser/kdestroy.c b/crypto/heimdal/kuser/kdestroy.c index 1823bf56ca4..feabe55fdcd 100644 --- a/crypto/heimdal/kuser/kdestroy.c +++ b/crypto/heimdal/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/crypto/heimdal/kuser/kswitch.c b/crypto/heimdal/kuser/kswitch.c index cdb6ee11cae..4887d43be15 100644 --- a/crypto/heimdal/kuser/kswitch.c +++ b/crypto/heimdal/kuser/kswitch.c @@ -86,14 +86,15 @@ kswitch(struct kswitch_options *opt, int argc, char **argv) krb5_err(kcc_context, 1, ret, "krb5_cc_cache_get_first"); while (krb5_cc_cache_next(kcc_context, cursor, &id) == 0) { - krb5_principal p; + krb5_principal p = NULL; char num[10]; ret = krb5_cc_get_principal(kcc_context, id, &p); + if (ret == 0) + ret = krb5_unparse_name(kcc_context, p, &name); if (ret) continue; - ret = krb5_unparse_name(kcc_context, p, &name); krb5_free_principal(kcc_context, p); snprintf(num, sizeof(num), "%d", (int)(len + 1)); diff --git a/crypto/heimdal/lib/asn1/der_copy.c b/crypto/heimdal/lib/asn1/der_copy.c index 3a0a8c5ffa6..abaaf8e5d74 100644 --- a/crypto/heimdal/lib/asn1/der_copy.c +++ b/crypto/heimdal/lib/asn1/der_copy.c @@ -135,8 +135,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/crypto/heimdal/lib/asn1/gen_decode.c b/crypto/heimdal/lib/asn1/gen_decode.c index 9d816d5400d..bf2d93b806d 100644 --- a/crypto/heimdal/lib/asn1/gen_decode.c +++ b/crypto/heimdal/lib/asn1/gen_decode.c @@ -584,14 +584,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"); @@ -600,23 +600,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/crypto/heimdal/lib/asn1/gen_free.c b/crypto/heimdal/lib/asn1/gen_free.c index b9cae7533b1..74449fe6ca8 100644 --- a/crypto/heimdal/lib/asn1/gen_free.c +++ b/crypto/heimdal/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/crypto/heimdal/lib/gssapi/krb5/accept_sec_context.c b/crypto/heimdal/lib/gssapi/krb5/accept_sec_context.c index 5a00e124c2c..baeafb95efa 100644 --- a/crypto/heimdal/lib/gssapi/krb5/accept_sec_context.c +++ b/crypto/heimdal/lib/gssapi/krb5/accept_sec_context.c @@ -425,6 +425,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/crypto/heimdal/lib/gssapi/krb5/arcfour.c b/crypto/heimdal/lib/gssapi/krb5/arcfour.c index b4ef8d39ffd..3b8d452877d 100644 --- a/crypto/heimdal/lib/gssapi/krb5/arcfour.c +++ b/crypto/heimdal/lib/gssapi/krb5/arcfour.c @@ -307,7 +307,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; @@ -331,9 +331,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); + cmp = (ct_memcmp(&SND_SEQ[4], "\x00\x00\x00\x00", 4) != 0); memset(SND_SEQ, 0, sizeof(SND_SEQ)); if (cmp != 0) { @@ -616,9 +616,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); + cmp = (ct_memcmp(&SND_SEQ[4], "\x00\x00\x00\x00", 4) != 0); if (cmp != 0) { *minor_status = 0; @@ -695,7 +695,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; diff --git a/crypto/heimdal/lib/gssapi/krb5/decapsulate.c b/crypto/heimdal/lib/gssapi/krb5/decapsulate.c index 640c064d0bf..343a3d7acb9 100644 --- a/crypto/heimdal/lib/gssapi/krb5/decapsulate.c +++ b/crypto/heimdal/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; @@ -80,6 +82,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) @@ -190,13 +196,13 @@ _gssapi_verify_pad(gss_buffer_t wrapped_token, size_t padlength; int i; - 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; diff --git a/crypto/heimdal/lib/gssapi/krb5/unwrap.c b/crypto/heimdal/lib/gssapi/krb5/unwrap.c index 5a003815a0f..640677e20b9 100644 --- a/crypto/heimdal/lib/gssapi/krb5/unwrap.c +++ b/crypto/heimdal/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; @@ -122,7 +129,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; @@ -195,9 +202,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 @@ -230,6 +238,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; } @@ -242,7 +252,12 @@ unwrap_des3 if (ret) return ret; - if (memcmp (p, "\x04\x00", 2) != 0) /* HMAC SHA1 DES3_KD */ + 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; if (ct_memcmp (p, "\x02\x00", 2) == 0) { @@ -289,7 +304,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; @@ -389,9 +404,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; } diff --git a/crypto/heimdal/lib/gssapi/mech/gss_display_status.c b/crypto/heimdal/lib/gssapi/mech/gss_display_status.c index 1e508caa9ba..9529fabf319 100644 --- a/crypto/heimdal/lib/gssapi/mech/gss_display_status.c +++ b/crypto/heimdal/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/crypto/heimdal/lib/gssapi/mech/gss_import_name.c b/crypto/heimdal/lib/gssapi/mech/gss_import_name.c index d1b3dc95b4a..83077022358 100644 --- a/crypto/heimdal/lib/gssapi/mech/gss_import_name.c +++ b/crypto/heimdal/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/crypto/heimdal/lib/gssapi/mech/gss_mech_switch.c b/crypto/heimdal/lib/gssapi/mech/gss_mech_switch.c index 55e01094ff9..6a100d391c5 100644 --- a/crypto/heimdal/lib/gssapi/mech/gss_mech_switch.c +++ b/crypto/heimdal/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/crypto/heimdal/lib/gssapi/mech/mech_locl.h b/crypto/heimdal/lib/gssapi/mech/mech_locl.h index 6c23ac5256b..0f4d8e51b2c 100644 --- a/crypto/heimdal/lib/gssapi/mech/mech_locl.h +++ b/crypto/heimdal/lib/gssapi/mech/mech_locl.h @@ -51,6 +51,7 @@ #include +#include #include #include #include diff --git a/crypto/heimdal/lib/gssapi/ntlm/init_sec_context.c b/crypto/heimdal/lib/gssapi/ntlm/init_sec_context.c index bae04e17406..48fc03b5ff5 100644 --- a/crypto/heimdal/lib/gssapi/ntlm/init_sec_context.c +++ b/crypto/heimdal/lib/gssapi/ntlm/init_sec_context.c @@ -52,6 +52,8 @@ from_file(const char *fn, const char *target_domain, continue; str = NULL; d = strtok_r(buf, ":", &str); + if (!d) + continue; if (d && strcasecmp(target_domain, d) != 0) continue; u = strtok_r(NULL, ":", &str); diff --git a/crypto/heimdal/lib/gssapi/spnego/accept_sec_context.c b/crypto/heimdal/lib/gssapi/spnego/accept_sec_context.c index 3a51dd3a0a6..b60dc19ad8e 100644 --- a/crypto/heimdal/lib/gssapi/spnego/accept_sec_context.c +++ b/crypto/heimdal/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; } /* diff --git a/crypto/heimdal/lib/hdb/hdb-mitdb.c b/crypto/heimdal/lib/hdb/hdb-mitdb.c index cd619b3b8eb..02c575050fe 100644 --- a/crypto/heimdal/lib/hdb/hdb-mitdb.c +++ b/crypto/heimdal/lib/hdb/hdb-mitdb.c @@ -720,7 +720,6 @@ mdb_remove(krb5_context context, HDB *db, krb5_const_principal principal) krb5_error_code code; krb5_data key; - mdb_principal2key(context, principal, &key); code = db->hdb__del(context, db, key); krb5_data_free(&key); return code; diff --git a/crypto/heimdal/lib/hx509/hxtool.c b/crypto/heimdal/lib/hx509/hxtool.c index 06c7958592f..690ee5da612 100644 --- a/crypto/heimdal/lib/hx509/hxtool.c +++ b/crypto/heimdal/lib/hx509/hxtool.c @@ -1288,6 +1288,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/crypto/heimdal/lib/hx509/ks_file.c b/crypto/heimdal/lib/hx509/ks_file.c index 6aa36f4e204..b8404a60f66 100644 --- a/crypto/heimdal/lib/hx509/ks_file.c +++ b/crypto/heimdal/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/crypto/heimdal/lib/hx509/name.c b/crypto/heimdal/lib/hx509/name.c index efd7b703422..ffb67c85e57 100644 --- a/crypto/heimdal/lib/hx509/name.c +++ b/crypto/heimdal/lib/hx509/name.c @@ -938,6 +938,7 @@ int hx509_general_name_unparse(GeneralName *name, char **str) { struct rk_strpool *strpool = NULL; + int ret = 0; *str = NULL; @@ -964,7 +965,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 = name->u.directoryName.element; dir.u.rdnSequence = name->u.directoryName.u.rdnSequence; @@ -1017,10 +1017,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/crypto/heimdal/lib/hx509/softp11.c b/crypto/heimdal/lib/hx509/softp11.c index 38f587e0fea..6a516a50cb6 100644 --- a/crypto/heimdal/lib/hx509/softp11.c +++ b/crypto/heimdal/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/crypto/heimdal/lib/ipc/client.c b/crypto/heimdal/lib/ipc/client.c index bb7d9750bff..809fc6a0104 100644 --- a/crypto/heimdal/lib/ipc/client.c +++ b/crypto/heimdal/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/crypto/heimdal/lib/kadm5/get_s.c b/crypto/heimdal/lib/kadm5/get_s.c index e03585e222a..6c456e11831 100644 --- a/crypto/heimdal/lib/kadm5/get_s.c +++ b/crypto/heimdal/lib/kadm5/get_s.c @@ -246,7 +246,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/crypto/heimdal/lib/kadm5/init_c.c b/crypto/heimdal/lib/kadm5/init_c.c index 1623ed1a995..5b83d2087a2 100644 --- a/crypto/heimdal/lib/kadm5/init_c.c +++ b/crypto/heimdal/lib/kadm5/init_c.c @@ -567,7 +567,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/crypto/heimdal/lib/kadm5/ipropd_master.c b/crypto/heimdal/lib/kadm5/ipropd_master.c index e05e9ee5586..10d4202e079 100644 --- a/crypto/heimdal/lib/kadm5/ipropd_master.c +++ b/crypto/heimdal/lib/kadm5/ipropd_master.c @@ -755,7 +755,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/crypto/heimdal/lib/kafs/afskrb5.c b/crypto/heimdal/lib/kafs/afskrb5.c index c04f43abbc2..919a6ee6354 100644 --- a/crypto/heimdal/lib/kafs/afskrb5.c +++ b/crypto/heimdal/lib/kafs/afskrb5.c @@ -89,8 +89,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/crypto/heimdal/lib/krb5/acl.c b/crypto/heimdal/lib/krb5/acl.c index c94aae361b8..cf8869a5e07 100644 --- a/crypto/heimdal/lib/krb5/acl.c +++ b/crypto/heimdal/lib/krb5/acl.c @@ -248,7 +248,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/crypto/heimdal/lib/krb5/addr_families.c b/crypto/heimdal/lib/krb5/addr_families.c index 5d321a7e917..6358b35b597 100644 --- a/crypto/heimdal/lib/krb5/addr_families.c +++ b/crypto/heimdal/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/crypto/heimdal/lib/krb5/context.c b/crypto/heimdal/lib/krb5/context.c index 99bf1b419b0..86bfe539b97 100644 --- a/crypto/heimdal/lib/krb5/context.c +++ b/crypto/heimdal/lib/krb5/context.c @@ -97,7 +97,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, 3, "kdc_timeout"); diff --git a/crypto/heimdal/lib/krb5/deprecated.c b/crypto/heimdal/lib/krb5/deprecated.c index 1d44d21b170..e7c0105ebf7 100644 --- a/crypto/heimdal/lib/krb5/deprecated.c +++ b/crypto/heimdal/lib/krb5/deprecated.c @@ -325,15 +325,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/crypto/heimdal/lib/krb5/init_creds_pw.c b/crypto/heimdal/lib/krb5/init_creds_pw.c index 37f4147c372..9f7c2f3eea3 100644 --- a/crypto/heimdal/lib/krb5/init_creds_pw.c +++ b/crypto/heimdal/lib/krb5/init_creds_pw.c @@ -1491,15 +1491,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/crypto/heimdal/lib/krb5/keytab.c b/crypto/heimdal/lib/krb5/keytab.c index 8ca515f2133..862d988d7a1 100644 --- a/crypto/heimdal/lib/krb5/keytab.c +++ b/crypto/heimdal/lib/krb5/keytab.c @@ -348,10 +348,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 @@ -361,11 +362,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; } @@ -473,11 +474,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; } @@ -620,6 +623,7 @@ krb5_kt_get_entry(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 @@ -674,21 +678,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; } /** @@ -869,6 +873,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/crypto/heimdal/lib/krb5/krb5.h b/crypto/heimdal/lib/krb5/krb5.h index 2d555ea0604..9c9b68f6d8c 100644 --- a/crypto/heimdal/lib/krb5/krb5.h +++ b/crypto/heimdal/lib/krb5/krb5.h @@ -914,3 +914,22 @@ extern KRB5_LIB_VARIABLE const char *krb5_cc_type_scc; #endif /* __KRB5_H__ */ +/* 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 + diff --git a/crypto/heimdal/lib/krb5/krb5_ccapi.h b/crypto/heimdal/lib/krb5/krb5_ccapi.h index 5a7fe6a4133..89e5665afc7 100644 --- a/crypto/heimdal/lib/krb5/krb5_ccapi.h +++ b/crypto/heimdal/lib/krb5/krb5_ccapi.h @@ -38,7 +38,7 @@ #include - #ifdef __APPLE__ +#ifdef __APPLE__ #pragma pack(push,2) #endif diff --git a/crypto/heimdal/lib/krb5/krbhst.c b/crypto/heimdal/lib/krb5/krbhst.c index 3242cdb9995..2d61f6c4b0b 100644 --- a/crypto/heimdal/lib/krb5/krbhst.c +++ b/crypto/heimdal/lib/krb5/krbhst.c @@ -96,6 +96,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/crypto/heimdal/lib/krb5/pac.c b/crypto/heimdal/lib/krb5/pac.c index 91f68d5e00e..1fe86e536be 100644 --- a/crypto/heimdal/lib/krb5/pac.c +++ b/crypto/heimdal/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 > 1000) { + 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; @@ -963,8 +1044,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); @@ -978,8 +1059,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); @@ -1032,7 +1123,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; @@ -1042,11 +1135,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); @@ -1077,11 +1190,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; diff --git a/crypto/heimdal/lib/krb5/rd_req.c b/crypto/heimdal/lib/krb5/rd_req.c index 21daeb596b5..7130a07efc6 100644 --- a/crypto/heimdal/lib/krb5/rd_req.c +++ b/crypto/heimdal/lib/krb5/rd_req.c @@ -802,11 +802,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/crypto/heimdal/lib/krb5/test_store.c b/crypto/heimdal/lib/krb5/test_store.c index 6b930775c0c..746e3509f51 100644 --- a/crypto/heimdal/lib/krb5/test_store.c +++ b/crypto/heimdal/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/crypto/heimdal/lib/krb5/transited.c b/crypto/heimdal/lib/krb5/transited.c index 5e21987bca9..09e540a2bed 100644 --- a/crypto/heimdal/lib/krb5/transited.c +++ b/crypto/heimdal/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/crypto/heimdal/lib/roken/getaddrinfo.c b/crypto/heimdal/lib/roken/getaddrinfo.c index c8ed95413fe..ae21bf11090 100644 --- a/crypto/heimdal/lib/roken/getaddrinfo.c +++ b/crypto/heimdal/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/crypto/heimdal/lib/wind/idn-lookup.c b/crypto/heimdal/lib/wind/idn-lookup.c index 1bc63a33dd8..378c912a392 100644 --- a/crypto/heimdal/lib/wind/idn-lookup.c +++ b/crypto/heimdal/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/crypto/heimdal/lib/wind/normalize.c b/crypto/heimdal/lib/wind/normalize.c index 15274f6855e..67fc1de9218 100644 --- a/crypto/heimdal/lib/wind/normalize.c +++ b/crypto/heimdal/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.45.0