From c8c9d8814bd328d9772b6a248aa142b72430cba1 Mon Sep 17 00:00:00 2001 From: Viktor Ashirov Date: Wed, 16 Jul 2025 11:22:30 +0200 Subject: [PATCH] Issue 6778 - Memory leak in roles_cache_create_object_from_entry part 2 Bug Description: Everytime a role with scope DN is processed, we leak rolescopeDN. Fix Description: * Initialize all pointer variables to NULL * Add additional NULL checks * Free rolescopeDN * Move test_rewriter_with_invalid_filter before the DB contains 90k entries * Use task.wait() for import task completion instead of parsing logs, increase the timeout Fixes: https://github.com/389ds/389-ds-base/issues/6778 Reviewed by: @progier389 (Thanks!) --- dirsrvtests/tests/suites/roles/basic_test.py | 164 +++++++++---------- ldap/servers/plugins/roles/roles_cache.c | 10 +- 2 files changed, 82 insertions(+), 92 deletions(-) diff --git a/dirsrvtests/tests/suites/roles/basic_test.py b/dirsrvtests/tests/suites/roles/basic_test.py index d92d6f0c3..ec208bae9 100644 --- a/dirsrvtests/tests/suites/roles/basic_test.py +++ b/dirsrvtests/tests/suites/roles/basic_test.py @@ -510,6 +510,76 @@ def test_vattr_on_managed_role(topo, request): request.addfinalizer(fin) +def test_rewriter_with_invalid_filter(topo, request): + """Test that server does not crash when having + invalid filter in filtered role + + :id: 5013b0b2-0af6-11f0-8684-482ae39447e5 + :setup: standalone server + :steps: + 1. Setup filtered role with good filter + 2. Setup nsrole rewriter + 3. Restart the server + 4. Search for entries + 5. Setup filtered role with bad filter + 6. Search for entries + :expectedresults: + 1. Operation should succeed + 2. Operation should succeed + 3. Operation should succeed + 4. Operation should succeed + 5. Operation should succeed + 6. Operation should succeed + """ + inst = topo.standalone + entries = [] + + def fin(): + inst.start() + for entry in entries: + entry.delete() + request.addfinalizer(fin) + + # Setup filtered role + roles = FilteredRoles(inst, f'ou=people,{DEFAULT_SUFFIX}') + filter_ko = '(&((objectClass=top)(objectClass=nsPerson))' + filter_ok = '(&(objectClass=top)(objectClass=nsPerson))' + role_properties = { + 'cn': 'TestFilteredRole', + 'nsRoleFilter': filter_ok, + 'description': 'Test good filter', + } + role = roles.create(properties=role_properties) + entries.append(role) + + # Setup nsrole rewriter + rewriters = Rewriters(inst) + rewriter_properties = { + "cn": "nsrole", + "nsslapd-libpath": 'libroles-plugin', + "nsslapd-filterrewriter": 'role_nsRole_filter_rewriter', + } + rewriter = rewriters.ensure_state(properties=rewriter_properties) + entries.append(rewriter) + + # Restart thge instance + inst.restart() + + # Search for entries + entries = inst.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, "(nsrole=%s)" % role.dn) + + # Set bad filter + role_properties = { + 'cn': 'TestFilteredRole', + 'nsRoleFilter': filter_ko, + 'description': 'Test bad filter', + } + role.ensure_state(properties=role_properties) + + # Search for entries + entries = inst.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, "(nsrole=%s)" % role.dn) + + def test_managed_and_filtered_role_rewrite(topo, request): """Test that filter components containing 'nsrole=xxx' are reworked if xxx is either a filtered role or a managed @@ -581,17 +651,11 @@ def test_managed_and_filtered_role_rewrite(topo, request): PARENT="ou=people,%s" % DEFAULT_SUFFIX dbgen_users(topo.standalone, 90000, import_ldif, DEFAULT_SUFFIX, entry_name=RDN, generic=True, parent=PARENT) - # online import + # Online import import_task = ImportTask(topo.standalone) import_task.import_suffix_from_ldif(ldiffile=import_ldif, suffix=DEFAULT_SUFFIX) - # Check for up to 200sec that the completion - for i in range(1, 20): - if len(topo.standalone.ds_error_log.match('.*import userRoot: Import complete. Processed 9000.*')) > 0: - break - time.sleep(10) - import_complete = topo.standalone.ds_error_log.match('.*import userRoot: Import complete. Processed 9000.*') - assert (len(import_complete) == 1) - + import_task.wait(timeout=400) + assert import_task.get_exit_code() == 0 # Restart server topo.standalone.restart() @@ -715,17 +779,11 @@ def test_not_such_entry_role_rewrite(topo, request): PARENT="ou=people,%s" % DEFAULT_SUFFIX dbgen_users(topo.standalone, 91000, import_ldif, DEFAULT_SUFFIX, entry_name=RDN, generic=True, parent=PARENT) - # online import + # Online import import_task = ImportTask(topo.standalone) import_task.import_suffix_from_ldif(ldiffile=import_ldif, suffix=DEFAULT_SUFFIX) - # Check for up to 200sec that the completion - for i in range(1, 20): - if len(topo.standalone.ds_error_log.match('.*import userRoot: Import complete. Processed 9100.*')) > 0: - break - time.sleep(10) - import_complete = topo.standalone.ds_error_log.match('.*import userRoot: Import complete. Processed 9100.*') - assert (len(import_complete) == 1) - + import_task.wait(timeout=400) + assert import_task.get_exit_code() == 0 # Restart server topo.standalone.restart() @@ -769,76 +827,6 @@ def test_not_such_entry_role_rewrite(topo, request): request.addfinalizer(fin) -def test_rewriter_with_invalid_filter(topo, request): - """Test that server does not crash when having - invalid filter in filtered role - - :id: 5013b0b2-0af6-11f0-8684-482ae39447e5 - :setup: standalone server - :steps: - 1. Setup filtered role with good filter - 2. Setup nsrole rewriter - 3. Restart the server - 4. Search for entries - 5. Setup filtered role with bad filter - 6. Search for entries - :expectedresults: - 1. Operation should succeed - 2. Operation should succeed - 3. Operation should succeed - 4. Operation should succeed - 5. Operation should succeed - 6. Operation should succeed - """ - inst = topo.standalone - entries = [] - - def fin(): - inst.start() - for entry in entries: - entry.delete() - request.addfinalizer(fin) - - # Setup filtered role - roles = FilteredRoles(inst, f'ou=people,{DEFAULT_SUFFIX}') - filter_ko = '(&((objectClass=top)(objectClass=nsPerson))' - filter_ok = '(&(objectClass=top)(objectClass=nsPerson))' - role_properties = { - 'cn': 'TestFilteredRole', - 'nsRoleFilter': filter_ok, - 'description': 'Test good filter', - } - role = roles.create(properties=role_properties) - entries.append(role) - - # Setup nsrole rewriter - rewriters = Rewriters(inst) - rewriter_properties = { - "cn": "nsrole", - "nsslapd-libpath": 'libroles-plugin', - "nsslapd-filterrewriter": 'role_nsRole_filter_rewriter', - } - rewriter = rewriters.ensure_state(properties=rewriter_properties) - entries.append(rewriter) - - # Restart thge instance - inst.restart() - - # Search for entries - entries = inst.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, "(nsrole=%s)" % role.dn) - - # Set bad filter - role_properties = { - 'cn': 'TestFilteredRole', - 'nsRoleFilter': filter_ko, - 'description': 'Test bad filter', - } - role.ensure_state(properties=role_properties) - - # Search for entries - entries = inst.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, "(nsrole=%s)" % role.dn) - - if __name__ == "__main__": CURRENT_FILE = os.path.realpath(__file__) pytest.main("-s -v %s" % CURRENT_FILE) diff --git a/ldap/servers/plugins/roles/roles_cache.c b/ldap/servers/plugins/roles/roles_cache.c index 60d7182e2..60f5a919a 100644 --- a/ldap/servers/plugins/roles/roles_cache.c +++ b/ldap/servers/plugins/roles/roles_cache.c @@ -1117,16 +1117,17 @@ roles_cache_create_object_from_entry(Slapi_Entry *role_entry, role_object **resu rolescopeDN = slapi_entry_attr_get_charptr(role_entry, ROLE_SCOPE_DN); if (rolescopeDN) { - Slapi_DN *rolescopeSDN; - Slapi_DN *top_rolescopeSDN, *top_this_roleSDN; + Slapi_DN *rolescopeSDN = NULL; + Slapi_DN *top_rolescopeSDN = NULL; + Slapi_DN *top_this_roleSDN = NULL; /* 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) && + if (rolescopeSDN && (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) { + if (top_rolescopeSDN && top_this_roleSDN && 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 { @@ -1148,6 +1149,7 @@ roles_cache_create_object_from_entry(Slapi_Entry *role_entry, role_object **resu rolescopeDN); slapi_sdn_free(&rolescopeSDN); } + slapi_ch_free_string(&rolescopeDN); } /* Depending upon role type, pull out the remaining information we need */ -- 2.49.0