From 3ba12317a0e1948820827987ae9d222b85efae10 Mon Sep 17 00:00:00 2001 From: Nicolas Williams Date: Mon, 28 Nov 2016 15:09:55 -0600 Subject: [PATCH] Misc fixes (coverity) --- kcm/acquire.c | 9 ++-- kcm/protocol.c | 8 +-- kpasswd/kpasswd-generator.c | 124 +++++++++++++++++++++++--------------------- lib/hx509/ks_file.c | 5 +- lib/hx509/softp11.c | 3 ++ lib/kadm5/init_c.c | 10 ++-- lib/krb5/auth_context.c | 4 +- lib/krb5/krbhst.c | 4 +- lib/krb5/pkinit.c | 4 +- lib/krb5/rd_req.c | 31 +++++------ lib/krb5/send_to_kdc.c | 2 + lib/krb5/verify_krb5_conf.c | 19 +++---- 12 files changed, 116 insertions(+), 107 deletions(-) diff --git a/kcm/acquire.c b/kcm/acquire.c index 562b77b81..a5450c05a 100644 --- a/kcm/acquire.c +++ b/kcm/acquire.c @@ -50,6 +50,7 @@ kcm_ccache_acquire(krb5_context context, char *in_tkt_service = NULL; const char *estr; + *credp = NULL; memset(&cred, 0, sizeof(cred)); KCM_ASSERT_VALID(ccache); @@ -82,7 +83,7 @@ kcm_ccache_acquire(krb5_context context, kcm_log(0, "Failed to unparse service principal name for cache %s: %s", ccache->name, estr); krb5_free_error_message(context, estr); - return ret; + goto out; } } @@ -121,14 +122,9 @@ kcm_ccache_acquire(krb5_context context, kcm_log(0, "Failed to acquire credentials for cache %s: %s", ccache->name, estr); krb5_free_error_message(context, estr); - if (in_tkt_service != NULL) - free(in_tkt_service); goto out; } - if (in_tkt_service != NULL) - free(in_tkt_service); - /* Swap them in */ kcm_ccache_remove_creds_internal(context, ccache); @@ -143,6 +139,7 @@ kcm_ccache_acquire(krb5_context context, } out: + free(in_tkt_service); if (opt) krb5_get_init_creds_opt_free(context, opt); diff --git a/kcm/protocol.c b/kcm/protocol.c index b9b8d8a03..c36bbe9c6 100644 --- a/kcm/protocol.c +++ b/kcm/protocol.c @@ -1137,17 +1137,19 @@ kcm_op_set_default_cache(krb5_context context, } if (c == NULL) { c = malloc(sizeof(*c)); - if (c == NULL) + if (c == NULL) { + free(name); return ENOMEM; + } c->session = client->session; c->uid = client->uid; - c->name = strdup(name); + c->name = name; c->next = default_caches; default_caches = c; } else { free(c->name); - c->name = strdup(name); + c->name = name; } return 0; diff --git a/kpasswd/kpasswd-generator.c b/kpasswd/kpasswd-generator.c index deb6d8e81..4fb49453e 100644 --- a/kpasswd/kpasswd-generator.c +++ b/kpasswd/kpasswd-generator.c @@ -36,24 +36,24 @@ RCSID("$Id$"); static unsigned -read_words (const char *filename, char ***ret_w) +read_words(const char *filename, char ***ret_w) { unsigned n, alloc; FILE *f; char buf[256]; char **w = NULL; - f = fopen (filename, "r"); + f = fopen(filename, "r"); if (f == NULL) - err (1, "cannot open %s", filename); + err(1, "cannot open %s", filename); alloc = n = 0; - while (fgets (buf, sizeof(buf), f) != NULL) { + while (fgets(buf, sizeof(buf), f) != NULL) { buf[strcspn(buf, "\r\n")] = '\0'; if (n >= alloc) { alloc += 16; - w = erealloc (w, alloc * sizeof(char *)); + w = erealloc(w, alloc * sizeof(char *)); } - w[n++] = estrdup (buf); + w[n++] = estrdup(buf); } *ret_w = w; if (n == 0) @@ -63,30 +63,30 @@ read_words (const char *filename, char ***ret_w) } static int -nop_prompter (krb5_context context, - void *data, - const char *name, - const char *banner, - int num_prompts, - krb5_prompt prompts[]) +nop_prompter(krb5_context context, + void *data, + const char *name, + const char *banner, + int num_prompts, + krb5_prompt prompts[]) { return 0; } static void -generate_requests (const char *filename, unsigned nreq) +generate_requests(const char *filename, unsigned nreq) { krb5_context context; krb5_error_code ret; int i; char **words; - unsigned nwords; + unsigned nwords, k; - ret = krb5_init_context (&context); + ret = krb5_init_context(&context); if (ret) errx (1, "krb5_init_context failed: %d", ret); - nwords = read_words (filename, &words); + nwords = read_words(filename, &words); for (i = 0; i < nreq; ++i) { char *name = words[rand() % nwords]; @@ -98,32 +98,32 @@ generate_requests (const char *filename, unsigned nreq) char *old_pwd, *new_pwd; int aret; - krb5_get_init_creds_opt_alloc (context, &opt); + krb5_get_init_creds_opt_alloc(context, &opt); krb5_get_init_creds_opt_set_tkt_life (opt, 300); krb5_get_init_creds_opt_set_forwardable (opt, FALSE); krb5_get_init_creds_opt_set_proxiable (opt, FALSE); - ret = krb5_parse_name (context, name, &principal); + ret = krb5_parse_name(context, name, &principal); if (ret) - krb5_err (context, 1, ret, "krb5_parse_name %s", name); + krb5_err(context, 1, ret, "krb5_parse_name %s", name); - aret = asprintf (&old_pwd, "%s", name); + aret = asprintf(&old_pwd, "%s", name); if (aret == -1) krb5_errx(context, 1, "out of memory"); - aret = asprintf (&new_pwd, "%s2", name); + aret = asprintf(&new_pwd, "%s2", name); if (aret == -1) krb5_errx(context, 1, "out of memory"); - ret = krb5_get_init_creds_password (context, - &cred, - principal, - old_pwd, - nop_prompter, - NULL, - 0, - "kadmin/changepw", - opt); - if( ret == KRB5KRB_AP_ERR_BAD_INTEGRITY + ret = krb5_get_init_creds_password(context, + &cred, + principal, + old_pwd, + nop_prompter, + NULL, + 0, + "kadmin/changepw", + opt); + if (ret == KRB5KRB_AP_ERR_BAD_INTEGRITY || ret == KRB5KRB_AP_ERR_MODIFIED) { char *tmp; @@ -131,37 +131,41 @@ generate_requests (const char *filename, unsigned nreq) new_pwd = old_pwd; old_pwd = tmp; - ret = krb5_get_init_creds_password (context, - &cred, - principal, - old_pwd, - nop_prompter, - NULL, - 0, - "kadmin/changepw", - opt); + ret = krb5_get_init_creds_password(context, + &cred, + principal, + old_pwd, + nop_prompter, + NULL, + 0, + "kadmin/changepw", + opt); } if (ret) - krb5_err (context, 1, ret, "krb5_get_init_creds_password"); + krb5_err(context, 1, ret, "krb5_get_init_creds_password"); - krb5_free_principal (context, principal); + krb5_free_principal(context, principal); - ret = krb5_set_password (context, - &cred, - new_pwd, - NULL, - &result_code, - &result_code_string, - &result_string); + ret = krb5_set_password(context, + &cred, + new_pwd, + NULL, + &result_code, + &result_code_string, + &result_string); if (ret) - krb5_err (context, 1, ret, "krb5_change_password"); + krb5_err(context, 1, ret, "krb5_change_password"); - free (old_pwd); - free (new_pwd); - krb5_free_cred_contents (context, &cred); + free(old_pwd); + free(new_pwd); + krb5_free_cred_contents(context, &cred); krb5_get_init_creds_opt_free(context, opt); } + + for (k = 0; k < nwords; k++) + free(words[k]); + free(words); } static int version_flag = 0; @@ -173,12 +177,12 @@ static struct getargs args[] = { }; static void -usage (int ret) +usage(int ret) { - arg_printusage (args, - sizeof(args)/sizeof(*args), - NULL, - "file [number]"); + arg_printusage(args, + sizeof(args)/sizeof(*args), + NULL, + "file [number]"); exit (ret); } @@ -204,9 +208,9 @@ main(int argc, char **argv) if (argc != 2) usage (1); srand (0); - nreq = strtol (argv[1], &end, 0); + nreq = strtol(argv[1], &end, 0); if (argv[1] == end || *end != '\0') usage (1); - generate_requests (argv[0], nreq); + generate_requests(argv[0], nreq); return 0; } diff --git a/lib/hx509/ks_file.c b/lib/hx509/ks_file.c index a487da393..642dd173b 100644 --- a/lib/hx509/ks_file.c +++ b/lib/hx509/ks_file.c @@ -96,9 +96,10 @@ try_decrypt(hx509_context context, password, passwordlen, 1, key, NULL); if (ret <= 0) { - hx509_set_error_string(context, 0, HX509_CRYPTO_INTERNAL_ERROR, + ret = HX509_CRYPTO_INTERNAL_ERROR; + hx509_set_error_string(context, 0, ret, "Failed to do string2key for private key"); - return HX509_CRYPTO_INTERNAL_ERROR; + goto out; } clear.data = malloc(len); diff --git a/lib/hx509/softp11.c b/lib/hx509/softp11.c index e90d9ba4c..839e95647 100644 --- a/lib/hx509/softp11.c +++ b/lib/hx509/softp11.c @@ -543,6 +543,8 @@ add_cert(hx509_context hxctx, void *ctx, hx509_cert cert) CK_FLAGS flags; type = CKO_PRIVATE_KEY; + + /* Note to static analyzers: `o' is still referred to via globals */ o = add_st_object(); if (o == NULL) { ret = CKR_DEVICE_MEMORY; @@ -593,6 +595,7 @@ add_cert(hx509_context hxctx, void *ctx, hx509_cert cert) hx509_xfree(issuer_data.data); hx509_xfree(subject_data.data); + /* Note to static analyzers: `o' is still referred to via globals */ return 0; } diff --git a/lib/kadm5/init_c.c b/lib/kadm5/init_c.c index fa4bc4b9a..6eddfb871 100644 --- a/lib/kadm5/init_c.c +++ b/lib/kadm5/init_c.c @@ -588,16 +588,18 @@ kadm5_c_init_with_context(krb5_context context, krb5_ccache cc; ret = _kadm5_c_init_context(&ctx, realm_params, context); - if(ret) + if (ret) return ret; - if(password != NULL && *password != '\0') { + if (password != NULL && *password != '\0') { ret = _kadm5_c_get_cred_cache(context, client_name, service_name, password, prompter, keytab, ccache, &cc); - if(ret) - return ret; /* XXX */ + if (ret) { + kadm5_c_destroy(ctx); + return ret; + } ccache = cc; } diff --git a/lib/krb5/auth_context.c b/lib/krb5/auth_context.c index 7a20eb51e..9c6c0c40f 100644 --- a/lib/krb5/auth_context.c +++ b/lib/krb5/auth_context.c @@ -292,9 +292,9 @@ copy_key(krb5_context context, krb5_keyblock *in, krb5_keyblock **out) { - if(in) + *out = NULL; + if (in) return krb5_copy_keyblock(context, in, out); - *out = NULL; /* is this right? */ return 0; } diff --git a/lib/krb5/krbhst.c b/lib/krb5/krbhst.c index 51cd3dcc7..f53512883 100644 --- a/lib/krb5/krbhst.c +++ b/lib/krb5/krbhst.c @@ -592,8 +592,10 @@ add_plugin_host(struct krb5_krbhst_data *kd, hostlen = strlen(host); hi = calloc(1, sizeof(*hi) + hostlen); - if(hi == NULL) + if (hi == NULL) { + freeaddrinfo(ai); return ENOMEM; + } hi->proto = proto; hi->port = hi->def_port = portnum; diff --git a/lib/krb5/pkinit.c b/lib/krb5/pkinit.c index 8a3eb7b65..9653734d2 100644 --- a/lib/krb5/pkinit.c +++ b/lib/krb5/pkinit.c @@ -1133,8 +1133,10 @@ pk_rd_pa_reply_enckey(krb5_context context, ret = der_put_length_and_tag (ptr + ph - 1, ph, content.length, ASN1_C_UNIV, CONS, UT_Sequence, &l); - if (ret) + if (ret) { + free(ptr); return ret; + } free(content.data); content.data = ptr; content.length += ph; diff --git a/lib/krb5/rd_req.c b/lib/krb5/rd_req.c index b16c88265..fbced144e 100644 --- a/lib/krb5/rd_req.c +++ b/lib/krb5/rd_req.c @@ -243,33 +243,26 @@ krb5_verify_authenticator_checksum(krb5_context context, size_t len) { krb5_error_code ret; - krb5_keyblock *key; + krb5_keyblock *key = NULL; krb5_authenticator authenticator; krb5_crypto crypto; - ret = krb5_auth_con_getauthenticator (context, - ac, - &authenticator); - if(ret) + ret = krb5_auth_con_getauthenticator(context, ac, &authenticator); + if (ret) return ret; - if(authenticator->cksum == NULL) { - krb5_free_authenticator(context, &authenticator); - return -17; + if (authenticator->cksum == NULL) { + ret = -17; + goto out; } ret = krb5_auth_con_getkey(context, ac, &key); - if(ret) { - krb5_free_authenticator(context, &authenticator); - return ret; - } + if (ret) + goto out; ret = krb5_crypto_init(context, key, 0, &crypto); - if(ret) + if (ret) goto out; - ret = krb5_verify_checksum (context, - crypto, - KRB5_KU_AP_REQ_AUTH_CKSUM, - data, - len, - authenticator->cksum); + ret = krb5_verify_checksum(context, crypto, + KRB5_KU_AP_REQ_AUTH_CKSUM, + data, len, authenticator->cksum); krb5_crypto_destroy(context, crypto); out: krb5_free_authenticator(context, &authenticator); diff --git a/lib/krb5/send_to_kdc.c b/lib/krb5/send_to_kdc.c index 1b0a53355..f4ffd4034 100644 --- a/lib/krb5/send_to_kdc.c +++ b/lib/krb5/send_to_kdc.c @@ -851,6 +851,8 @@ submit_request(krb5_context context, krb5_sendto_ctx ctx, krb5_krbhst_info *hi) host = heim_alloc(sizeof(*host), "sendto-host", deallocate_host); if (host == NULL) { + if (freeai) + freeaddrinfo(ai); rk_closesocket(fd); return ENOMEM; } diff --git a/lib/krb5/verify_krb5_conf.c b/lib/krb5/verify_krb5_conf.c index 420c8b324..0db8807a4 100644 --- a/lib/krb5/verify_krb5_conf.c +++ b/lib/krb5/verify_krb5_conf.c @@ -163,22 +163,22 @@ check_host(krb5_context context, const char *path, char *data) /* XXX data could be a list of hosts that this code can't handle */ /* XXX copied from krbhst.c */ - if(strncmp(p, "http://", 7) == 0){ + if (strncmp(p, "http://", 7) == 0){ p += 7; hints.ai_socktype = SOCK_STREAM; strlcpy(service, "http", sizeof(service)); defport = 80; - } else if(strncmp(p, "http/", 5) == 0) { + } else if (strncmp(p, "http/", 5) == 0) { p += 5; hints.ai_socktype = SOCK_STREAM; strlcpy(service, "http", sizeof(service)); defport = 80; - }else if(strncmp(p, "tcp/", 4) == 0){ + } else if (strncmp(p, "tcp/", 4) == 0){ p += 4; hints.ai_socktype = SOCK_STREAM; strlcpy(service, "kerberos", sizeof(service)); defport = 88; - } else if(strncmp(p, "udp/", 4) == 0) { + } else if (strncmp(p, "udp/", 4) == 0) { p += 4; hints.ai_socktype = SOCK_DGRAM; strlcpy(service, "kerberos", sizeof(service)); @@ -188,14 +188,14 @@ check_host(krb5_context context, const char *path, char *data) strlcpy(service, "kerberos", sizeof(service)); defport = 88; } - if(strsep_copy(&p, ":", hostname, sizeof(hostname)) < 0) { + if (strsep_copy(&p, ":", hostname, sizeof(hostname)) < 0) { return 1; } hostname[strcspn(hostname, "/")] = '\0'; - if(p != NULL) { + if (p != NULL) { char *end; int tmp = strtol(p, &end, 0); - if(end == p) { + if (end == p) { krb5_warnx(context, "%s: failed to parse port number in %s", path, data); return 1; @@ -204,14 +204,15 @@ check_host(krb5_context context, const char *path, char *data) snprintf(service, sizeof(service), "%u", defport); } ret = getaddrinfo(hostname, service, &hints, &ai); - if(ret == EAI_SERVICE && !isdigit((unsigned char)service[0])) { + if (ret == EAI_SERVICE && !isdigit((unsigned char)service[0])) { snprintf(service, sizeof(service), "%u", defport); ret = getaddrinfo(hostname, service, &hints, &ai); } - if(ret != 0) { + if (ret != 0) { krb5_warnx(context, "%s: %s (%s)", path, gai_strerror(ret), hostname); return 1; } + freeaddrinfo(ai); return 0; } -- 2.11.4.GIT