ldap/servers/plugins/posix-winsync/posix-group-func.c | 14 ++++++++++---- ldap/servers/plugins/posix-winsync/posix-winsync-config.c | 4 +++- ldap/servers/plugins/posix-winsync/posix-winsync.c | 9 +++++---- ldap/servers/plugins/replication/windows_private.c | 10 +++++----- 4 files changed, 23 insertions(+), 14 deletions(-)
New commits: commit 0c24dfcff939c6c30d336cc498f074d743daf302 Author: Rich Megginson rmeggins@redhat.com Date: Mon Aug 27 14:23:22 2012 -0600
coverity - posix winsync mem leaks, null check, deadcode, null ref, use after free
Fixes the following issues: 13068 Resource leak In posix_winsync_pre_ds_mod_group_cb(): Leak of memory or pointers to system resources.
13067 Resource leak In posix_winsync_end_update_cb(): Leak of memory or pointers to system resources
13066 Resource leak In modGroupMembership(): Leak of memory or pointers to system resources
13065 Resource leak In dn_in_set(): Leak of memory or pointers to system resources
13064 Resource leak In dn_in_set(): Leak of memory or pointers to system resources
13063 Dereference after null check In windows_plugin_add(): Pointer is checked against null but then dereferenced anyway
13062 Explicit null dereferenced In searchUid(): Dereference of an explicit null value
13061 Logically dead code In check_account_lock(): Code can never be reached because of a logical contradiction
13059 Use after free In windows_plugin_cleanup_agmt(): A pointer to freed memory is dereferenced, used as a function argument, or otherwise used Reviewed by: nkinder (Thanks!) (cherry picked from commit 1a9b5ffe655c7ec8dd935106376ad8dabe69b561)
diff --git a/ldap/servers/plugins/posix-winsync/posix-group-func.c b/ldap/servers/plugins/posix-winsync/posix-group-func.c index 72963ac..d4bbcfe 100644 --- a/ldap/servers/plugins/posix-winsync/posix-group-func.c +++ b/ldap/servers/plugins/posix-winsync/posix-group-func.c @@ -86,7 +86,7 @@ searchUid(const char *udn) } slapi_free_search_results_internal(int_search_pb); slapi_pblock_destroy(int_search_pb); - if (posix_winsync_config_get_lowercase()) { + if (uid && posix_winsync_config_get_lowercase()) { return slapi_dn_ignore_case(uid); } return uid; @@ -103,11 +103,15 @@ int dn_in_set(const char* uid, char **uids) { int i; - Slapi_DN *sdn_uid = slapi_sdn_new_dn_byval(uid); - Slapi_DN *sdn_ul = slapi_sdn_new(); + Slapi_DN *sdn_uid = NULL; + Slapi_DN *sdn_ul = NULL;
if (uids == NULL || uid == NULL) return false; + + sdn_uid = slapi_sdn_new_dn_byval(uid); + sdn_ul = slapi_sdn_new(); + for (i = 0; uids[i]; i++) { slapi_sdn_set_dn_byref(sdn_ul, uids[i]); if (slapi_sdn_compare(sdn_uid, sdn_ul) == 0) { @@ -346,7 +350,9 @@ modGroupMembership(Slapi_Entry *entry, Slapi_Mods *smods, int *do_modify) "modGroupMembership: add to modlist %s\n", uid); doModify = true; } - /* slapi_value_free(&v); */ + slapi_valueset_free(vs); + slapi_value_init_berval(v, NULL); /* otherwise we will try to free memory we do not own */ + slapi_value_free(&v); } } } diff --git a/ldap/servers/plugins/posix-winsync/posix-winsync.c b/ldap/servers/plugins/posix-winsync/posix-winsync.c index 6eddc23..dc56475 100644 --- a/ldap/servers/plugins/posix-winsync/posix-winsync.c +++ b/ldap/servers/plugins/posix-winsync/posix-winsync.c @@ -184,8 +184,8 @@ check_account_lock(Slapi_Entry *ds_entry, int *isvirt) rc = 1; /* no attr == entry is enabled */ slapi_log_error(SLAPI_LOG_PLUGIN, posix_winsync_plugin_name, "<-- check_account_lock - entry [%s] does not " - "have attribute nsAccountLock - entry %s locked\n", - slapi_entry_get_dn_const(ds_entry), rc ? "is not" : "is"); + "have attribute nsAccountLock - entry is not locked\n", + slapi_entry_get_dn_const(ds_entry)); }
return rc; @@ -918,7 +918,7 @@ posix_winsync_pre_ds_mod_group_cb(void *cbdata, const Slapi_Entry *rawentry, Sla slapi_value_init_string(voc, "posixGroup"); slapi_entry_attr_find(ds_entry, "objectClass", &oc_attr); if (slapi_attr_value_find(oc_attr, slapi_value_get_berval(voc)) != 0) { - Slapi_ValueSet *oc_vs = slapi_valueset_new(); + Slapi_ValueSet *oc_vs = NULL; Slapi_Value *oc_nv = slapi_value_new();
slapi_attr_get_valueset(oc_attr, &oc_vs); @@ -933,7 +933,7 @@ posix_winsync_pre_ds_mod_group_cb(void *cbdata, const Slapi_Entry *rawentry, Sla slapi_value_free(&oc_nv); slapi_log_error(SLAPI_LOG_PLUGIN, posix_winsync_plugin_name, "_pre_ds_mod_group_cb step\n"); - /* slapi_valuset_free(oc_vs); */ + slapi_valueset_free(oc_vs); slapi_log_error(SLAPI_LOG_PLUGIN, posix_winsync_plugin_name, "_pre_ds_mod_group_cb step\n"); } @@ -1292,6 +1292,7 @@ posix_winsync_end_update_cb(void *cbdata, const Slapi_DN *ds_subtree, const Slap slapi_log_error(SLAPI_LOG_PLUGIN, posix_winsync_plugin_name, "--> posix_winsync_end_update_cb, create task %s\n", dn); if (NULL == dn) { + slapi_pblock_destroy(pb); slapi_log_error(SLAPI_LOG_FATAL, posix_winsync_plugin_name, "posix_winsync_end_update_cb: " "failed to create task dn: cn=%s,%s,cn=tasks,cn=config\n", diff --git a/ldap/servers/plugins/replication/windows_private.c b/ldap/servers/plugins/replication/windows_private.c index 058de6e..3ee6a71 100644 --- a/ldap/servers/plugins/replication/windows_private.c +++ b/ldap/servers/plugins/replication/windows_private.c @@ -1246,7 +1246,7 @@ windows_plugin_add(void **theapi, int maxapi) } elem = PR_NEXT_LINK(elem); } - if (wpi) { /* was not added - precedence too high */ + if (elem && wpi) { /* was not added - precedence too high */ /* just add to end of list */ PR_INSERT_BEFORE(wpi, elem); wpi = NULL; /* owned by list now */ @@ -1372,10 +1372,8 @@ windows_plugin_cleanup_agmt(Repl_Agmt *ra)
while (list && !PR_CLIST_IS_EMPTY(list)) { elem = PR_LIST_HEAD(list); - if (elem != list) { - PR_REMOVE_LINK(elem); - slapi_ch_free((void **)&elem); - } + PR_REMOVE_LINK(elem); + slapi_ch_free((void **)&elem); } slapi_ch_free((void **)&list); windows_private_set_api_cookie(ra, NULL);
commit ae6a33f6a27689465365c7bc2c17225ceeba5141 Author: Rich Megginson rmeggins@redhat.com Date: Mon Aug 27 12:31:30 2012 -0600
fix mem leaks with parent dn log message, setting winsync windows domain Reviewed by: nkinder (Thanks!) (cherry picked from commit f59ea3c6cfe5bcd1e8604c0215df651d491587b9)
diff --git a/ldap/servers/plugins/posix-winsync/posix-winsync-config.c b/ldap/servers/plugins/posix-winsync/posix-winsync-config.c index 8239456..a2d21be 100644 --- a/ldap/servers/plugins/posix-winsync/posix-winsync-config.c +++ b/ldap/servers/plugins/posix-winsync/posix-winsync-config.c @@ -81,8 +81,10 @@ posix_winsync_agmt_init(const Slapi_DN *ds_subtree, const Slapi_DN *ad_subtree) sdn = slapi_get_next_suffix(&node, 0); } if (!sdn) { + char *pardn = slapi_dn_parent(slapi_sdn_get_dn(ds_subtree)); slapi_log_error(SLAPI_LOG_FATAL, POSIX_WINSYNC_PLUGIN_NAME, "suffix not found for '%s'\n", - slapi_dn_parent(slapi_sdn_get_dn(ds_subtree))); + pardn); + slapi_ch_free_string(&pardn); }
slapi_log_error(SLAPI_LOG_PLUGIN, POSIX_WINSYNC_PLUGIN_NAME, diff --git a/ldap/servers/plugins/replication/windows_private.c b/ldap/servers/plugins/replication/windows_private.c index 26d03c2..058de6e 100644 --- a/ldap/servers/plugins/replication/windows_private.c +++ b/ldap/servers/plugins/replication/windows_private.c @@ -368,6 +368,7 @@ void windows_agreement_delete(Repl_Agmt *ra) slapi_filter_free(dp->directory_filter, 1); slapi_filter_free(dp->deleted_filter, 1); slapi_entry_free(dp->raw_entry); + slapi_ch_free_string(&dp->windows_domain); dp->raw_entry = NULL; dp->api_cookie = NULL; slapi_ch_free((void **)dp); @@ -534,6 +535,7 @@ windows_private_set_windows_domain(const Repl_Agmt *ra, char *domain) dp = (Dirsync_Private *) agmt_get_priv(ra); PR_ASSERT (dp);
+ slapi_ch_free_string(&dp->windows_domain); dp->windows_domain = domain; LDAPDebug0Args( LDAP_DEBUG_TRACE, "<= windows_private_set_windows_domain\n" );
389-commits@lists.fedoraproject.org
-
Richard Allen Megginson