From f5730129dfbc4ef3808735051b12cbfda518f4fb Mon Sep 17 00:00:00 2001 From: Rich Megginson Date: Apr 09 2014 20:17:49 +0000 Subject: Ticket #47772 empty modify returns LDAP_INVALID_DN_SYNTAX https://fedorahosted.org/389/ticket/47772 Reviewed by: mreynolds (Thanks!) Branch: master Fix Description: Do not call normalize_mods2bvals if mods is NULL - just allow the empty modify op to proceed. Since normalize_mods2bvals only cares about DN syntax attributes, it is appropriate to return LDAP_INVALID_DN_SYNTAX if normalize_mods2bvals returns NULL. Since this fix allows NULL mods to proceed throughout the code, I checked for all places that SLAPI_MODIFY_MODS was used, to make sure we don't try to dereference NULL mods, and to make sure the NULL mods were handled correctly otherwise. Platforms tested: RHEL6 x86_64 Flag Day: no Doc impact: no --- diff --git a/ldap/servers/plugins/chainingdb/cb_config.c b/ldap/servers/plugins/chainingdb/cb_config.c index 7cbd7ba..25f466d 100644 --- a/ldap/servers/plugins/chainingdb/cb_config.c +++ b/ldap/servers/plugins/chainingdb/cb_config.c @@ -380,7 +380,7 @@ cb_config_modify_check_callback(Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slap slapi_pblock_get( pb, SLAPI_MODIFY_MODS, &mods ); - for (i = 0; mods[i] ; i++) { + for (i = 0; mods && mods[i] ; i++) { attr_name = mods[i]->mod_type; if ( !strcasecmp ( attr_name, CB_CONFIG_GLOBAL_FORWARD_CTRLS )) { @@ -413,7 +413,7 @@ cb_config_modify_callback(Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_Entr slapi_pblock_get( pb, SLAPI_MODIFY_MODS, &mods ); - for (i = 0; mods[i] ; i++) { + for (i = 0; mods && mods[i] ; i++) { attr_name = mods[i]->mod_type; if ( !strcasecmp ( attr_name, CB_CONFIG_GLOBAL_FORWARD_CTRLS )) { diff --git a/ldap/servers/plugins/chainingdb/cb_instance.c b/ldap/servers/plugins/chainingdb/cb_instance.c index 10958d7..fa9c441 100644 --- a/ldap/servers/plugins/chainingdb/cb_instance.c +++ b/ldap/servers/plugins/chainingdb/cb_instance.c @@ -306,7 +306,7 @@ int cb_instance_modify_config_check_callback(Slapi_PBlock *pb, Slapi_Entry* entr slapi_pblock_get( pb, SLAPI_MODIFY_MODS, &mods ); /* First pass to validate input */ - for (i = 0; mods[i] && LDAP_SUCCESS == rc; i++) { + for (i = 0; mods && mods[i] && LDAP_SUCCESS == rc; i++) { attr_name = mods[i]->mod_type; /* specific processing for multi-valued attributes */ @@ -379,7 +379,7 @@ int cb_instance_modify_config_callback(Slapi_PBlock *pb, Slapi_Entry* entryBefor /* input checked in the preop modify callback */ - for (i = 0; mods[i] && LDAP_SUCCESS == rc; i++) { + for (i = 0; mods && mods[i] && LDAP_SUCCESS == rc; i++) { attr_name = mods[i]->mod_type; /* specific processing for multi-valued attributes */ diff --git a/ldap/servers/plugins/chainingdb/cb_modify.c b/ldap/servers/plugins/chainingdb/cb_modify.c index 06e6b82..65acb58 100644 --- a/ldap/servers/plugins/chainingdb/cb_modify.c +++ b/ldap/servers/plugins/chainingdb/cb_modify.c @@ -291,7 +291,7 @@ cb_remove_illegal_mods(cb_backend_instance *inst, LDAPMod **mods) slapi_rwlock_wrlock(inst->rwl_config_lock); for (j=0; inst->illegal_attributes[j]; j++) { - for ( i = 0; mods[i] != NULL; i++ ) { + for ( i = 0; mods && mods[i] != NULL; i++ ) { if (slapi_attr_types_equivalent(inst->illegal_attributes[j],mods[i]->mod_type)) { tmp = mods[i]; for ( j = i; mods[j] != NULL; j++ ) { diff --git a/ldap/servers/plugins/replication/cl5_config.c b/ldap/servers/plugins/replication/cl5_config.c index e168bbd..55727c2 100644 --- a/ldap/servers/plugins/replication/cl5_config.c +++ b/ldap/servers/plugins/replication/cl5_config.c @@ -342,7 +342,7 @@ changelog5_config_modify (Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_Entr config.trimInterval = CL5_NUM_IGNORE; slapi_pblock_get( pb, SLAPI_MODIFY_MODS, &mods ); - for (i = 0; mods[i] != NULL; i++) + for (i = 0; mods && mods[i] != NULL; i++) { if (mods[i]->mod_op & LDAP_MOD_DELETE) { diff --git a/ldap/servers/plugins/replication/legacy_consumer.c b/ldap/servers/plugins/replication/legacy_consumer.c index aa5a9b5..8eb928b 100644 --- a/ldap/servers/plugins/replication/legacy_consumer.c +++ b/ldap/servers/plugins/replication/legacy_consumer.c @@ -321,7 +321,7 @@ legacy_consumer_config_modify (Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi slapi_pblock_get( pb, SLAPI_MODIFY_MODS, &mods ); slapi_rwlock_wrlock (legacy_consumer_config_lock); - for (i = 0; (mods[i] && (!not_allowed)); i++) + for (i = 0; mods && (mods[i] && (!not_allowed)); i++) { if (mods[i]->mod_op & LDAP_MOD_DELETE) { diff --git a/ldap/servers/plugins/replication/repl5_agmt.c b/ldap/servers/plugins/replication/repl5_agmt.c index 8fb5a02..139c8a3 100644 --- a/ldap/servers/plugins/replication/repl5_agmt.c +++ b/ldap/servers/plugins/replication/repl5_agmt.c @@ -1920,7 +1920,7 @@ agmt_notify_change(Repl_Agmt *agmt, Slapi_PBlock *pb) slapi_pblock_get(pb, SLAPI_MODIFY_MODS, &mods); slapi_rwlock_rdlock(agmt->attr_lock); - for (i = 0; !affects_non_fractional_attribute && NULL != agmt->frac_attrs[i]; i++) + for (i = 0; mods && !affects_non_fractional_attribute && NULL != agmt->frac_attrs[i]; i++) { for (j = 0; !affects_non_fractional_attribute && NULL != mods[j]; j++) { diff --git a/ldap/servers/plugins/replication/repl5_replica_config.c b/ldap/servers/plugins/replication/repl5_replica_config.c index a8c72c1..f433fc9 100644 --- a/ldap/servers/plugins/replication/repl5_replica_config.c +++ b/ldap/servers/plugins/replication/repl5_replica_config.c @@ -351,7 +351,7 @@ replica_config_modify (Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_Entry* if (*returncode != LDAP_SUCCESS) break; - for (i = 0; (mods[i] && (LDAP_SUCCESS == rc)); i++) + for (i = 0; mods && (mods[i] && (LDAP_SUCCESS == rc)); i++) { if (*returncode != LDAP_SUCCESS) break; @@ -699,7 +699,7 @@ replica_config_post_modify(Slapi_PBlock *pb, if (*returncode != LDAP_SUCCESS) break; - for (i = 0; (mods[i] && (LDAP_SUCCESS == rc)); i++) + for (i = 0; mods && (mods[i] && (LDAP_SUCCESS == rc)); i++) { if (*returncode != LDAP_SUCCESS) break; diff --git a/ldap/servers/plugins/uiduniq/7bit.c b/ldap/servers/plugins/uiduniq/7bit.c index 3474bf9..c84c79c 100644 --- a/ldap/servers/plugins/uiduniq/7bit.c +++ b/ldap/servers/plugins/uiduniq/7bit.c @@ -472,7 +472,7 @@ preop_modify(Slapi_PBlock *pb) which are add or replace ops and are bvalue encoded */ /* find out how many mods meet this criteria */ - for(mods=firstMods;*mods;mods++) + for(mods=firstMods;mods && *mods;mods++) { mod = *mods; if ((slapi_attr_type_cmp(mod->mod_type, attr_name, 1) == 0) && /* mod contains target attr */ diff --git a/ldap/servers/plugins/uiduniq/uid.c b/ldap/servers/plugins/uiduniq/uid.c index 984b93e..d4f0c84 100644 --- a/ldap/servers/plugins/uiduniq/uid.c +++ b/ldap/servers/plugins/uiduniq/uid.c @@ -761,7 +761,7 @@ preop_modify(Slapi_PBlock *pb) which are add or replace ops and are bvalue encoded */ /* find out how many mods meet this criteria */ - for(;*mods;mods++) + for(;mods && *mods;mods++) { mod = *mods; if ((slapi_attr_type_cmp(mod->mod_type, attrName, 1) == 0) && /* mod contains target attr */ diff --git a/ldap/servers/slapd/auditlog.c b/ldap/servers/slapd/auditlog.c index f6afd10..fabe21e 100644 --- a/ldap/servers/slapd/auditlog.c +++ b/ldap/servers/slapd/auditlog.c @@ -154,7 +154,7 @@ write_audit_file( addlenstr( l, attr_changetype ); addlenstr( l, ": modify\n" ); mods = change; - for ( j = 0; mods[j] != NULL; j++ ) + for ( j = 0; (mods != NULL) && (mods[j] != NULL); j++ ) { int operationtype= mods[j]->mod_op & ~LDAP_MOD_BVALUES; diff --git a/ldap/servers/slapd/back-ldbm/ldbm_attrcrypt_config.c b/ldap/servers/slapd/back-ldbm/ldbm_attrcrypt_config.c index 0b7deaa..86479c9 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_attrcrypt_config.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_attrcrypt_config.c @@ -287,7 +287,7 @@ ldbm_instance_attrcrypt_config_modify_callback(Slapi_PBlock *pb, Slapi_Entry *e, return SLAPI_DSE_CALLBACK_ERROR; } - for (i = 0; mods[i] != NULL; i++) { + for (i = 0; (mods != NULL) && (mods[i] != NULL); i++) { char *config_attr = (char *)mods[i]->mod_type; diff --git a/ldap/servers/slapd/back-ldbm/ldbm_config.c b/ldap/servers/slapd/back-ldbm/ldbm_config.c index ef5cdce..0f8cc76 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_config.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_config.c @@ -2124,7 +2124,7 @@ int ldbm_config_modify_entry_callback(Slapi_PBlock *pb, Slapi_Entry* entryBefore * 2nd pass: set apply mods to 1 to apply changes to internal storage */ for ( apply_mod = 0; apply_mod <= 1 && LDAP_SUCCESS == rc; apply_mod++ ) { - for (i = 0; mods[i] && LDAP_SUCCESS == rc; i++) { + for (i = 0; mods && mods[i] && LDAP_SUCCESS == rc; i++) { attr_name = mods[i]->mod_type; /* There are some attributes that we don't care about, like modifiersname. */ diff --git a/ldap/servers/slapd/back-ldbm/ldbm_instance_config.c b/ldap/servers/slapd/back-ldbm/ldbm_instance_config.c index 565e9ad..9b93f9a 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_instance_config.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_instance_config.c @@ -781,7 +781,7 @@ ldbm_instance_modify_config_entry_callback(Slapi_PBlock *pb, Slapi_Entry* entryB * 2nd pass: set apply mods to 1 to apply changes to internal storage */ for ( apply_mod = 0; apply_mod <= 1 && LDAP_SUCCESS == rc; apply_mod++ ) { - for (i = 0; mods[i] && LDAP_SUCCESS == rc; i++) { + for (i = 0; mods && mods[i] && LDAP_SUCCESS == rc; i++) { attr_name = mods[i]->mod_type; if (strcasecmp(attr_name, CONFIG_INSTANCE_SUFFIX) == 0) { diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modify.c b/ldap/servers/slapd/back-ldbm/ldbm_modify.c index 92692f0..8bbeae0 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modify.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_modify.c @@ -253,7 +253,7 @@ modify_apply_check_expand( * If the objectClass attribute type was modified in any way, expand * the objectClass values to reflect the inheritance hierarchy. */ - for ( i = 0; mods[i] != NULL && !repl_op; ++i ) { + for ( i = 0; (mods != NULL) && (mods[i] != NULL) && !repl_op; ++i ) { if ( 0 == strcasecmp( SLAPI_ATTR_OBJECTCLASS, mods[i]->mod_type )) { slapi_schema_expand_objectclasses( ec->ep_entry ); break; @@ -938,7 +938,7 @@ remove_illegal_mods(LDAPMod **mods) LDAPMod *tmp; /* remove any attempts by the user to modify these attrs */ - for ( i = 0; mods[i] != NULL; i++ ) { + for ( i = 0; (mods != NULL) && (mods[i] != NULL); i++ ) { if ( strcasecmp( mods[i]->mod_type, numsubordinates ) == 0 || strcasecmp( mods[i]->mod_type, hassubordinates ) == 0 ) { diff --git a/ldap/servers/slapd/back-ldif/modify.c b/ldap/servers/slapd/back-ldif/modify.c index 7fff067..9a92b17 100644 --- a/ldap/servers/slapd/back-ldif/modify.c +++ b/ldap/servers/slapd/back-ldif/modify.c @@ -560,7 +560,7 @@ apply_mods( Slapi_Entry *e, LDAPMod **mods ) LDAPDebug( LDAP_DEBUG_TRACE, "=> apply_mods\n", 0, 0, 0 ); err = LDAP_SUCCESS; - for ( j = 0; mods[j] != NULL; j++ ) { + for ( j = 0; (mods != NULL) && (mods[j] != NULL); j++ ) { switch ( mods[j]->mod_op & ~LDAP_MOD_BVALUES ) { case LDAP_MOD_ADD: LDAPDebug( LDAP_DEBUG_ARGS, " add: %s\n", diff --git a/ldap/servers/slapd/configdse.c b/ldap/servers/slapd/configdse.c index 2cea73a..339be42 100644 --- a/ldap/servers/slapd/configdse.c +++ b/ldap/servers/slapd/configdse.c @@ -394,7 +394,7 @@ modify_config_dse(Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_Entry* e, in */ for ( apply_mods = 0; apply_mods <= 1; apply_mods++ ) { int i = 0; - for (i = 0; (mods[i] && (LDAP_SUCCESS == rc)); i++) { + for (i = 0; mods && (mods[i] && (LDAP_SUCCESS == rc)); i++) { /* send all aci modifications to the backend */ config_attr = (char *)mods[i]->mod_type; if (ignore_attr_type(config_attr)) @@ -497,7 +497,7 @@ postop_modify_config_dse(Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_Entry slapi_pblock_get( pb, SLAPI_MODIFY_MODS, &mods ); returntext[0] = '\0'; - for (i = 0; mods[i]; i++) { + for (i = 0; mods && mods[i]; i++) { if (mods[i]->mod_op & LDAP_MOD_REPLACE ) { /* Check if the server needs to be restarted */ for (j = 0; j < num_requires_restart; j++) diff --git a/ldap/servers/slapd/mapping_tree.c b/ldap/servers/slapd/mapping_tree.c index d01b285..338fffe 100644 --- a/ldap/servers/slapd/mapping_tree.c +++ b/ldap/servers/slapd/mapping_tree.c @@ -1096,7 +1096,7 @@ int mapping_tree_entry_modify_callback(Slapi_PBlock *pb, Slapi_Entry* entryBefor return SLAPI_DSE_CALLBACK_ERROR; } - for (i = 0; mods[i] != NULL; i++) { + for (i = 0; (mods != NULL) && (mods[i] != NULL); i++) { if ( (strcasecmp(mods[i]->mod_type, "cn") == 0) || (strcasecmp(mods[i]->mod_type, MAPPING_TREE_PARENT_ATTRIBUTE) == 0) ) diff --git a/ldap/servers/slapd/modify.c b/ldap/servers/slapd/modify.c index 79e7c7c..7763700 100644 --- a/ldap/servers/slapd/modify.c +++ b/ldap/servers/slapd/modify.c @@ -402,14 +402,17 @@ do_modify( Slapi_PBlock *pb ) mods = slapi_mods_get_ldapmods_passout (&smods); /* normalize the mods */ - normalized_mods = normalize_mods2bvals((const LDAPMod**)mods); - ldap_mods_free (mods, 1 /* Free the Array and the Elements */); - if (normalized_mods == NULL) { - op_shared_log_error_access(pb, "MOD", rawdn?rawdn:"", - "mod includes invalid dn format"); - send_ldap_result(pb, LDAP_INVALID_DN_SYNTAX, NULL, - "mod includes invalid dn format", 0, NULL); - goto free_and_return; + if (mods) { + normalized_mods = normalize_mods2bvals((const LDAPMod**)mods); + ldap_mods_free (mods, 1 /* Free the Array and the Elements */); + if (normalized_mods == NULL) { + /* NOTE: normalize_mods2bvals only handles DN syntax currently */ + op_shared_log_error_access(pb, "MOD", rawdn?rawdn:"", + "mod includes invalid dn format"); + send_ldap_result(pb, LDAP_INVALID_DN_SYNTAX, NULL, + "mod includes invalid dn format", 0, NULL); + goto free_and_return; + } } slapi_pblock_set(pb, SLAPI_MODIFY_MODS, normalized_mods); @@ -1445,7 +1448,7 @@ hash_rootpw (LDAPMod **mods) return 0; } - for (i=0; mods[i] != NULL; i++) { + for (i=0; (mods != NULL) && (mods[i] != NULL); i++) { LDAPMod *mod = mods[i]; if (strcasecmp (mod->mod_type, CONFIG_ROOTPW_ATTRIBUTE) != 0) continue; diff --git a/ldap/servers/slapd/schema.c b/ldap/servers/slapd/schema.c index 24febad..d6aae58 100644 --- a/ldap/servers/slapd/schema.c +++ b/ldap/servers/slapd/schema.c @@ -2082,7 +2082,7 @@ modify_schema_dse (Slapi_PBlock *pb, Slapi_Entry *entryBefore, Slapi_Entry *entr * True for DS 4.x as well, although it tried to keep going even after * an error was detected (which was very wrong). */ - for (i = 0; rc == SLAPI_DSE_CALLBACK_OK && mods[i]; i++) { + for (i = 0; rc == SLAPI_DSE_CALLBACK_OK && mods && mods[i]; i++) { schema_dse_attr_name = (char *) mods[i]->mod_type; num_mods++; /* incr the number of mods */ diff --git a/ldap/servers/slapd/task.c b/ldap/servers/slapd/task.c index 5e03bc2..6340db8 100644 --- a/ldap/servers/slapd/task.c +++ b/ldap/servers/slapd/task.c @@ -792,7 +792,7 @@ static int task_modify(Slapi_PBlock *pb, Slapi_Entry *e, /* ignore eAfter, just scan the mods for anything unacceptable */ slapi_pblock_get(pb, SLAPI_MODIFY_MODS, &mods); - for (i = 0; mods[i] != NULL; i++) { + for (i = 0; (mods != NULL) && (mods[i] != NULL); i++) { /* for some reason, "modifiersName" and "modifyTimestamp" are * stuck in by the server */ if ((strcasecmp(mods[i]->mod_type, "ttl") != 0) && diff --git a/ldap/servers/slapd/test-plugins/testpostop.c b/ldap/servers/slapd/test-plugins/testpostop.c index f18f4ab..d91ddd4 100644 --- a/ldap/servers/slapd/test-plugins/testpostop.c +++ b/ldap/servers/slapd/test-plugins/testpostop.c @@ -355,7 +355,7 @@ write_changelog( that has been added, replaced, or deleted. */ fprintf( fp, "changetype: modify\n" ); mods = (LDAPMod **)change; - for ( j = 0; mods[j] != NULL; j++ ) { + for ( j = 0; (mods != NULL) && (mods[j] != NULL); j++ ) { switch ( mods[j]->mod_op & ~LDAP_MOD_BVALUES ) { case LDAP_MOD_ADD: fprintf( fp, "add: %s\n", mods[j]->mod_type );