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(a)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(a)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" );