Issue 49122 - Filtered nsrole that uses nsrole crashes the

 server

Bug Description:  When evaluating a filter role that uses "nsrole" in the filter
                  crashes the server due infinite loop that leads to a stack
                  overflow.

Fix Description:  Virtual attributes are not allowed to be used in role filters.
                  We were already checking for COS attributes, but not nsrole.

                  Also did some minor code cleanup

https://pagure.io/389-ds-base/issue/49122

Reviewed by: nhosoi(Thanks!)

dirsrvtests/tests/tickets/ticket49122_test.py

@@ -0,0 +1,73 @@
+import time
+import ldap
+import logging
+import pytest
+from lib389 import DirSrv, Entry, tools, tasks
+from lib389.tools import DirSrvTools
+from lib389._constants import *
+from lib389.properties import *
+from lib389.tasks import *
+from lib389.utils import *
+from lib389.topologies import topology_st as topo
+
+DEBUGGING = os.getenv("DEBUGGING", default=False)
+if DEBUGGING:
+    logging.getLogger(__name__).setLevel(logging.DEBUG)
+else:
+    logging.getLogger(__name__).setLevel(logging.INFO)
+log = logging.getLogger(__name__)
+
+USER_DN = 'uid=user,' + DEFAULT_SUFFIX
+ROLE_DN = 'cn=Filtered_Role_That_Includes_Empty_Role,' + DEFAULT_SUFFIX
+
+
+def test_ticket49122(topo):
+    """Search for non-existant role and make sure the server does not crash
+    """
+
+    # Enable roles plugin
+    topo.standalone.plugins.enable(name=PLUGIN_ROLES)
+    topo.standalone.restart()
+
+    # Add invalid role
+    try:
+        topo.standalone.add_s(Entry((
+            ROLE_DN, {'objectclass': ['top', 'ldapsubentry', 'nsroledefinition',
+                                      'nscomplexroledefinition', 'nsfilteredroledefinition'],
+                      'cn': 'Filtered_Role_That_Includes_Empty_Role',
+                      'nsRoleFilter': '(!(nsrole=cn=This_Is_An_Empty_Managed_NsRoleDefinition,dc=example,dc=com))',
+                      'description': 'A filtered role with filter that will crash the server'})))
+    except ldap.LDAPError as e:
+        topo.standalone.log.fatal('Failed to add filtered role: error ' + e.message['desc'])
+        assert False
+
+    # Add test user
+    try:
+        topo.standalone.add_s(Entry((
+            USER_DN, {'objectclass': "top extensibleObject".split(),
+                      'uid': 'user'})))
+    except ldap.LDAPError as e:
+        topo.standalone.log.fatal('Failed to add test user: error ' + str(e))
+        assert False
+
+    if DEBUGGING:
+        # Add debugging steps(if any)...
+        print "Attach gdb"
+        time.sleep(20)
+
+    # Search for the role
+    try:
+        topo.standalone.search_s(USER_DN, ldap.SCOPE_SUBTREE, 'objectclass=*', ['nsrole'])
+    except ldap.LDAPError as e:
+        topo.standalone.log.fatal('Search failed: error ' + str(e))
+        assert False
+
+    topo.standalone.log.info('Test Passed')
+
+
+if __name__ == '__main__':
+    # Run isolated
+    # -s for DEBUG mode
+    CURRENT_FILE = os.path.realpath(__file__)
+    pytest.main("-s %s" % CURRENT_FILE)
+

ldap/servers/plugins/roles/roles_cache.c

@@ -1072,6 +1072,26 @@ static int roles_cache_create_role_under(roles_cache_def** roles_cache_suffix, S
 	return(rc);
 }
 
+/*
+ * Check that we are not using nsrole in the filter
+ */
+static int roles_check_filter(Slapi_Filter *filter_list)
+{
+	Slapi_Filter  *f;
+	char *type = NULL;
+
+	for ( f = slapi_filter_list_first( filter_list );
+	          f != NULL;
+	          f = slapi_filter_list_next( filter_list, f ) )
+	{
+		slapi_filter_get_attribute_type(f, &type);
+		if (strcasecmp(type, NSROLEATTR) == 0){
+			return -1;
+		}
+	}
+
+	return 0;
+}
 
 /* roles_cache_create_object_from_entry
    ------------------------------------
@@ -1087,17 +1107,17 @@ static int roles_cache_create_object_from_entry(Slapi_Entry *role_entry, role_ob
 	int rc = 0;
 	int type = 0;
 	role_object *this_role = NULL;
-        char *rolescopeDN = NULL;
+	char *rolescopeDN = NULL;
 
 	slapi_log_err(SLAPI_LOG_PLUGIN, ROLES_PLUGIN_SUBSYSTEM, 
 					"--> roles_cache_create_object_from_entry\n");
 
-    *result = NULL;
+	*result = NULL;
 
-    /* Do not allow circular dependencies */
-    if ( hint > MAX_NESTED_ROLES ) 
+	/* Do not allow circular dependencies */
+	if ( hint > MAX_NESTED_ROLES ) 
 	{
-        char *ndn = NULL;
+		char *ndn = NULL;
 
 		ndn = slapi_entry_get_ndn( role_entry );
 		slapi_log_err(SLAPI_LOG_ERR, ROLES_PLUGIN_SUBSYSTEM,
@@ -1108,85 +1128,83 @@ static int roles_cache_create_object_from_entry(Slapi_Entry *role_entry, role_ob
 		return (0);
 	}
 
-    /* Create the role cache definition */
-    this_role = (role_object*)slapi_ch_calloc(1, sizeof(role_object));
-    if (this_role == NULL ) 
+	/* Create the role cache definition */
+	this_role = (role_object*)slapi_ch_calloc(1, sizeof(role_object));
+	if (this_role == NULL ) 
 	{
-        return ENOMEM;
-    }
+		return ENOMEM;
+	}
 
-    /* Check the entry is OK */
-    /* Determine role type and assign to structure */
-    /* We determine the role type by reading the objectclass */
+	/* Check the entry is OK */
+	/* Determine role type and assign to structure */
+	/* We determine the role type by reading the objectclass */
 	if ( roles_cache_is_role_entry(role_entry) == 0 )
 	{
-        /* Bad type */
-        slapi_ch_free((void**)&this_role);
-        return SLAPI_ROLE_DEFINITION_ERROR;
-    }
+		/* Bad type */
+		slapi_ch_free((void**)&this_role);
+		return SLAPI_ROLE_DEFINITION_ERROR;
+	}
 
-    type = roles_cache_determine_class(role_entry);
+	type = roles_cache_determine_class(role_entry);
 
-    if (type != 0) 
+	if (type != 0) 
 	{
-        this_role->type = type;
-    }
+		this_role->type = type;
+	}
 	else
 	{
-        /* Bad type */
-        slapi_ch_free((void**)&this_role);
-        return SLAPI_ROLE_DEFINITION_ERROR;
-    }
+		/* Bad type */
+		slapi_ch_free((void**)&this_role);
+		return SLAPI_ROLE_DEFINITION_ERROR;
+	}
 
 	this_role->dn = slapi_sdn_new();
 	slapi_sdn_copy(slapi_entry_get_sdn(role_entry),this_role->dn);
-        
-        rolescopeDN = slapi_entry_attr_get_charptr(role_entry, ROLE_SCOPE_DN);
-        if (rolescopeDN) {
-                Slapi_DN *rolescopeSDN;
-                Slapi_DN *top_rolescopeSDN, *top_this_roleSDN;
-
-                /* Before accepting to use this scope, first check if it belongs to the same suffix */
-                rolescopeSDN = slapi_sdn_new_dn_byref(rolescopeDN);
-                if ((strlen((char *) slapi_sdn_get_ndn(rolescopeSDN)) > 0) && 
-                        (slapi_dn_syntax_check(NULL, (char *) slapi_sdn_get_ndn(rolescopeSDN), 1) == 0)) {
-                        top_rolescopeSDN = roles_cache_get_top_suffix(rolescopeSDN);
-                        top_this_roleSDN = roles_cache_get_top_suffix(this_role->dn);
-                        if (slapi_sdn_compare(top_rolescopeSDN, top_this_roleSDN) == 0) {
-                                /* rolescopeDN belongs to the same suffix as the role, we can use this scope */
-                                this_role->rolescopedn = rolescopeSDN;
-                        } else {
-                                slapi_log_err(SLAPI_LOG_ERR, ROLES_PLUGIN_SUBSYSTEM,
-                                        "roles_cache_create_object_from_entry - %s: invalid %s - %s not in the same suffix. Scope skipped.\n",
-                                        (char*) slapi_sdn_get_dn(this_role->dn),
-                                        ROLE_SCOPE_DN,
-                                        rolescopeDN);
-                                slapi_sdn_free(&rolescopeSDN);
-                        }
-                        slapi_sdn_free(&top_rolescopeSDN);
-                        slapi_sdn_free(&top_this_roleSDN);
-                } else {
-                        /* this is an invalid DN, just ignore this parameter*/
-                        slapi_log_err(SLAPI_LOG_ERR, ROLES_PLUGIN_SUBSYSTEM,
-                                "roles_cache_create_object_from_entry - %s: invalid %s - %s not a valid DN. Scope skipped.\n",
-                                (char*) slapi_sdn_get_dn(this_role->dn),
-                                ROLE_SCOPE_DN,
-                                rolescopeDN);
-                        slapi_sdn_free(&rolescopeSDN);
-                }
-        }
+		
+	rolescopeDN = slapi_entry_attr_get_charptr(role_entry, ROLE_SCOPE_DN);
+	if (rolescopeDN) {
+		Slapi_DN *rolescopeSDN;
+		Slapi_DN *top_rolescopeSDN, *top_this_roleSDN;
+
+		/* Before accepting to use this scope, first check if it belongs to the same suffix */
+		rolescopeSDN = slapi_sdn_new_dn_byref(rolescopeDN);
+		if ((strlen((char *) slapi_sdn_get_ndn(rolescopeSDN)) > 0) &&
+			(slapi_dn_syntax_check(NULL, (char *) slapi_sdn_get_ndn(rolescopeSDN), 1) == 0)) {
+			top_rolescopeSDN = roles_cache_get_top_suffix(rolescopeSDN);
+			top_this_roleSDN = roles_cache_get_top_suffix(this_role->dn);
+			if (slapi_sdn_compare(top_rolescopeSDN, top_this_roleSDN) == 0) {
+				/* rolescopeDN belongs to the same suffix as the role, we can use this scope */
+				this_role->rolescopedn = rolescopeSDN;
+			} else {
+				slapi_log_err(SLAPI_LOG_ERR, ROLES_PLUGIN_SUBSYSTEM,
+					"roles_cache_create_object_from_entry - %s: invalid %s - %s not in the same suffix. Scope skipped.\n",
+					(char*) slapi_sdn_get_dn(this_role->dn),
+					ROLE_SCOPE_DN,
+					rolescopeDN);
+				slapi_sdn_free(&rolescopeSDN);
+			}
+			slapi_sdn_free(&top_rolescopeSDN);
+			slapi_sdn_free(&top_this_roleSDN);
+		} else {
+			/* this is an invalid DN, just ignore this parameter*/
+			slapi_log_err(SLAPI_LOG_ERR, ROLES_PLUGIN_SUBSYSTEM,
+				"roles_cache_create_object_from_entry - %s: invalid %s - %s not a valid DN. Scope skipped.\n",
+				(char*) slapi_sdn_get_dn(this_role->dn),
+				ROLE_SCOPE_DN,
+				rolescopeDN);
+			slapi_sdn_free(&rolescopeSDN);
+		}
+	}
 
-    /* Depending upon role type, pull out the remaining information we need */
+	/* Depending upon role type, pull out the remaining information we need */
 	switch (this_role->type)
 	{
 		case ROLE_TYPE_MANAGED:
-
 			/* Nothing further needed */
 			break;
 
 		case ROLE_TYPE_FILTERED:
 		{
-
 			Slapi_Filter *filter = NULL;
 			char *filter_attr_value = NULL;
 			Slapi_PBlock *pb = NULL;
@@ -1200,6 +1218,7 @@ static int roles_cache_create_object_from_entry(Slapi_Entry *role_entry, role_ob
 				slapi_ch_free((void**)&this_role);
 				return SLAPI_ROLE_ERROR_NO_FILTER_SPECIFIED;
 			}
+
 			/* search (&(objectclass=costemplate)(filter_attr_value))*/
 			/* if found, reject it (returning SLAPI_ROLE_ERROR_FILTER_BAD) */
 			pb = slapi_pblock_new();
@@ -1208,33 +1227,33 @@ static int roles_cache_create_object_from_entry(Slapi_Entry *role_entry, role_ob
 				Slapi_Entry **cosentries = NULL;
 				char *costmpl_filter = NULL;
 				if ((*filter_attr_value == '(') &&
-				    (*(filter_attr_value+strlen(filter_attr_value)-1) == ')')) {
+					(*(filter_attr_value+strlen(filter_attr_value)-1) == ')')) {
 					costmpl_filter =
-					      slapi_ch_smprintf("(&(objectclass=costemplate)%s)", 
-					                        filter_attr_value);
+						  slapi_ch_smprintf("(&(objectclass=costemplate)%s)", 
+											filter_attr_value);
 				} else {
 					costmpl_filter =
-					      slapi_ch_smprintf("(&(objectclass=costemplate)(%s))", 
-					                        filter_attr_value);
+						  slapi_ch_smprintf("(&(objectclass=costemplate)(%s))", 
+											filter_attr_value);
 				}
 				slapi_search_internal_set_pb(pb, parent, LDAP_SCOPE_SUBTREE,
-				                             costmpl_filter, NULL, 0, NULL, 
-				                             NULL, roles_get_plugin_identity(),
-				                             0);
+											 costmpl_filter, NULL, 0, NULL, 
+											 NULL, roles_get_plugin_identity(),
+											 0);
 				slapi_search_internal_pb(pb);
 				slapi_pblock_get(pb, SLAPI_PLUGIN_INTOP_SEARCH_ENTRIES, 
-				                 &cosentries);
+								 &cosentries);
 				slapi_ch_free_string(&costmpl_filter);
 				slapi_ch_free_string(&parent);
 				if (cosentries && *cosentries) {
 					slapi_free_search_results_internal(pb);
 					slapi_pblock_destroy(pb);
 					slapi_log_err(SLAPI_LOG_ERR, ROLES_PLUGIN_SUBSYSTEM,
-					    "roles_cache_create_object_from_entry - %s: not allowed to refer virtual attribute "
-					    "in the value of %s %s. The %s is disabled.\n",
-					    (char*)slapi_sdn_get_ndn(this_role->dn),
-					    ROLE_FILTER_ATTR_NAME, filter_attr_value,
-					    ROLE_FILTER_ATTR_NAME);
+						"roles_cache_create_object_from_entry - %s: not allowed to refer virtual attribute "
+						"in the value of %s %s. The %s is disabled.\n",
+						(char*)slapi_sdn_get_ndn(this_role->dn),
+						ROLE_FILTER_ATTR_NAME, filter_attr_value,
+						ROLE_FILTER_ATTR_NAME);
 					slapi_ch_free_string(&filter_attr_value);
 					slapi_ch_free((void**)&this_role);
 					return SLAPI_ROLE_ERROR_FILTER_BAD;
@@ -1245,16 +1264,27 @@ static int roles_cache_create_object_from_entry(Slapi_Entry *role_entry, role_ob
 
 			/* Turn it into a slapi filter object */
 			filter = slapi_str2filter(filter_attr_value);
-			slapi_ch_free_string(&filter_attr_value);
-
-			if ( filter == NULL ) 
+			if ( filter == NULL )
 			{
 				/* An error has occured */
 				slapi_ch_free((void**)&this_role);
+				slapi_ch_free_string(&filter_attr_value);
+				return SLAPI_ROLE_ERROR_FILTER_BAD;
+			}
+			if (roles_check_filter(filter)) {
+				slapi_log_err(SLAPI_LOG_ERR, ROLES_PLUGIN_SUBSYSTEM,
+					"roles_cache_create_object_from_entry - \"%s\": not allowed to use \"nsrole\" "
+					"in the role filter \"%s\".  %s is disabled.\n",
+					(char*)slapi_sdn_get_ndn(this_role->dn),
+					filter_attr_value,
+					ROLE_FILTER_ATTR_NAME);
+				slapi_ch_free((void**)&this_role);
+				slapi_ch_free_string(&filter_attr_value);
 				return SLAPI_ROLE_ERROR_FILTER_BAD;
 			}
 			/* Store on the object */
 			this_role->filter = filter;
+			slapi_ch_free_string(&filter_attr_value);
 
 			break;
 		}
@@ -1273,50 +1303,49 @@ static int roles_cache_create_object_from_entry(Slapi_Entry *role_entry, role_ob
 				int i = 0;
 				char *string = NULL;
 				Slapi_DN nested_role_dn;
-                role_object_nested *nested_role_object = NULL;
+				role_object_nested *nested_role_object = NULL;
  
-                for ( i = 0; va[i] != NULL; i++ ) 
+				for ( i = 0; va[i] != NULL; i++ ) 
 				{
-                    string = (char*)slapi_value_get_string(va[i]);
+					string = (char*)slapi_value_get_string(va[i]);
 
-                    /* Make a DN from the string */
-                    slapi_sdn_init_dn_byref(&nested_role_dn,string);
+					/* Make a DN from the string */
+					slapi_sdn_init_dn_byref(&nested_role_dn,string);
 
 					slapi_log_err(SLAPI_LOG_PLUGIN, 
 									ROLES_PLUGIN_SUBSYSTEM, "roles_cache_create_object_from_entry - dn %s, nested %s\n",
 									(char*)slapi_sdn_get_ndn(this_role->dn),string);
 
-                    /* Make a role object nested from the DN */
-                    rc = roles_cache_object_nested_from_dn(&nested_role_dn,&nested_role_object);
+					/* Make a role object nested from the DN */
+					rc = roles_cache_object_nested_from_dn(&nested_role_dn,&nested_role_object);
 
-                    /* Insert it into the nested list */
-                    if ( (rc == 0) && nested_role_object) 
+					/* Insert it into the nested list */
+					if ( (rc == 0) && nested_role_object) 
 					{
 						/* Add to the tree where avl_data is a role_object_nested struct */
-                        rc = roles_cache_insert_object_nested(&(this_role->avl_tree),nested_role_object);
-                    }
-                    slapi_sdn_done(&nested_role_dn);
-                }    
-            }
-       
+						rc = roles_cache_insert_object_nested(&(this_role->avl_tree),nested_role_object);
+					}
+					slapi_sdn_done(&nested_role_dn);
+				}    
+			}
+	   
 			break;
 		}
 
 		default:
 			slapi_log_err(SLAPI_LOG_ERR, ROLES_PLUGIN_SUBSYSTEM,
-				"roles_cache_create_object_from_entry - wrong role type\n");
+					"roles_cache_create_object_from_entry - wrong role type\n");
 	}
 
-    if ( rc == 0 ) 
+	if ( rc == 0 ) 
 	{
-        *result = this_role;
-    }
+		*result = this_role;
+	}
 
 	slapi_log_err(SLAPI_LOG_PLUGIN, ROLES_PLUGIN_SUBSYSTEM, 
-					"<-- roles_cache_create_object_from_entry\n");
-
+			"<-- roles_cache_create_object_from_entry\n");
 
-    return rc;
+	return rc;
 }
 
 /* roles_cache_determine_class: