[Freeipa-devel] [PATCHES] #2037 Coverity issues
Alexander Bokovoy
abokovoy at redhat.com
Mon Nov 7 22:22:01 UTC 2011
On Thu, 03 Nov 2011, Simo Sorce wrote:
> These are actual issues. Most are resource leaks.
> And one is a bad sizeof() computation that will cause us later to
> overwrite out of bound memory, so potentially a segfault.
Went through all of these. ACK.
> https://fedorahosted.org/freeipa/ticket/2037
> ---
> daemons/ipa-kdb/ipa_kdb.c | 13 +++++++------
> 1 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/daemons/ipa-kdb/ipa_kdb.c b/daemons/ipa-kdb/ipa_kdb.c
> index 6a6c2063902f8b2a76d97f3510f09333c5af168d..481b1f392766498c5d7c6333fe73bafefde87dae 100644
> --- a/daemons/ipa-kdb/ipa_kdb.c
> +++ b/daemons/ipa-kdb/ipa_kdb.c
> @@ -263,6 +263,13 @@ int ipadb_get_connection(struct ipadb_context *ipactx)
>
> done:
> ldap_msgfree(res);
> +
> + ldap_value_free_len(vals);
> + for (i = 0; i < c && cvals[i]; i++) {
> + free(cvals[i]);
> + }
> + free(cvals);
> +
> if (ret) {
> if (ipactx->lcontext) {
> ldap_unbind_ext_s(ipactx->lcontext, NULL, NULL);
> @@ -274,12 +281,6 @@ done:
> return EIO;
> }
>
> - ldap_value_free_len(vals);
> - for (i = 0; i < c; i++) {
> - free(cvals[i]);
> - }
> - free(cvals);
> -
> return 0;
> }
ACK.
> https://fedorahosted.org/freeipa/ticket/2037
> ---
> daemons/ipa-kdb/ipa_kdb_passwords.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/daemons/ipa-kdb/ipa_kdb_passwords.c b/daemons/ipa-kdb/ipa_kdb_passwords.c
> index 93e9e206081af412a472ab0c7624611a628a15b7..0bb7fa72496789241e27ecc852a1c6ede7f8e40a 100644
> --- a/daemons/ipa-kdb/ipa_kdb_passwords.c
> +++ b/daemons/ipa-kdb/ipa_kdb_passwords.c
> @@ -203,6 +203,7 @@ krb5_error_code ipadb_change_pwd(krb5_context context,
> ret = asprintf(&ied->pw_policy_dn,
> "cn=global_policy,%s", ipactx->realm_base);
> if (ret == -1) {
> + free(ied);
> return ENOMEM;
> }
> db_entry->e_data = (krb5_octet *)ied;
ACK.
> https://fedorahosted.org/freeipa/ticket/2037
> ---
> util/ipa_pwd.c | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/util/ipa_pwd.c b/util/ipa_pwd.c
> index c41617533ec25e8e656e9bb7a69d5b8b5dd8b5f7..fda6cb34ef24059362207325db61aedb62d7b665 100644
> --- a/util/ipa_pwd.c
> +++ b/util/ipa_pwd.c
> @@ -560,7 +560,7 @@ int ipapwd_generate_new_history(char *password,
> unsigned char *hash = NULL;
> unsigned int hash_len;
> char *new_element;
> - char **ordered;
> + char **ordered = NULL;
> int c, i, n;
> int len;
> int ret;
> @@ -626,9 +626,11 @@ int ipapwd_generate_new_history(char *password,
>
> *new_pwd_history = ordered;
> *new_pwd_hlen = n;
> + ordered = NULL;
> ret = IPAPWD_POLICY_OK;
>
> done:
> + free(ordered);
> free(hash);
> return ret;
> }
ACK
> https://fedorahosted.org/freeipa/ticket/2037
> ---
> daemons/ipa-kdb/ipa_kdb_principals.c | 7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c
> index fdd834f355fd9e056058fa205b217e9e1f142e51..117eea86952ee4662930b80ba5e54c75aa587faf 100644
> --- a/daemons/ipa-kdb/ipa_kdb_principals.c
> +++ b/daemons/ipa-kdb/ipa_kdb_principals.c
> @@ -1571,6 +1571,7 @@ static krb5_error_code ipadb_entry_to_mods(krb5_context kcontext,
> char **new_history;
> int nh_len;
> int ret;
> + int i;
>
> ied = (struct ipadb_e_data *)entry->e_data;
> if (ied->magic != IPA_E_DATA_MAGIC) {
> @@ -1619,6 +1620,12 @@ static krb5_error_code ipadb_entry_to_mods(krb5_context kcontext,
>
> kerr = ipadb_get_ldap_mod_str_list(imods, "passwordHistory",
> new_history, nh_len, mod_op);
> +
> + for (i = 0; i < nh_len; i++) {
> + free(new_history[i]);
> + }
> + free(new_history);
> +
> if (kerr) {
> goto done;
> }
ACK.
> https://fedorahosted.org/freeipa/ticket/2037
> ---
> daemons/ipa-kdb/ipa_kdb_principals.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c
> index 117eea86952ee4662930b80ba5e54c75aa587faf..bb1356a01a27a639c15439187ffb8d3537c1fcec 100644
> --- a/daemons/ipa-kdb/ipa_kdb_principals.c
> +++ b/daemons/ipa-kdb/ipa_kdb_principals.c
> @@ -334,6 +334,7 @@ done:
> free(keys[i].key_data_contents[0]);
> free(keys[i].key_data_contents[1]);
> }
> + free(keys);
> *result = NULL;
> *num = 0;
> }
ACK.
> https://fedorahosted.org/freeipa/ticket/2037
> ---
> daemons/ipa-kdb/ipa_kdb_principals.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c
> index bb1356a01a27a639c15439187ffb8d3537c1fcec..818ef033f5a085c0f60e479021f0964d81487704 100644
> --- a/daemons/ipa-kdb/ipa_kdb_principals.c
> +++ b/daemons/ipa-kdb/ipa_kdb_principals.c
> @@ -128,6 +128,7 @@ static int ipadb_ldap_attr_to_tl_data(LDAP *lcontext, LDAPMessage *le,
>
> done:
> if (ret) {
> + free(next);
> if (*result) {
> prev = *result;
> while (prev) {
ACK.
> https://fedorahosted.org/freeipa/ticket/2037
> ---
> daemons/ipa-kdb/ipa_kdb_principals.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c
> index 818ef033f5a085c0f60e479021f0964d81487704..33ed7b0e15ff1fa29150d1c2695c5a3b0c4c5f03 100644
> --- a/daemons/ipa-kdb/ipa_kdb_principals.c
> +++ b/daemons/ipa-kdb/ipa_kdb_principals.c
> @@ -554,6 +554,8 @@ static krb5_error_code ipadb_parse_ldap_entry(krb5_context kcontext,
> }
> ied->magic = IPA_E_DATA_MAGIC;
>
> + entry->e_data = (krb5_octet *)ied;
> +
> /* mark this as an ipa_user if it has the posixaccount objectclass */
> ret = ipadb_ldap_attr_has_value(lcontext, lentry,
> "objectClass", "posixAccount");
> @@ -610,8 +612,6 @@ static krb5_error_code ipadb_parse_ldap_entry(krb5_context kcontext,
> ied->last_pwd_change = restime;
> }
>
> - entry->e_data = (krb5_octet *)ied;
> -
> kerr = 0;
>
> done:
ACK.
> https://fedorahosted.org/freeipa/ticket/2037
> ---
> daemons/ipa-kdb/ipa_kdb_pwdpolicy.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/daemons/ipa-kdb/ipa_kdb_pwdpolicy.c b/daemons/ipa-kdb/ipa_kdb_pwdpolicy.c
> index 3dc4c218891e3ab2735eac1dcc721173657827f7..d439feb907eebda70b513ac9ca70f3e259ad3909 100644
> --- a/daemons/ipa-kdb/ipa_kdb_pwdpolicy.c
> +++ b/daemons/ipa-kdb/ipa_kdb_pwdpolicy.c
> @@ -52,7 +52,7 @@ krb5_error_code ipadb_get_pwd_policy(krb5_context kcontext, char *name,
> krb5_error_code kerr;
> LDAPMessage *res = NULL;
> LDAPMessage *lentry;
> - osa_policy_ent_t pentry;
> + osa_policy_ent_t pentry = NULL;
> uint32_t result;
> int ret;
>
> @@ -150,6 +150,9 @@ krb5_error_code ipadb_get_pwd_policy(krb5_context kcontext, char *name,
> *policy = pentry;
>
> done:
> + if (kerr) {
> + free(pentry);
> + }
> free(esc_name);
> free(src_filter);
> ldap_msgfree(res);
ACK.
> https://fedorahosted.org/freeipa/ticket/2037
> ---
> daemons/ipa-kdb/ipa_kdb_pwdpolicy.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/daemons/ipa-kdb/ipa_kdb_pwdpolicy.c b/daemons/ipa-kdb/ipa_kdb_pwdpolicy.c
> index d439feb907eebda70b513ac9ca70f3e259ad3909..46a0513307c859ff2cfef7ad58442edb1b9cc78d 100644
> --- a/daemons/ipa-kdb/ipa_kdb_pwdpolicy.c
> +++ b/daemons/ipa-kdb/ipa_kdb_pwdpolicy.c
> @@ -85,7 +85,7 @@ krb5_error_code ipadb_get_pwd_policy(krb5_context kcontext, char *name,
> goto done;
> }
>
> - pentry = calloc(1, sizeof(osa_policy_ent_t));
> + pentry = calloc(1, sizeof(osa_policy_ent_rec));
> if (!pentry) {
> kerr = ENOMEM;
> goto done;
How this one has even worked? :)
ACK.
--
/ Alexander Bokovoy
More information about the Freeipa-devel
mailing list