git: f8a825bde75c - stable/12 - heimdal: Fix multiple security vulnerabilities
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Tue, 15 Nov 2022 21:17:12 UTC
The branch stable/12 has been updated by cy: URL: https://cgit.FreeBSD.org/src/commit/?id=f8a825bde75cb5164a896c9234979e9c18e97361 commit f8a825bde75cb5164a896c9234979e9c18e97361 Author: Cy Schubert <cy@FreeBSD.org> AuthorDate: 2022-11-08 08:53:29 +0000 Commit: Cy Schubert <cy@FreeBSD.org> CommitDate: 2022-11-15 21:16:56 +0000 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 (cherry picked from commit ed549cb0c53f8438c52593ce811f6fcc812248e9) --- 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 + .../heimdal/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 ++-- .../heimdal/lib/gssapi/mech/gss_display_status.c | 3 +- crypto/heimdal/lib/gssapi/mech/gss_import_name.c | 2 +- crypto/heimdal/lib/gssapi/mech/gss_mech_switch.c | 2 + crypto/heimdal/lib/gssapi/mech/mech_locl.h | 1 + crypto/heimdal/lib/gssapi/ntlm/init_sec_context.c | 2 + .../heimdal/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 c390441f23dc..1ddbded6bf77 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 916837b42de1..6eafc9391f54 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 f99f9572334a..e52a836b9ad6 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 940425f2a54b..39b48c2e09a6 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 f9b940ac5b7d..0ca5b487a699 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 0cf7157b7a71..c57a4c0b5f49 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 9398803f04d2..f6b2342f0318 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 75b26a15f501..1cfc688b2a6c 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 b0510f408924..90dcf33048a7 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 87e33930be15..19d669798830 100644 --- a/crypto/heimdal/kdc/krb5tgs.c +++ b/crypto/heimdal/kdc/krb5tgs.c @@ -1928,30 +1928,40 @@ 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; + } + + 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 @@ 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/crypto/heimdal/kdc/kstash.c b/crypto/heimdal/kdc/kstash.c index 0b75fb8d84a1..9a5217b27ccf 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 75edda464f4e..c3955aaf66a7 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 1823bf56ca48..feabe55fdcdb 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 cdb6ee11cae8..4887d43be15c 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 3a0a8c5ffa6a..abaaf8e5d740 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 9d816d5400d7..bf2d93b806df 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 b9cae7533b17..74449fe6ca82 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 5a00e124c2cf..baeafb95efaf 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 b4ef8d39ffd7..3b8d452877dd 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 640c064d0bf1..343a3d7acb97 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 5a003815a0f3..640677e20b9a 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 1e508caa9baf..9529fabf319d 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 d1b3dc95b4a4..830770223583 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 55e01094ff91..6a100d391c5a 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 6c23ac5256b1..0f4d8e51b2c3 100644 --- a/crypto/heimdal/lib/gssapi/mech/mech_locl.h +++ b/crypto/heimdal/lib/gssapi/mech/mech_locl.h @@ -51,6 +51,7 @@ #include <roken.h> +#include <krb5.h> #include <gssapi.h> #include <gssapi_mech.h> #include <gssapi_krb5.h> diff --git a/crypto/heimdal/lib/gssapi/ntlm/init_sec_context.c b/crypto/heimdal/lib/gssapi/ntlm/init_sec_context.c index bae04e174060..48fc03b5ff5e 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 3a51dd3a0a61..b60dc19ad8e3 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 cd619b3b8eb4..02c575050fe2 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 06c7958592ff..690ee5da612b 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 6aa36f4e204e..b8404a60f66f 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 efd7b703422f..ffb67c85e574 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 38f587e0fea2..6a516a50cb66 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 bb7d9750bffa..809fc6a01047 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 e03585e222a8..6c456e118316 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 1623ed1a995d..5b83d2087a25 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 e05e9ee55864..10d4202e0791 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, "<error-formatting-time>"); + 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 c04f43abbc25..919a6ee63542 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 c94aae361b8e..cf8869a5e07b 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 5d321a7e917d..6358b35b5974 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 *** 629 LINES SKIPPED ***