From d8822be083b6ffd4c21004ede8cd14a8246c079c Mon Sep 17 00:00:00 2001
From: Rich Megginson <rmeggins@xxxxxxxxxx>
Date: Tue, 21 Aug 2012 10:00:21 -0600
Subject: [PATCH] coverity - mbo dead code - winsync leaks, deadcode, null check, test code
This fixes the following issues:
13060 Logically dead code
In memberof_test_membership_callback(): 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
13058 Dereference before null check
In windows_generate_update_mods(): All paths that lead to this null pointer comparison already dereference the pointer earlier
13057 Dereference before null check
In windows_generate_update_mods(): All paths that lead to this null pointer comparison already dereference the pointer earlier
13056 Resource leak
In windows_plugin_add(): Leak of memory or pointers to system resources
13055 Dereference after null check
In windows_generate_update_mods(): Pointer is checked against null but then dereferenced anyway
13054 Dereference after null check
In test_winsync_pre_ds_search_all_cb(): Pointer is checked against null but then dereferenced anyway
I've also commented out any code from the test winsync plugin except for
logging the entrance and exit of each function.
---
ldap/servers/plugins/memberof/memberof.c | 4 -
ldap/servers/plugins/replication/windows_private.c | 79 ++++++++++++++-----
.../plugins/replication/windows_protocol_util.c | 27 +++++--
3 files changed, 77 insertions(+), 33 deletions(-)
diff --git a/ldap/servers/plugins/memberof/memberof.c b/ldap/servers/plugins/memberof/memberof.c
index 2ca4d2f..6943d91 100644
--- a/ldap/servers/plugins/memberof/memberof.c
+++ b/ldap/servers/plugins/memberof/memberof.c
@@ -1900,10 +1900,6 @@ int memberof_test_membership_callback(Slapi_Entry *e, void *callback_data)
goto bail;
}
slapi_value_set_flags(entry_dn, SLAPI_ATTR_FLAG_NORMALIZED_CIS);
- if(0 == entry_dn)
- {
- goto bail;
- }
/* divide groups into member and non-member lists */
slapi_entry_attr_find(e, config->memberof_attr, &attr );
diff --git a/ldap/servers/plugins/replication/windows_private.c b/ldap/servers/plugins/replication/windows_private.c
index 20accfd..0935c02 100644
--- a/ldap/servers/plugins/replication/windows_private.c
+++ b/ldap/servers/plugins/replication/windows_private.c
@@ -1108,13 +1108,15 @@ windows_plugin_add(void **theapi, int maxapi)
while (elem && (elem != &winsync_plugin_list)) {
if (precedence < elem->precedence) {
PR_INSERT_BEFORE(wpi, elem);
+ wpi = NULL; /* owned by list now */
break;
}
elem = PR_NEXT_LINK(elem);
}
- if (elem == &winsync_plugin_list) {
+ if (wpi) { /* was not added - precedence too high */
/* just add to end of list */
PR_INSERT_BEFORE(wpi, elem);
+ wpi = NULL; /* owned by list now */
}
return 0;
}
@@ -1237,8 +1239,10 @@ windows_plugin_cleanup_agmt(Repl_Agmt *ra)
while (list && !PR_CLIST_IS_EMPTY(list)) {
elem = PR_LIST_HEAD(list);
- PR_REMOVE_LINK(elem);
- slapi_ch_free((void **)&elem);
+ if (elem != list) {
+ PR_REMOVE_LINK(elem);
+ slapi_ch_free((void **)&elem);
+ }
}
slapi_ch_free((void **)&list);
windows_private_set_api_cookie(ra, NULL);
@@ -1686,12 +1690,19 @@ test_winsync_pre_ds_search_all_cb(void *cbdata, const char *agmt_dn,
"--> test_winsync_pre_ds_search_all_cb -- orig filter [%s] -- begin\n",
((filter && *filter) ? *filter : "NULL"));
- /* We only want to grab users from the ds side - no groups */
- slapi_ch_free_string(filter);
- /* maybe use ntUniqueId=* - only get users that have already been
- synced with AD already - ntUniqueId and ntUserDomainId are
- indexed for equality only - need to add presence? */
- *filter = slapi_ch_strdup("(&(objectclass=ntuser)(ntUserDomainId=*))");
+#ifdef THIS_IS_JUST_AN_EXAMPLE
+ if (filter) {
+ /* We only want to grab users from the ds side - no groups */
+ slapi_ch_free_string(filter);
+ /* maybe use ntUniqueId=* - only get users that have already been
+ synced with AD already - ntUniqueId and ntUserDomainId are
+ indexed for equality only - need to add presence? */
+ *filter = slapi_ch_strdup("(&(objectclass=ntuser)(ntUserDomainId=*))");
+ slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
+ "--> test_winsync_pre_ds_search_all_cb -- new filter [%s]\n",
+ *filter ? *filter : "NULL"));
+ }
+#endif
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"<-- test_winsync_pre_ds_search_all_cb -- end\n");
@@ -1792,6 +1803,7 @@ test_winsync_get_new_ds_user_dn_cb(void *cbdata, const Slapi_Entry *rawentry,
"--> test_winsync_get_new_ds_user_dn_cb -- old dn [%s] -- begin\n",
*new_dn_string);
+#ifdef THIS_IS_JUST_AN_EXAMPLE
rdns = slapi_ldap_explode_dn(*new_dn_string, 0);
if (!rdns || !rdns[0]) {
slapi_ldap_value_free(rdns);
@@ -1801,6 +1813,7 @@ test_winsync_get_new_ds_user_dn_cb(void *cbdata, const Slapi_Entry *rawentry,
slapi_ch_free_string(new_dn_string);
*new_dn_string = PR_smprintf("%s,%s", rdns[0], slapi_sdn_get_dn(ds_suffix));
slapi_ldap_value_free(rdns);
+#endif
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"<-- test_winsync_get_new_ds_user_dn_cb -- new dn [%s] -- end\n",
@@ -1914,10 +1927,12 @@ test_winsync_post_ad_mod_user_cb(void *cookie, const Slapi_Entry *rawentry, Slap
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"--> test_winsync_post_ad_mod_user_cb -- begin\n");
+#ifdef THIS_IS_JUST_AN_EXAMPLE
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"Result of modifying AD entry [%s] was [%d:%s]\n",
slapi_entry_get_dn(ad_entry), *result, ldap_err2string(*result));
-
+#endif
+
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"<-- test_winsync_post_ad_mod_user_cb -- end\n");
@@ -1930,10 +1945,12 @@ test_winsync_post_ad_mod_group_cb(void *cookie, const Slapi_Entry *rawentry, Sla
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"--> test_winsync_post_ad_mod_group_cb -- begin\n");
+#ifdef THIS_IS_JUST_AN_EXAMPLE
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"Result of modifying AD entry [%s] was [%d:%s]\n",
slapi_entry_get_dn(ad_entry), *result, ldap_err2string(*result));
-
+#endif
+
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"<-- test_winsync_post_ad_mod_group_cb -- end\n");
@@ -1946,10 +1963,12 @@ test_winsync_post_ds_mod_user_cb(void *cookie, const Slapi_Entry *rawentry, Slap
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"--> test_winsync_post_ds_mod_user_cb -- begin\n");
+#ifdef THIS_IS_JUST_AN_EXAMPLE
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"Result of modifying DS entry [%s] was [%d:%s]\n",
slapi_entry_get_dn(ds_entry), *result, ldap_err2string(*result));
-
+#endif
+
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"<-- test_winsync_post_ds_mod_user_cb -- end\n");
@@ -1962,10 +1981,12 @@ test_winsync_post_ds_mod_group_cb(void *cookie, const Slapi_Entry *rawentry, Sla
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"--> test_winsync_post_ds_mod_group_cb -- begin\n");
+#ifdef THIS_IS_JUST_AN_EXAMPLE
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"Result of modifying DS entry [%s] was [%d:%s]\n",
slapi_entry_get_dn(ds_entry), *result, ldap_err2string(*result));
-
+#endif
+
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"<-- test_winsync_post_ds_mod_group_cb -- end\n");
@@ -1978,10 +1999,12 @@ test_winsync_post_ds_add_user_cb(void *cookie, const Slapi_Entry *rawentry, Slap
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"--> test_winsync_post_ds_add_user_cb -- begin\n");
+#ifdef THIS_IS_JUST_AN_EXAMPLE
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"Result of adding DS entry [%s] was [%d:%s]\n",
slapi_entry_get_dn(ds_entry), *result, ldap_err2string(*result));
-
+#endif
+
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"<-- test_winsync_post_ds_add_user_cb -- end\n");
@@ -1994,10 +2017,12 @@ test_winsync_post_ds_add_group_cb(void *cookie, const Slapi_Entry *rawentry, Sla
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"--> test_winsync_post_ds_add_group_cb -- begin\n");
+#ifdef THIS_IS_JUST_AN_EXAMPLE
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"Result of adding DS entry [%s] was [%d:%s]\n",
slapi_entry_get_dn(ds_entry), *result, ldap_err2string(*result));
-
+#endif
+
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"<-- test_winsync_post_ds_add_group_cb -- end\n");
@@ -2010,11 +2035,13 @@ test_winsync_pre_ad_add_user_cb(void *cookie, Slapi_Entry *ds_entry, Slapi_Entry
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"--> test_winsync_pre_ad_add_user_cb -- begin\n");
+#ifdef THIS_IS_JUST_AN_EXAMPLE
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"Adding AD entry [%s] from add of DS entry [%s]\n",
slapi_entry_get_dn(ad_entry), slapi_entry_get_dn(ds_entry));
/* make modifications to ad_entry here */
-
+#endif
+
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"<-- test_winsync_pre_ad_add_user_cb -- end\n");
@@ -2027,11 +2054,13 @@ test_winsync_pre_ad_add_group_cb(void *cookie, Slapi_Entry *ds_entry, Slapi_Entr
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"--> test_winsync_pre_ad_add_group_cb -- begin\n");
+#ifdef THIS_IS_JUST_AN_EXAMPLE
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"Adding AD entry [%s] from add of DS entry [%s]\n",
slapi_entry_get_dn(ad_entry), slapi_entry_get_dn(ds_entry));
/* make modifications to ad_entry here */
-
+#endif
+
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"<-- test_winsync_pre_ad_add_group_cb -- end\n");
@@ -2044,10 +2073,12 @@ test_winsync_post_ad_add_user_cb(void *cookie, Slapi_Entry *ds_entry, Slapi_Entr
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"--> test_winsync_post_ad_add_user_cb -- begin\n");
+#ifdef THIS_IS_JUST_AN_EXAMPLE
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"Result of adding AD entry [%s] was [%d:%s]\n",
slapi_entry_get_dn(ad_entry), *result, ldap_err2string(*result));
-
+#endif
+
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"<-- test_winsync_post_ad_add_user_cb -- end\n");
@@ -2060,10 +2091,12 @@ test_winsync_post_ad_add_group_cb(void *cookie, Slapi_Entry *ds_entry, Slapi_Ent
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"--> test_winsync_post_ad_add_group_cb -- begin\n");
+#ifdef THIS_IS_JUST_AN_EXAMPLE
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"Result of adding AD entry [%s] was [%d:%s]\n",
slapi_entry_get_dn(ad_entry), *result, ldap_err2string(*result));
-
+#endif
+
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"<-- test_winsync_post_ad_add_group_cb -- end\n");
@@ -2076,10 +2109,12 @@ test_winsync_post_ad_mod_user_mods_cb(void *cookie, const Slapi_Entry *rawentry,
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"--> test_winsync_post_ad_mod_user_mods_cb -- begin\n");
+#ifdef THIS_IS_JUST_AN_EXAMPLE
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"Result of modifying AD entry [%s] was [%d:%s]\n",
slapi_sdn_get_dn(remote_dn), *result, ldap_err2string(*result));
-
+#endif
+
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"<-- test_winsync_post_ad_mod_user_mods_cb -- end\n");
@@ -2092,10 +2127,12 @@ test_winsync_post_ad_mod_group_mods_cb(void *cookie, const Slapi_Entry *rawentry
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"--> test_winsync_post_ad_mod_group_mods_cb -- begin\n");
+#ifdef THIS_IS_JUST_AN_EXAMPLE
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"Result of modifying AD entry [%s] was [%d:%s]\n",
slapi_sdn_get_dn(remote_dn), *result, ldap_err2string(*result));
-
+#endif
+
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"<-- test_winsync_post_ad_mod_group_mods_cb -- end\n");
diff --git a/ldap/servers/plugins/replication/windows_protocol_util.c b/ldap/servers/plugins/replication/windows_protocol_util.c
index 0b16bde..af3cfa1 100644
--- a/ldap/servers/plugins/replication/windows_protocol_util.c
+++ b/ldap/servers/plugins/replication/windows_protocol_util.c
@@ -4204,6 +4204,16 @@ windows_generate_update_mods(Private_Repl_Protocol *prp,Slapi_Entry *remote_entr
LDAPDebug( LDAP_DEBUG_TRACE, "=> windows_generate_update_mods\n", 0, 0, 0 );
*do_modify = 0;
+
+ if (!remote_entry || !local_entry) {
+ slapi_log_error(SLAPI_LOG_REPL, windows_repl_plugin_name,
+ "%s: windows_generate_update_mods: remote_entry is [%s] local_entry is [%s] "
+ "cannot generate update mods\n", agmt_get_long_name(prp->agmt),
+ remote_entry ? slapi_entry_get_dn_const(remote_entry) : "NULL",
+ local_entry ? slapi_entry_get_dn_const(local_entry) : "NULL");
+ goto bail;
+ }
+
if (to_windows)
{
windows_is_local_entry_user_or_group(remote_entry,&is_user,&is_group);
@@ -4212,8 +4222,8 @@ windows_generate_update_mods(Private_Repl_Protocol *prp,Slapi_Entry *remote_entr
windows_is_remote_entry_user_or_group(remote_entry,&is_user,&is_group);
}
- for (rc = slapi_entry_first_attr(remote_entry, &attr); rc == 0;
- rc = slapi_entry_next_attr(remote_entry, attr, &attr))
+ for (rc = slapi_entry_first_attr(remote_entry, &attr); rc == 0;
+ rc = slapi_entry_next_attr(remote_entry, attr, &attr))
{
int is_present_local = 0;
char *type = NULL;
@@ -4383,9 +4393,9 @@ windows_generate_update_mods(Private_Repl_Protocol *prp,Slapi_Entry *remote_entr
"local attribute %s in local entry %s for remote attribute "
"%s in remote entry %s\n",
local_type ? local_type : "NULL",
- local_entry ? slapi_entry_get_dn(local_entry) : "NULL",
+ slapi_entry_get_dn(local_entry),
type ? type : "NULL",
- remote_entry ? slapi_entry_get_dn(remote_entry) : "NULL");
+ slapi_entry_get_dn(remote_entry));
}
slapi_valueset_free(local_values);
local_values = NULL;
@@ -4395,9 +4405,9 @@ windows_generate_update_mods(Private_Repl_Protocol *prp,Slapi_Entry *remote_entr
"local attribute %s in local entry %s for remote attribute "
"%s in remote entry %s\n",
local_type ? local_type : "NULL",
- local_entry ? slapi_entry_get_dn(local_entry) : "NULL",
+ slapi_entry_get_dn(local_entry),
type ? type : "NULL",
- remote_entry ? slapi_entry_get_dn(remote_entry) : "NULL");
+ slapi_entry_get_dn(remote_entry));
}
slapi_valueset_free(mapped_remote_values);
mapped_remote_values = NULL;
@@ -4407,9 +4417,9 @@ windows_generate_update_mods(Private_Repl_Protocol *prp,Slapi_Entry *remote_entr
"local attribute %s in local entry %s for remote attribute "
"%s in remote entry %s\n",
local_type ? local_type : "NULL",
- local_entry ? slapi_entry_get_dn(local_entry) : "NULL",
+ slapi_entry_get_dn(local_entry),
type ? type : "NULL",
- remote_entry ? slapi_entry_get_dn(remote_entry) : "NULL");
+ slapi_entry_get_dn(remote_entry));
}
}
} else
@@ -4580,6 +4590,7 @@ windows_generate_update_mods(Private_Repl_Protocol *prp,Slapi_Entry *remote_entr
{
slapi_mods_dump(smods,"windows sync");
}
+bail:
LDAPDebug( LDAP_DEBUG_TRACE, "<= windows_generate_update_mods: %d\n", retval, 0, 0 );
return retval;
}
--
1.7.1
--
389-devel mailing list
389-devel@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/389-devel
