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

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 )) {

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 */

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++ ) {

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)
 		{

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)
 		{

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++)
 						{

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;

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 */

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 */

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;
 

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;
 

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. */

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) {

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 )
 		{

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",

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++)

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) )

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;

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 */
 

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) &&

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 );