From 80ee43bf0f6ad4a8702f0695b9294a3797483ebe Mon Sep 17 00:00:00 2001
From: Rich Megginson <rmeggins@redhat.com>
Date: Fri, 12 Sep 2014 13:11:26 -0600
Subject: [PATCH] Ticket #47892 coverity defects found in 1.3.3.1
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
https://fedorahosted.org/389/ticket/47892
Reviewed by: mreynolds, nkinder (Thanks!)
Branch: rhel-7.1
Fix Description:
9. 389-ds-base-1.3.3.1/ldap/servers/plugins/memberof/memberof.c:2079:var_compare_op – Comparing "group_norm_vals" to null implies that "group_norm_vals" might be null.
11. 389-ds-base-1.3.3.1/ldap/servers/plugins/memberof/memberof.c:2099:var_deref_model – Passing null pointer "group_norm_vals" to function "slapi_valueset_add_value_ext(Slapi_ValueSet *, Slapi_Value const *, unsigned long)", which dereferences it.
12. 389-ds-base-1.3.3.1/ldap/servers/slapd/valueset.c:896:2:deref_parm_in_call – Function "slapi_valueset_add_attr_valuearray_ext(Slapi_Attr const *, Slapi_ValueSet *, Slapi_Value **, int, unsigned long, int *)" dereferences "vs".
15. 389-ds-base-1.3.3.1/ldap/servers/slapd/valueset.c:1075:2:deref_parm – Directly dereferencing parameter "vs".
Added a check for group_norm_vals == NULL at beginning of function. I think this had a chain effect causing 11, 12, and 15 as well.
various - deprecated conversion from string constant - added const_cast<char *> as recommended by C++ guides.
2. 389-ds-base-1.3.3.1/ldap/servers/slapd/back-ldbm/ldif2ldbm.c:2198:78:warning – 'j' may be used uninitialized in this function [-Wmaybe-uninitialized]
Should have been using SLAPI_ATTR_TOMBSTONE_CSN
2. 389-ds-base-1.3.3.1/ldap/servers/plugins/acl/aclparse.c:538:28:warning – 'is_target_to' may be used uninitialized in this function [-Wmaybe-uninitialized]
2. 389-ds-base-1.3.3.1/ldap/servers/plugins/acl/acl.c:2493:26:warning – 'attrFilterArray' may be used uninitialized in this function [-Wmaybe-uninitialized]
These are false positives.
The minor memleaks were also fixed.
Platforms tested: Fedora 20
Flag Day: no
Doc impact: no
(cherry picked from commit 66e43aee7151acf6939b1a646eb869c7ccf0f7a4)
(cherry picked from commit 6dc23ec794cd5644a40c223e7b66066f195d8d7d)
---
ldap/servers/plugins/memberof/memberof.c | 6 +++---
ldap/servers/slapd/back-ldbm/ldif2ldbm.c | 2 +-
ldap/servers/slapd/tools/ldif.c | 5 ++++-
ldap/servers/slapd/tools/rsearch/nametable.c | 1 +
lib/base/pool.cpp | 10 +++++-----
lib/libaccess/aclcache.cpp | 2 +-
6 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/ldap/servers/plugins/memberof/memberof.c b/ldap/servers/plugins/memberof/memberof.c
index 6549718..bd87ee9 100644
--- a/ldap/servers/plugins/memberof/memberof.c
+++ b/ldap/servers/plugins/memberof/memberof.c
@@ -2044,10 +2044,10 @@ int memberof_get_groups_callback(Slapi_Entry *e, void *callback_data)
goto bail;
}
- if (!groupvals)
+ if (!groupvals || !group_norm_vals)
{
slapi_log_error( SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM,
- "memberof_get_groups_callback: NULL groupvals\n");
+ "memberof_get_groups_callback: NULL groupvals or group_norm_vals\n");
rc = -1;
goto bail;
@@ -2076,7 +2076,7 @@ int memberof_get_groups_callback(Slapi_Entry *e, void *callback_data)
* in config. We only need this attribute for it's syntax so the comparison can be
* performed. Since all of the grouping attributes are validated to use the Dinstinguished
* Name syntax, we can safely just use the first group_slapiattr. */
- if (group_norm_vals && slapi_valueset_find(
+ if (slapi_valueset_find(
((memberof_get_groups_data*)callback_data)->config->group_slapiattrs[0], group_norm_vals, group_ndn_val))
{
/* we either hit a recursive grouping, or an entry is
diff --git a/ldap/servers/slapd/back-ldbm/ldif2ldbm.c b/ldap/servers/slapd/back-ldbm/ldif2ldbm.c
index ab3a4a5..523da09 100644
--- a/ldap/servers/slapd/back-ldbm/ldif2ldbm.c
+++ b/ldap/servers/slapd/back-ldbm/ldif2ldbm.c
@@ -2195,7 +2195,7 @@ ldbm_back_ldbm2index(Slapi_PBlock *pb)
if (task) {
slapi_task_log_notice(task, "%s: ERROR: failed to begin txn for "
"update index '%s' (err %d: %s)",
- inst->inst_name, indexAttrs[j], rc,
+ inst->inst_name, SLAPI_ATTR_TOMBSTONE_CSN, rc,
dblayer_strerror(rc));
}
return_value = -2;
diff --git a/ldap/servers/slapd/tools/ldif.c b/ldap/servers/slapd/tools/ldif.c
index e8e6f9b..e4527ba 100644
--- a/ldap/servers/slapd/tools/ldif.c
+++ b/ldap/servers/slapd/tools/ldif.c
@@ -142,7 +142,8 @@ int main( int argc, char **argv )
}
if (( out = ldif_type_and_value( type, val, cur )) == NULL ) {
- perror( "ldif_type_and_value" );
+ perror( "ldif_type_and_value" );
+ free( val );
return( 1 );
}
@@ -166,6 +167,7 @@ int main( int argc, char **argv )
maxlen *= 2;
if( (buf = (char *)realloc(buf, maxlen)) == NULL ) {
perror( "realloc" );
+ free( buf );
return( 1 );
}
(void)fgets(buf+curlen, maxlen/2 + 1, stdin);
@@ -176,6 +178,7 @@ int main( int argc, char **argv )
if (( out = ldif_type_and_value( type, buf, strlen( buf ) ))
== NULL ) {
perror( "ldif_type_and_value" );
+ free( buf );
return( 1 );
}
fputs( out, stdout );
diff --git a/ldap/servers/slapd/tools/rsearch/nametable.c b/ldap/servers/slapd/tools/rsearch/nametable.c
index 03a6ae1..9d56a34 100644
--- a/ldap/servers/slapd/tools/rsearch/nametable.c
+++ b/ldap/servers/slapd/tools/rsearch/nametable.c
@@ -160,6 +160,7 @@ int nt_load(NameTable *nt, const char *filename)
free(s);
break;
}
+ free(s);
}
PR_Close(fd);
return nt->size;
diff --git a/lib/base/pool.cpp b/lib/base/pool.cpp
index 9ee1b8c..ab952d1 100644
--- a/lib/base/pool.cpp
+++ b/lib/base/pool.cpp
@@ -178,7 +178,7 @@ _create_block(int size)
crit_exit(freelist_lock);
if (((newblock = (block_t *)PERM_MALLOC(sizeof(block_t))) == NULL) ||
((newblock->data = (char *)PERM_MALLOC(bytes)) == NULL)) {
- ereport(LOG_CATASTROPHE, "%s", XP_GetAdminStr(DBT_poolCreateBlockOutOfMemory_));
+ ereport(LOG_CATASTROPHE, const_cast<char *>("%s"), XP_GetAdminStr(DBT_poolCreateBlockOutOfMemory_));
if (newblock)
PERM_FREE(newblock);
return NULL;
@@ -259,7 +259,7 @@ pool_create()
}
if ( (newpool->curr_block =_create_block(BLOCK_SIZE)) == NULL) {
- ereport(LOG_CATASTROPHE, "%s", XP_GetAdminStr(DBT_poolCreateOutOfMemory_));
+ ereport(LOG_CATASTROPHE, const_cast<char *>("%s"), XP_GetAdminStr(DBT_poolCreateOutOfMemory_));
PERM_FREE(newpool);
return NULL;
}
@@ -280,7 +280,7 @@ pool_create()
crit_exit(known_pools_lock);
}
else
- ereport(LOG_CATASTROPHE, "%s", XP_GetAdminStr(DBT_poolCreateOutOfMemory_1));
+ ereport(LOG_CATASTROPHE, const_cast<char *>("%s"), XP_GetAdminStr(DBT_poolCreateOutOfMemory_1));
return (pool_handle_t *)newpool;
}
@@ -386,7 +386,7 @@ pool_malloc(pool_handle_t *pool_handle, size_t size)
*/
blocksize = ( (size + BLOCK_SIZE-1) / BLOCK_SIZE ) * BLOCK_SIZE;
if ( (pool->curr_block = _create_block(blocksize)) == NULL) {
- ereport(LOG_CATASTROPHE, "%s", XP_GetAdminStr(DBT_poolMallocOutOfMemory_));
+ ereport(LOG_CATASTROPHE, const_cast<char *>("%s"), XP_GetAdminStr(DBT_poolMallocOutOfMemory_));
#ifdef POOL_LOCKING
crit_exit(pool->lock);
#endif
@@ -408,7 +408,7 @@ pool_malloc(pool_handle_t *pool_handle, size_t size)
void _pool_free_error()
{
- ereport(LOG_WARN, "%s", XP_GetAdminStr(DBT_freeUsedWherePermFreeShouldHaveB_));
+ ereport(LOG_WARN, const_cast<char *>("%s"), XP_GetAdminStr(DBT_freeUsedWherePermFreeShouldHaveB_));
return;
}
diff --git a/lib/libaccess/aclcache.cpp b/lib/libaccess/aclcache.cpp
index 52e019b..a96b0c9 100644
--- a/lib/libaccess/aclcache.cpp
+++ b/lib/libaccess/aclcache.cpp
@@ -133,7 +133,7 @@ ACL_ListHashInit()
&ACLPermAllocOps,
NULL);
if (ACLListHash == NULL) {
- ereport(LOG_SECURITY, "Unable to allocate ACL List Hash\n");
+ ereport(LOG_SECURITY, const_cast<char *>("Unable to allocate ACL List Hash\n"));
return;
}
--
1.9.3