Summary: Avoid adding a group as a memberOf itself.

Resolves: 439450

1 files changed, 23 insertions, 10 deletions

diff --git a/ldap/servers/plugins/memberof/memberof.c b/ldap/servers/plugins/memberof/memberof.c
index 5c69fda5..a1341a7e 100644
--- a/ldap/servers/plugins/memberof/memberof.c
+++ b/ldap/servers/plugins/memberof/memberof.c
@@ -946,6 +946,27 @@ int memberof_modop_one_replace_r(Slapi_PBlock *pb, int mod_op, char *group_dn,
 	}
 	/* continue with operation */
 	{
+		Slapi_Value *to_dn_val = slapi_value_new_string(op_to);
+		Slapi_Value *this_dn_val = slapi_value_new_string(op_this);
+
+		/* We want to avoid listing a group as a memberOf itself
+		 * in case someone set up a circular grouping.
+		 */
+		if (0 == memberof_compare(&this_dn_val, &to_dn_val))
+		{
+			slapi_log_error( SLAPI_LOG_PLUGIN,
+				MEMBEROF_PLUGIN_SUBSYSTEM,
+				"memberof_modop_one_r: not processing memberOf "
+				"operations  on self entry: %s\n", this_dn_val);
+			slapi_value_free(&to_dn_val);
+			slapi_value_free(&this_dn_val);     
+			goto bail;
+		}
+
+		/* We don't need the Slapi_Value copies of the DN's anymore */
+		slapi_value_free(&to_dn_val);
+		slapi_value_free(&this_dn_val);
+
 		if(stack && LDAP_MOD_DELETE == mod_op)
 		{
 			if(memberof_is_legit_member(pb, group_dn, 
@@ -1012,20 +1033,12 @@ int memberof_modop_one_replace_r(Slapi_PBlock *pb, int mod_op, char *group_dn,
 
 		if(LDAP_MOD_ADD == mod_op)
 		{
-			Slapi_Value *to_dn_val = slapi_value_new_string(op_to);
-			Slapi_Value *this_dn_val = slapi_value_new_string(op_this);
-
 			/* If we failed to update memberOf for op_to, we shouldn't
-			 * try to fix up membership for parent groups.  We also want
-			 * to avoid going into an endless loop if we've hit a
-			 * circular grouping. */
-			if ((rc == 0) && (0 != memberof_compare(&this_dn_val, &to_dn_val))) {
+			 * try to fix up membership for parent groups. */
+			if (rc == 0) {
 				/* fix up membership for groups that are now in scope */
 				memberof_add_membership(pb, op_this, op_to);
 			}
-
-			slapi_value_free(&to_dn_val);
-			slapi_value_free(&this_dn_val);
 		}
 	}