From 80ee43bf0f6ad4a8702f0695b9294a3797483ebe Mon Sep 17 00:00:00 2001 From: Rich Megginson 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 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("%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("%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("%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("%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("%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("Unable to allocate ACL List Hash\n")); return; } -- 1.9.3