Coverity Fixes (Part 3)

11692 - Explicit null dereferenced (libavl/avl.c)
11695 - Explicit null dereferenced (cb_conn_stateless.c)
11696 - Explicit null dereferenced (memberof_config.c)
11697 - Explicit null dereferenced (memberof.c)
11698 - Explicit null dereferenced (memberof.c)
11699 - Explicit null dereferenced (memberof.c)
11700 - Explicit null dereferenced (memberof.c)
11701 - Explicit null dereferenced (cl5_api.c)
11702 - Explicit null dereferenced (cl5_api.c)
11703 - Dereference after null check (cl5_clcache.c)
11704 - Dereference after null check (repl5_replica_config.c)
11705 - Explicit null dereferenced (syntaxes/string.c)
11706 - Dereference after null check (plugin.c)
11707 - Dereference after null check (plugin.c)
11711 - Dereference after null check (ldif2ldbm.c)
11726 - Dereference after null check (valueset.c)
11729 - Explicit null dereferenced (libaccess/oneeval.cpp)
11744 - Explicit null dereferenced (dbverify.c)
11745 - Out-of-bounds read (linked_attrs.c)
11745 - Out-of-bounds read (memberof.c)

https://bugzilla.redhat.com/show_bug.cgi?id=970221

Reviewed by: richm(Thanks!)

ldap/libraries/libavl/avl.c

@@ -780,8 +780,11 @@ avl_getfirst( Avlnode *root )
 		return( 0 );
 
 	(void) avl_apply( root, avl_buildlist, (caddr_t) 0, -1, AVL_INORDER );
-
-	return( avl_list[ avl_nextlist++ ] );
+	if(avl_list && avl_list[avl_nextlist++]){
+		return avl_list[avl_nextlist];
+	} else {
+		return( NULL );
+	}
 }
 
 caddr_t

ldap/servers/plugins/chainingdb/cb_conn_stateless.c

@@ -856,7 +856,7 @@ void cb_stale_all_connections( cb_backend_instance * cb)
 	    else {
        	       	if (conn==pools[i]->conn.conn_list) {
        	       		pools[i]->conn.conn_list=next_conn;
-       	       	} else {
+       	       	} else if(prev_conn){
        	       		prev_conn->next=next_conn;
        	       	}
        	       	cb_close_and_dispose_connection(conn);

ldap/servers/plugins/linkedattrs/linked_attrs.c

@@ -1231,10 +1231,21 @@ linked_attrs_load_array(Slapi_Value **array, Slapi_Attr *attr)
 int
 linked_attrs_compare(const void *a, const void *b)
 {
+        Slapi_Value *val1;
+        Slapi_Value *val2;
+        Slapi_Attr *linkattr;
         int rc = 0;
-        Slapi_Value *val1 = *((Slapi_Value **)a);
-        Slapi_Value *val2 = *((Slapi_Value **)b);
-        Slapi_Attr *linkattr = slapi_attr_new();
+
+        if(a == NULL && b != NULL){
+            return 1;
+        } else if(a != NULL && b == NULL){
+            return -1;
+        } else if(a == NULL && b == NULL){
+            return 0;
+        }
+        val1 = *((Slapi_Value **)a);
+        val2 = *((Slapi_Value **)b);
+        linkattr = slapi_attr_new();
 
         slapi_attr_init(linkattr, "distinguishedName");
 

ldap/servers/plugins/memberof/memberof.c

@@ -460,7 +460,7 @@ memberof_del_dn_from_groups(Slapi_PBlock *pb, MemberOfConfig *config, Slapi_DN *
 	/* Loop through each grouping attribute to find groups that have
 	 * dn as a member.  For any matches, delete the dn value from the
 	 * same grouping attribute. */
-	for (i = 0; config->groupattrs[i]; i++)
+	for (i = 0; config->groupattrs && config->groupattrs[i]; i++)
 	{
 		memberof_del_dn_data data = {(char *)slapi_sdn_get_dn(sdn),
 		                             config->groupattrs[i]};
@@ -712,7 +712,7 @@ memberof_replace_dn_from_groups(Slapi_PBlock *pb, MemberOfConfig *config,
 	/* Loop through each grouping attribute to find groups that have
 	 * pre_dn as a member.  For any matches, replace pre_dn with post_dn
 	 * using the same grouping attribute. */
-	for (i = 0; config->groupattrs[i]; i++)
+	for (i = 0; config->groupattrs && config->groupattrs[i]; i++)
 	{
 		replace_dn_data data = {(char *)slapi_sdn_get_ndn(pre_sdn),
 		                        (char *)slapi_sdn_get_ndn(post_sdn),
@@ -2203,8 +2203,18 @@ void memberof_load_array(Slapi_Value **array, Slapi_Attr *attr)
  */
 int memberof_compare(MemberOfConfig *config, const void *a, const void *b)
 {
-	Slapi_Value *val1 = *((Slapi_Value **)a);
-	Slapi_Value *val2 = *((Slapi_Value **)b);
+	Slapi_Value *val1;
+	Slapi_Value *val2;
+
+	if(a == NULL && b != NULL){
+		return 1;
+	} else if(a != NULL && b == NULL){
+		return -1;
+	} else if(a == NULL && b == NULL){
+		return 0;
+	}
+	val1 = *((Slapi_Value **)a);
+	val2 = *((Slapi_Value **)b);
 
 	/* We only need to provide a Slapi_Attr here for it's syntax.  We
 	 * already validated all grouping attributes to use the Distinguished

ldap/servers/plugins/memberof/memberof_config.c

@@ -486,7 +486,7 @@ memberof_free_config(MemberOfConfig *config)
 		slapi_ch_array_free(config->groupattrs);
 		slapi_filter_free(config->group_filter, 1);
 
-		for (i = 0; config->group_slapiattrs[i]; i++)
+		for (i = 0; config->group_slapiattrs && config->group_slapiattrs[i]; i++)
 		{
 			slapi_attr_free(&config->group_slapiattrs[i]);
 		}

ldap/servers/plugins/replication/cl5_api.c

@@ -3507,6 +3507,13 @@ static void _cl5TrimFile (Object *obj, long *numToTrim, ReplicaId cleaned_rid)
 			 * This change can be trimmed if it exceeds purge
 			 * parameters and has been seen by all consumers.
 			 */
+			if(op.csn == NULL){
+				slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name_cl, "_cl5TrimFile: "
+						"Operation missing csn, moving on to next entry.\n");
+				cl5_operation_parameters_done (&op);
+				finished =_cl5GetNextEntry (&entry, it);
+				continue;
+			}
 			csn_rid = csn_get_replicaid (op.csn);
 			if ( (*numToTrim > 0 || _cl5CanTrim (entry.time, numToTrim)) &&
 				 ruv_covers_csn_strict (ruv, op.csn) )
@@ -3836,7 +3843,15 @@ static int  _cl5ConstructRUV (const char *replGen, Object *obj, PRBool purge)
     rc = _cl5GetFirstEntry (obj, &entry, &iterator, NULL);
     while (rc == CL5_SUCCESS)
     {
-        rid = csn_get_replicaid (op.csn);
+        if(op.csn){
+            rid = csn_get_replicaid (op.csn);
+        } else {
+            slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name_cl, "_cl5ConstructRUV: "
+                "Operation missing csn, moving on to next entry.\n");
+            cl5_operation_parameters_done (&op);
+            rc = _cl5GetNextEntry (&entry, iterator);
+            continue;
+        }
         if(is_cleaned_rid(rid)){
             /* skip this entry as the rid is invalid */
             slapi_log_error(SLAPI_LOG_REPL, repl_plugin_name_cl, "_cl5ConstructRUV: "

ldap/servers/plugins/replication/cl5_clcache.c

@@ -732,9 +732,14 @@ clcache_skip_change ( CLC_Buffer *buf )
 		 */
 		if ( csn_time_difference(buf->buf_current_csn, cscb->local_maxcsn) == 0 &&
 			 (csn_get_seqnum(buf->buf_current_csn) ==
-				csn_get_seqnum(cscb->local_maxcsn) + 1) ) {
-			csn_init_by_csn ( cscb->local_maxcsn, buf->buf_current_csn );
-			csn_init_by_csn ( cscb->consumer_maxcsn, buf->buf_current_csn );
+				csn_get_seqnum(cscb->local_maxcsn) + 1) )
+		{
+			if(cscb->local_maxcsn){
+				csn_init_by_csn ( cscb->local_maxcsn, buf->buf_current_csn );
+			}
+			if(cscb->consumer_maxcsn){
+				csn_init_by_csn ( cscb->consumer_maxcsn, buf->buf_current_csn );
+			}
 			skip = 0;
 			break;
 		}

ldap/servers/plugins/replication/repl5_replica_config.c

@@ -2364,6 +2364,11 @@ delete_cleaned_rid_config(cleanruv_data *clean_data)
     int found = 0, i;
     int rc, ret, rid;
 
+    if(clean_data == NULL){
+        slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name, "delete_cleaned_rid_config: cleanruv data is NULL, "
+                "failed to clean the config.\n");
+        return;
+    }
     /*
      *  If there is no maxcsn, set the proper csnstr
      */

ldap/servers/plugins/syntaxes/string.c

@@ -84,7 +84,11 @@ string_filter_ava( struct berval *bvfilter, Slapi_Value **bvals, int syntax,
 			bvfilter_norm.bv_val = alt;
 			alt = NULL;
 		}
-		bvfilter_norm.bv_len = strlen(bvfilter_norm.bv_val);
+		if(bvfilter_norm.bv_val){
+			bvfilter_norm.bv_len = strlen(bvfilter_norm.bv_val);
+		} else {
+			bvfilter_norm.bv_len = 0;
+		}
 	}
 
 	for ( i = 0; (bvals != NULL) && (bvals[i] != NULL); i++ ) {
@@ -103,7 +107,7 @@ string_filter_ava( struct berval *bvfilter, Slapi_Value **bvals, int syntax,
 							    if(retVal) {
 									*retVal = bvals[i];
 								}
-								slapi_ch_free ((void**)&bvfilter_norm.bv_val);
+								slapi_ch_free_string(&bvfilter_norm.bv_val);
                                 return( 0 );
                         }
                         break;
@@ -112,7 +116,7 @@ string_filter_ava( struct berval *bvfilter, Slapi_Value **bvals, int syntax,
 							    if(retVal) {
 									*retVal = bvals[i];
 								}
-								slapi_ch_free ((void**)&bvfilter_norm.bv_val);
+								slapi_ch_free_string(&bvfilter_norm.bv_val);
                                 return( 0 );
                         }
                         break;
@@ -121,14 +125,14 @@ string_filter_ava( struct berval *bvfilter, Slapi_Value **bvals, int syntax,
 							    if(retVal) {
 									*retVal = bvals[i];
 								}
-								slapi_ch_free ((void**)&bvfilter_norm.bv_val);
+								slapi_ch_free_string(&bvfilter_norm.bv_val);
                                 return( 0 );
                         }
                         break;
                 }
         }
 
-	slapi_ch_free ((void**)&bvfilter_norm.bv_val);
+	slapi_ch_free_string(&bvfilter_norm.bv_val);
 	return( -1 );
 }
 

ldap/servers/slapd/back-ldbm/dbverify.c

@@ -119,9 +119,11 @@ dbverify_ext( ldbm_instance *inst, int verbose )
             char *p = NULL;
             p = strstr(filep, LDBM_FILENAME_SUFFIX); /* since already checked,
                                                         it must have it */
-            *p = '\0';
+            if(p)
+                *p = '\0';
             ainfo_get( inst->inst_be, filep+1, &ai );
-            *p = '.';
+            if(p)
+                *p = '.';
             if (ai->ai_key_cmp_fn) {
                 dbp->app_private = (void *)ai->ai_key_cmp_fn;
                 dbp->set_bt_compare(dbp, dblayer_bt_compare);

ldap/servers/slapd/back-ldbm/ldif2ldbm.c

@@ -2246,15 +2246,23 @@ ldbm_back_ldbm2index(Slapi_PBlock *pb)
          * Update the Virtual List View indexes
          */
         for ( vlvidx = 0; vlvidx < numvlv; vlvidx++ ) {
+            char *ai = "Unknown index";
+
             if ( g_get_shutdown() || c_get_shutdown() ) {
                 goto err_out;
             }
+            if(indexAttrs){
+                  if(indexAttrs[vlvidx]){
+                      ai = indexAttrs[vlvidx];
+                  }
+            }
             if (!run_from_cmdline) {
                 rc = dblayer_txn_begin(li, NULL, &txn);
                 if (0 != rc) {
+
                     LDAPDebug(LDAP_DEBUG_ANY,
                       "%s: ERROR: failed to begin txn for update index '%s'\n",
-                      inst->inst_name, indexAttrs[vlvidx], 0);
+                      inst->inst_name, ai, 0);
                     LDAPDebug(LDAP_DEBUG_ANY,
                         "%s: Error %d: %s\n", inst->inst_name, rc,
                         dblayer_strerror(rc));
@@ -2262,7 +2270,7 @@ ldbm_back_ldbm2index(Slapi_PBlock *pb)
                         slapi_task_log_notice(task,
                          "%s: ERROR: failed to begin txn for update index '%s' "
                          "(err %d: %s)", inst->inst_name,
-                         indexAttrs[vlvidx], rc, dblayer_strerror(rc));
+                         ai, rc, dblayer_strerror(rc));
                     }
                     return_value = -2;
                     goto err_out;
@@ -2281,7 +2289,7 @@ ldbm_back_ldbm2index(Slapi_PBlock *pb)
                 if (0 != rc) {
                     LDAPDebug(LDAP_DEBUG_ANY,
                       "%s: ERROR: failed to commit txn for update index '%s'\n",
-                      inst->inst_name, indexAttrs[vlvidx], 0);
+                      inst->inst_name, ai, 0);
                     LDAPDebug(LDAP_DEBUG_ANY,
                         "%s: Error %d: %s\n", inst->inst_name, rc,
                         dblayer_strerror(rc));
@@ -2289,7 +2297,7 @@ ldbm_back_ldbm2index(Slapi_PBlock *pb)
                         slapi_task_log_notice(task,
                         "%s: ERROR: failed to commit txn for update index '%s' "
                         "(err %d: %s)", inst->inst_name,
-                        indexAttrs[vlvidx], rc, dblayer_strerror(rc));
+                        ai, rc, dblayer_strerror(rc));
                     }
                     return_value = -2;
                     goto err_out;

ldap/servers/slapd/dn.c

@@ -2567,3 +2567,4 @@ slapi_sdn_get_size(const Slapi_DN *sdn)
     sz += strlen(sdn->dn) + 1;
     return sz;
 }
+

ldap/servers/slapd/plugin.c

@@ -1493,15 +1493,24 @@ int
 slapi_berval_cmp (const struct berval* L, const struct berval* R) /* JCM - This does not belong here. But, where should it go? */
 {
     int result = 0;
+
+    if(L == NULL && R != NULL){
+        return 1;
+    } else if(L != NULL && R == NULL){
+        return -1;
+    } else if(L == NULL && R == NULL){
+        return 0;
+    }
     if (L->bv_len < R->bv_len) {
-	result = memcmp (L->bv_val, R->bv_val, L->bv_len);
-	if (result == 0)
-	  result = -1;
+        result = memcmp (L->bv_val, R->bv_val, L->bv_len);
+        if (result == 0)
+            result = -1;
     } else {
-	result = memcmp (L->bv_val, R->bv_val, R->bv_len);
-	if (result == 0 && (L->bv_len > R->bv_len))
-	  result = 1;
+        result = memcmp (L->bv_val, R->bv_val, R->bv_len);
+        if (result == 0 && (L->bv_len > R->bv_len))
+            result = 1;
     }
+
     return result;
 }
 

ldap/servers/slapd/valueset.c

@@ -190,20 +190,23 @@ valuearray_add_valuearray(Slapi_Value ***vals, Slapi_Value **addvals, PRUint32 f
 {
     int valslen;
     int addvalslen;
-	int maxvals;
+    int maxvals;
 
-	addvalslen= valuearray_count(addvals);
+    if(vals == NULL){
+        return;
+    }
+    addvalslen= valuearray_count(addvals);
     if(*vals == NULL)
     {
-		valslen= 0;
-		maxvals= 0;
+        valslen= 0;
+        maxvals= 0;
     }
     else
     {
-		valslen= valuearray_count(*vals);
-		maxvals= valslen+1;
+        valslen= valuearray_count(*vals);
+        maxvals= valslen+1;
     }
-	valuearray_add_valuearray_fast(vals,addvals,valslen,addvalslen,&maxvals,1/*Exact*/,flags & SLAPI_VALUE_FLAG_PASSIN);
+    valuearray_add_valuearray_fast(vals,addvals,valslen,addvalslen,&maxvals,1/*Exact*/,flags & SLAPI_VALUE_FLAG_PASSIN);
 }
 
 int

lib/libaccess/oneeval.cpp

@@ -381,20 +381,19 @@ ACLEvalBuildContext(
     /* 	  Loop through all the ACLs in the list    */
     while (wrapper)        
     {
-	acl = wrapper->acl;
+        acl = wrapper->acl;
         ace = acl->expr_list_head;
 
         while (ace)    /* Loop through all the ACEs in this ACL    */
         {
-
             /* allocate a new ace list entry and link it in    to the ordered
              * list.
              */
             new_ace = (ACLAceEntry_t *)PERM_CALLOC(sizeof(ACLAceEntry_t));
             if (new_ace == (ACLAceEntry_t *)NULL) {
-		nserrGenerate(errp, ACLERRNOMEM, ACLERR4020, ACL_Program, 1,
-		XP_GetAdminStr(DBT_EvalBuildContextUnableToAllocAceEntry));
-		goto error;
+                nserrGenerate(errp, ACLERRNOMEM, ACLERR4020, ACL_Program, 1,
+                XP_GetAdminStr(DBT_EvalBuildContextUnableToAllocAceEntry));
+                goto error;
             }
             new_ace->acep    = ace;
             ace_cnt++;
@@ -402,7 +401,8 @@ ACLEvalBuildContext(
             if (cache->acelist == NULL)
                 cache->acelist = acelast    = new_ace;
             else {
-                acelast->next  = new_ace;
+                if(acelast)
+                    acelast->next  = new_ace;
                 acelast        = new_ace;
                 new_ace->acep  = ace;
             }