From 1f463f1401b0adfd12cca7851d57a72fa6c58ce0 Mon Sep 17 00:00:00 2001

From: Mark Reynolds <mreynolds@redhat.com>

Date: Wed, 5 Jun 2013 12:15:05 -0400

Subject: [PATCH 91/99] 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!)

(cherry picked from commit 36f25726b9723f743bc240cb44b88f74ad478ef2)

---

 ldap/libraries/libavl/avl.c                         |  7 +++++--

 ldap/servers/plugins/chainingdb/cb_conn_stateless.c |  2 +-

 ldap/servers/plugins/linkedattrs/linked_attrs.c     | 17 ++++++++++++++---

 ldap/servers/plugins/memberof/memberof.c            | 18 ++++++++++++++----

 ldap/servers/plugins/memberof/memberof_config.c     |  2 +-

 ldap/servers/plugins/replication/cl5_api.c          | 17 ++++++++++++++++-

 ldap/servers/plugins/replication/cl5_clcache.c      | 11 ++++++++---

 .../plugins/replication/repl5_replica_config.c      |  5 +++++

 ldap/servers/plugins/syntaxes/string.c              | 14 +++++++++-----

 ldap/servers/slapd/back-ldbm/dbverify.c             |  6 ++++--

 ldap/servers/slapd/back-ldbm/ldif2ldbm.c            | 16 ++++++++++++----

 ldap/servers/slapd/dn.c                             |  1 +

 ldap/servers/slapd/plugin.c                         | 21 +++++++++++++++------

 ldap/servers/slapd/valueset.c                       | 17 ++++++++++-------

 lib/libaccess/oneeval.cpp                           | 12 ++++++------

 15 files changed, 121 insertions(+), 45 deletions(-)

diff --git a/ldap/libraries/libavl/avl.c b/ldap/libraries/libavl/avl.c

index 7577891..18c43e0 100644

--- a/ldap/libraries/libavl/avl.c

+++ b/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

diff --git a/ldap/servers/plugins/chainingdb/cb_conn_stateless.c b/ldap/servers/plugins/chainingdb/cb_conn_stateless.c

index a9abc31..a85b392 100644

--- a/ldap/servers/plugins/chainingdb/cb_conn_stateless.c

+++ b/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);

diff --git a/ldap/servers/plugins/linkedattrs/linked_attrs.c b/ldap/servers/plugins/linkedattrs/linked_attrs.c

index ff3dc3a..4bea10f 100644

--- a/ldap/servers/plugins/linkedattrs/linked_attrs.c

+++ b/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");

diff --git a/ldap/servers/plugins/memberof/memberof.c b/ldap/servers/plugins/memberof/memberof.c

index a3f875d..d11983b 100644

--- a/ldap/servers/plugins/memberof/memberof.c

+++ b/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

diff --git a/ldap/servers/plugins/memberof/memberof_config.c b/ldap/servers/plugins/memberof/memberof_config.c

index b4d557a..3fd63a9 100644

--- a/ldap/servers/plugins/memberof/memberof_config.c

+++ b/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]);

 		}

diff --git a/ldap/servers/plugins/replication/cl5_api.c b/ldap/servers/plugins/replication/cl5_api.c

index a06c43f..f17650d 100644

--- a/ldap/servers/plugins/replication/cl5_api.c

+++ b/ldap/servers/plugins/replication/cl5_api.c

@@ -3506,6 +3506,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) )

@@ -3835,7 +3842,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: "

diff --git a/ldap/servers/plugins/replication/cl5_clcache.c b/ldap/servers/plugins/replication/cl5_clcache.c

index 5329b4b..67e64f5 100644

--- a/ldap/servers/plugins/replication/cl5_clcache.c

+++ b/ldap/servers/plugins/replication/cl5_clcache.c

@@ -750,9 +750,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;

 		}

diff --git a/ldap/servers/plugins/replication/repl5_replica_config.c b/ldap/servers/plugins/replication/repl5_replica_config.c

index 7c625eb..7b684e9 100644

--- a/ldap/servers/plugins/replication/repl5_replica_config.c

+++ b/ldap/servers/plugins/replication/repl5_replica_config.c

@@ -2366,6 +2366,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

      */

diff --git a/ldap/servers/plugins/syntaxes/string.c b/ldap/servers/plugins/syntaxes/string.c

index 54cd7c8..6c0da94 100644

--- a/ldap/servers/plugins/syntaxes/string.c

+++ b/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 );

 }

diff --git a/ldap/servers/slapd/back-ldbm/dbverify.c b/ldap/servers/slapd/back-ldbm/dbverify.c

index 43fc9d5..ffd5900 100644

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

+++ b/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);

diff --git a/ldap/servers/slapd/back-ldbm/ldif2ldbm.c b/ldap/servers/slapd/back-ldbm/ldif2ldbm.c

index c802ff2..47e0269 100644

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

+++ b/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;

diff --git a/ldap/servers/slapd/dn.c b/ldap/servers/slapd/dn.c

index 2f50e97..dda439b 100644

--- a/ldap/servers/slapd/dn.c

+++ b/ldap/servers/slapd/dn.c

@@ -2614,3 +2614,4 @@ slapi_sdn_get_size(const Slapi_DN *sdn)

     }

     return sz;

 }

+

diff --git a/ldap/servers/slapd/plugin.c b/ldap/servers/slapd/plugin.c

index 436cc02..0a4f4e0 100644

--- a/ldap/servers/slapd/plugin.c

+++ b/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;

 }

diff --git a/ldap/servers/slapd/valueset.c b/ldap/servers/slapd/valueset.c

index a91256c..f04acc3 100644

--- a/ldap/servers/slapd/valueset.c

+++ b/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

diff --git a/lib/libaccess/oneeval.cpp b/lib/libaccess/oneeval.cpp

index eff4e10..a6d3bbd 100644

--- a/lib/libaccess/oneeval.cpp

+++ b/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;

             }

-- 

1.8.1.4