From 1f463f1401b0adfd12cca7851d57a72fa6c58ce0 Mon Sep 17 00:00:00 2001 From: Mark Reynolds 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