Blob Blame Raw
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