From bef2815bb672d498d233777f5a1e44b10d9f967c Mon Sep 17 00:00:00 2001
From: Rich Megginson <rmeggins@redhat.com>
Date: Wed, 9 Apr 2014 13:43:37 -0600
Subject: [PATCH] Ticket #47772 empty modify returns LDAP_INVALID_DN_SYNTAX

https://fedorahosted.org/389/ticket/47772
Reviewed by: ???
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        |    4 +-
 ldap/servers/plugins/chainingdb/cb_instance.c      |    4 +-
 ldap/servers/plugins/chainingdb/cb_modify.c        |    2 +-
 ldap/servers/plugins/replication/cl5_config.c      |    2 +-
 ldap/servers/plugins/replication/legacy_consumer.c |    2 +-
 ldap/servers/plugins/replication/repl5_agmt.c      |    2 +-
 .../plugins/replication/repl5_replica_config.c     |    4 +-
 ldap/servers/plugins/uiduniq/7bit.c                |    2 +-
 ldap/servers/plugins/uiduniq/uid.c                 |    2 +-
 ldap/servers/slapd/auditlog.c                      |    2 +-
 .../slapd/back-ldbm/ldbm_attrcrypt_config.c        |    2 +-
 ldap/servers/slapd/back-ldbm/ldbm_config.c         |    2 +-
 .../servers/slapd/back-ldbm/ldbm_instance_config.c |    2 +-
 ldap/servers/slapd/back-ldbm/ldbm_modify.c         |    4 +-
 ldap/servers/slapd/back-ldif/modify.c              |    2 +-
 ldap/servers/slapd/configdse.c                     |    4 +-
 ldap/servers/slapd/mapping_tree.c                  |    2 +-
 ldap/servers/slapd/modify.c                        |   21 +++++++++++--------
 ldap/servers/slapd/schema.c                        |    2 +-
 ldap/servers/slapd/task.c                          |    2 +-
 ldap/servers/slapd/test-plugins/testpostop.c       |    2 +-
 21 files changed, 37 insertions(+), 34 deletions(-)

diff --git a/ldap/servers/plugins/chainingdb/cb_config.c b/ldap/servers/plugins/chainingdb/cb_config.c
index 717ea3c..5aec8ce 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 95781b5..5f594d6 100644
--- a/ldap/servers/plugins/chainingdb/cb_instance.c
+++ b/ldap/servers/plugins/chainingdb/cb_instance.c
@@ -305,7 +305,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 */
@@ -378,7 +378,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 980cb7f..900cfc0 100644
--- a/ldap/servers/plugins/replication/cl5_config.c
+++ b/ldap/servers/plugins/replication/cl5_config.c
@@ -340,7 +340,7 @@ changelog5_config_modify (Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_Entr
 	config.maxAge = slapi_ch_strdup(CL5_STR_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 2440ce7..c80c166 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 935fb37..9cb8db8 100644
--- a/ldap/servers/plugins/replication/repl5_agmt.c
+++ b/ldap/servers/plugins/replication/repl5_agmt.c
@@ -1833,7 +1833,7 @@ agmt_notify_change(Repl_Agmt *agmt, Slapi_PBlock *pb)
 					int i, j;
 
 					slapi_pblock_get(pb, SLAPI_MODIFY_MODS, &mods);
-					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 9a0b190..4149d6f 100644
--- a/ldap/servers/plugins/replication/repl5_replica_config.c
+++ b/ldap/servers/plugins/replication/repl5_replica_config.c
@@ -349,7 +349,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;
@@ -598,7 +598,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 fbcc530..3d6f12f 100644
--- a/ldap/servers/plugins/uiduniq/7bit.c
+++ b/ldap/servers/plugins/uiduniq/7bit.c
@@ -455,7 +455,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 b713f73..b35004a 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_config.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_config.c
@@ -2006,7 +2006,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 f41bd88..2037618 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_instance_config.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_instance_config.c
@@ -767,7 +767,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 32510ab..b8b6114 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;
@@ -854,7 +854,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 12175c4..9265b98 100644
--- a/ldap/servers/slapd/configdse.c
+++ b/ldap/servers/slapd/configdse.c
@@ -392,7 +392,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))
@@ -487,7 +487,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 19c5708..654cba0 100644
--- a/ldap/servers/slapd/mapping_tree.c
+++ b/ldap/servers/slapd/mapping_tree.c
@@ -1074,7 +1074,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 5e52f26..239cfcc 100644
--- a/ldap/servers/slapd/modify.c
+++ b/ldap/servers/slapd/modify.c
@@ -390,14 +390,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);
 
@@ -1275,7 +1278,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 258f6eb..f55d1f5 100644
--- a/ldap/servers/slapd/schema.c
+++ b/ldap/servers/slapd/schema.c
@@ -1676,7 +1676,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 5fc0b08..2fe1de7 100644
--- a/ldap/servers/slapd/task.c
+++ b/ldap/servers/slapd/task.c
@@ -774,7 +774,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 );
-- 
1.7.1

