--- crypto/heimdal/admin/change.c.orig +++ crypto/heimdal/admin/change.c @@ -217,7 +217,6 @@ 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; --- crypto/heimdal/appl/gssmask/gssmask.c.orig +++ crypto/heimdal/appl/gssmask/gssmask.c @@ -949,7 +949,9 @@ 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); --- crypto/heimdal/kadmin/kadmind.c.orig +++ crypto/heimdal/kadmin/kadmind.c @@ -116,7 +116,11 @@ } 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)); --- crypto/heimdal/kadmin/mod.c.orig +++ crypto/heimdal/kadmin/mod.c @@ -106,7 +106,7 @@ 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 @@ 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); } } --- crypto/heimdal/kadmin/stash.c.orig +++ crypto/heimdal/kadmin/stash.c @@ -103,7 +103,10 @@ } } 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); } --- crypto/heimdal/kcm/protocol.c.orig +++ crypto/heimdal/kcm/protocol.c @@ -423,7 +423,7 @@ free(name); kcm_release_ccache(context, ccache); - return 0; + return ret; } /* --- crypto/heimdal/kdc/digest.c.orig +++ crypto/heimdal/kdc/digest.c @@ -1467,6 +1467,10 @@ 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) { --- crypto/heimdal/kdc/hpropd.c.orig +++ crypto/heimdal/kdc/hpropd.c @@ -107,7 +107,9 @@ } argc -= optidx; +#ifndef __clang_analyzer__ argv += optidx; +#endif if (argc != 0) usage(1); @@ -125,6 +127,7 @@ krb5_ticket *ticket; char *server; + memset(&ss, 0, sizeof(ss)); sock = STDIN_FILENO; #ifdef SUPPORT_INETD if (inetd_flag == -1) { --- crypto/heimdal/kdc/kdc-replay.c.orig +++ crypto/heimdal/kdc/kdc-replay.c @@ -184,6 +184,8 @@ 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); --- crypto/heimdal/kdc/krb5tgs.c.orig +++ crypto/heimdal/kdc/krb5tgs.c @@ -1928,30 +1928,40 @@ 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 @@ /* * 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; --- crypto/heimdal/kdc/kstash.c.orig +++ crypto/heimdal/kdc/kstash.c @@ -126,6 +126,8 @@ 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); --- crypto/heimdal/kdc/pkinit.c.orig +++ crypto/heimdal/kdc/pkinit.c @@ -249,7 +249,6 @@ memset(dh_gen_key, 0, size); } - ret = 0; #ifdef HAVE_OPENSSL } else if (client_params->keyex == USE_ECDH) { --- crypto/heimdal/kuser/kdestroy.c.orig +++ crypto/heimdal/kuser/kdestroy.c @@ -90,7 +90,9 @@ } argc -= optidx; +#ifndef __clang_analyzer__ argv += optidx; +#endif if (argc != 0) usage (1); --- crypto/heimdal/kuser/kswitch.c.orig +++ crypto/heimdal/kuser/kswitch.c @@ -86,14 +86,15 @@ 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)); --- crypto/heimdal/lib/asn1/der_copy.c.orig +++ crypto/heimdal/lib/asn1/der_copy.c @@ -135,8 +135,12 @@ 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; --- crypto/heimdal/lib/asn1/gen_decode.c.orig +++ crypto/heimdal/lib/asn1/gen_decode.c @@ -584,14 +584,14 @@ 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 @@ 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" --- crypto/heimdal/lib/asn1/gen_free.c.orig +++ crypto/heimdal/lib/asn1/gen_free.c @@ -61,6 +61,13 @@ 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)) --- crypto/heimdal/lib/gssapi/krb5/accept_sec_context.c.orig +++ crypto/heimdal/lib/gssapi/krb5/accept_sec_context.c @@ -425,6 +425,7 @@ * 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) { --- crypto/heimdal/lib/gssapi/krb5/arcfour.c.orig +++ crypto/heimdal/lib/gssapi/krb5/arcfour.c @@ -307,7 +307,7 @@ 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 @@ _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 @@ _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 @@ 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; --- crypto/heimdal/lib/gssapi/krb5/decapsulate.c.orig +++ crypto/heimdal/lib/gssapi/krb5/decapsulate.c @@ -54,6 +54,8 @@ 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 @@ 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 @@ 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; --- crypto/heimdal/lib/gssapi/krb5/unwrap.c.orig +++ crypto/heimdal/lib/gssapi/krb5/unwrap.c @@ -64,6 +64,8 @@ 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 @@ 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 @@ } 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 @@ 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 @@ 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 @@ 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 @@ } 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 @@ 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; } --- crypto/heimdal/lib/gssapi/mech/gss_display_status.c.orig +++ crypto/heimdal/lib/gssapi/mech/gss_display_status.c @@ -91,8 +91,7 @@ "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", --- crypto/heimdal/lib/gssapi/mech/gss_import_name.c.orig +++ crypto/heimdal/lib/gssapi/mech/gss_import_name.c @@ -113,7 +113,7 @@ len -= t; t = (p[0] << 24) | (p[1] << 16) | (p[2] << 8) | p[3]; - p += 4; + /* p += 4; */ len -= 4; if (!composite && len != t) --- crypto/heimdal/lib/gssapi/mech/gss_mech_switch.c.orig +++ crypto/heimdal/lib/gssapi/mech/gss_mech_switch.c @@ -137,6 +137,8 @@ } } } + if (byte_count == 0) + return EINVAL; if (!res) { res = malloc(byte_count); if (!res) --- crypto/heimdal/lib/gssapi/mech/mech_locl.h.orig +++ crypto/heimdal/lib/gssapi/mech/mech_locl.h @@ -51,6 +51,7 @@ #include +#include #include #include #include --- crypto/heimdal/lib/gssapi/ntlm/init_sec_context.c.orig +++ crypto/heimdal/lib/gssapi/ntlm/init_sec_context.c @@ -52,6 +52,8 @@ continue; str = NULL; d = strtok_r(buf, ":", &str); + if (!d) + continue; if (d && strcasecmp(target_domain, d) != 0) continue; u = strtok_r(NULL, ":", &str); --- crypto/heimdal/lib/gssapi/spnego/accept_sec_context.c.orig +++ crypto/heimdal/lib/gssapi/spnego/accept_sec_context.c @@ -619,13 +619,15 @@ 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; } /* --- crypto/heimdal/lib/hdb/hdb-mitdb.c.orig +++ crypto/heimdal/lib/hdb/hdb-mitdb.c @@ -720,7 +720,6 @@ 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; --- crypto/heimdal/lib/hx509/hxtool.c.orig +++ crypto/heimdal/lib/hx509/hxtool.c @@ -1288,6 +1288,7 @@ const char *outfile = argv[0]; memset(&key, 0, sizeof(key)); + memset(&signer, 0, sizeof(signer)); get_key(opt->key_string, opt->generate_key_string, --- crypto/heimdal/lib/hx509/ks_file.c.orig +++ crypto/heimdal/lib/hx509/ks_file.c @@ -533,7 +533,7 @@ { 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 @@ 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 --- crypto/heimdal/lib/hx509/name.c.orig +++ crypto/heimdal/lib/hx509/name.c @@ -938,6 +938,7 @@ hx509_general_name_unparse(GeneralName *name, char **str) { struct rk_strpool *strpool = NULL; + int ret = 0; *str = NULL; @@ -964,7 +965,6 @@ 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 @@ 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; } --- crypto/heimdal/lib/hx509/softp11.c.orig +++ crypto/heimdal/lib/hx509/softp11.c @@ -342,6 +342,9 @@ 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 @@ 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++; --- crypto/heimdal/lib/ipc/client.c.orig +++ crypto/heimdal/lib/ipc/client.c @@ -332,10 +332,8 @@ 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; } --- crypto/heimdal/lib/kadm5/get_s.c.orig +++ crypto/heimdal/lib/kadm5/get_s.c @@ -246,7 +246,7 @@ 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); --- crypto/heimdal/lib/kadm5/init_c.c.orig +++ crypto/heimdal/lib/kadm5/init_c.c @@ -567,7 +567,7 @@ 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); --- crypto/heimdal/lib/kadm5/ipropd_master.c.orig +++ crypto/heimdal/lib/kadm5/ipropd_master.c @@ -755,7 +755,10 @@ 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; } --- crypto/heimdal/lib/kafs/afskrb5.c.orig +++ crypto/heimdal/lib/kafs/afskrb5.c @@ -89,8 +89,6 @@ return ENOMEM; kt->ticket_len = cred->ticket.length; memcpy(kt->ticket, cred->ticket.data, kt->ticket_len); - - ret = 0; } --- crypto/heimdal/lib/krb5/acl.c.orig +++ crypto/heimdal/lib/krb5/acl.c @@ -248,7 +248,7 @@ ...) { krb5_error_code ret; - struct acl_field *acl; + struct acl_field *acl = NULL; char buf[256]; va_list ap; FILE *f; --- crypto/heimdal/lib/krb5/addr_families.c.orig +++ crypto/heimdal/lib/krb5/addr_families.c @@ -525,7 +525,7 @@ 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; --- crypto/heimdal/lib/krb5/context.c.orig +++ crypto/heimdal/lib/krb5/context.c @@ -97,7 +97,7 @@ 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"); --- crypto/heimdal/lib/krb5/deprecated.c.orig +++ crypto/heimdal/lib/krb5/deprecated.c @@ -325,15 +325,13 @@ 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; } --- crypto/heimdal/lib/krb5/init_creds_pw.c.orig +++ crypto/heimdal/lib/krb5/init_creds_pw.c @@ -1491,15 +1491,13 @@ 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; } --- crypto/heimdal/lib/krb5/keytab.c.orig +++ crypto/heimdal/lib/krb5/keytab.c @@ -348,10 +348,11 @@ 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 @@ 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_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 @@ 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_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_error_code ret; char *name; + memset(&entry, 0, sizeof(entry)); ret = krb5_kt_start_seq_get(context, id, &cursor); if (ret) goto notfound; --- crypto/heimdal/lib/krb5/krb5.h.orig +++ crypto/heimdal/lib/krb5/krb5.h @@ -914,3 +914,22 @@ #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 + --- crypto/heimdal/lib/krb5/krb5_ccapi.h.orig +++ crypto/heimdal/lib/krb5/krb5_ccapi.h @@ -38,7 +38,7 @@ #include - #ifdef __APPLE__ +#ifdef __APPLE__ #pragma pack(push,2) #endif --- crypto/heimdal/lib/krb5/krbhst.c.orig +++ crypto/heimdal/lib/krb5/krbhst.c @@ -96,6 +96,12 @@ 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); --- crypto/heimdal/lib/krb5/pac.c.orig +++ crypto/heimdal/lib/krb5/pac.c @@ -112,6 +112,56 @@ } +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 @@ 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 @@ 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_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 @@ /* * 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ /* 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 @@ /* 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; --- crypto/heimdal/lib/krb5/rd_req.c.orig +++ crypto/heimdal/lib/krb5/rd_req.c @@ -802,11 +802,10 @@ 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); --- crypto/heimdal/lib/krb5/test_store.c.orig +++ crypto/heimdal/lib/krb5/test_store.c @@ -64,7 +64,7 @@ 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); --- crypto/heimdal/lib/krb5/transited.c.orig +++ crypto/heimdal/lib/krb5/transited.c @@ -281,6 +281,7 @@ 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 @@ } 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 @@ r = make_realm(tmp); if(r == NULL){ free_realms(*realms); + *realms = NULL; return krb5_enomem(context); } *realms = append_realm(*realms, r); --- crypto/heimdal/lib/roken/getaddrinfo.c.orig +++ crypto/heimdal/lib/roken/getaddrinfo.c @@ -188,7 +188,7 @@ 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 @@ 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 @@ ¤t, const_v4, &v4_addr, NULL); } *res = first; - return 0; + return ret; } static int --- crypto/heimdal/lib/wind/idn-lookup.c.orig +++ crypto/heimdal/lib/wind/idn-lookup.c @@ -156,7 +156,9 @@ 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; } --- crypto/heimdal/lib/wind/normalize.c.orig +++ crypto/heimdal/lib/wind/normalize.c @@ -227,9 +227,9 @@ unsigned i; if (n % 5 == 0) { - cur = *in++; if (in_len-- == 0) return c->val; + cur = *in++; } i = cur >> 16; --