From bc8bdaa57ba9b57671e2921705b99eaa70729ce7 Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Wed, 11 Nov 2020 11:45:11 -0500 Subject: [PATCH 4/8] Issue 4429 - NULL dereference in revert_cache() Bug Description: During a delete, if the DN (with an escaped leading space) of an existing entry fail to parse the server will revert the entry update. In this case it will lead to a crash becuase ther ldbm inst struct is not set before it attempts the cache revert. Fix Description: Check the the ldbm instance struct is not NULL before dereferencing it. Relates: https://github.com/389ds/389-ds-base/issues/4429 Reviewed by: firstyear & spichugi(Thanks!!) --- .../tests/suites/syntax/acceptance_test.py | 40 +++++++++++++++++++ ldap/servers/slapd/back-ldbm/cache.c | 3 ++ 2 files changed, 43 insertions(+) diff --git a/dirsrvtests/tests/suites/syntax/acceptance_test.py b/dirsrvtests/tests/suites/syntax/acceptance_test.py index db8f63c7e..543718689 100644 --- a/dirsrvtests/tests/suites/syntax/acceptance_test.py +++ b/dirsrvtests/tests/suites/syntax/acceptance_test.py @@ -6,12 +6,14 @@ # See LICENSE for details. # --- END COPYRIGHT BLOCK --- +import ldap import logging import pytest import os from lib389.schema import Schema from lib389.config import Config from lib389.idm.user import UserAccounts +from lib389.idm.group import Groups from lib389._constants import DEFAULT_SUFFIX from lib389.topologies import log, topology_st as topo @@ -105,6 +107,44 @@ def test_invalid_uidnumber(topo, validate_syntax_off): log.info('Found an invalid entry with wrong uidNumber - Success') +def test_invalid_dn_syntax_crash(topo): + """Add an entry with an escaped space, restart the server, and try to delete + it. In this case the DN is not correctly parsed and causes cache revert to + to dereference a NULL pointer. So the delete can fail as long as the server + does not crash. + + :id: 62d87272-dfb8-4627-9ca1-dbe33082caf8 + :setup: Standalone Instance + :steps: + 1. Add entry with leading escaped space in the RDN + 2. Restart the server so the entry is rebuilt from the database + 3. Delete the entry + 4. The server should still be running + :expectedresults: + 1. Success + 2. Success + 3. Success + 4. Success + """ + + # Create group + groups = Groups(topo.standalone, DEFAULT_SUFFIX) + group = groups.create(properties={'cn': ' test'}) + + # Restart the server + topo.standalone.restart() + + # Delete group + try: + group.delete() + except ldap.NO_SUCH_OBJECT: + # This is okay in this case as we are only concerned about a crash + pass + + # Make sure server is still running + groups.list() + + if __name__ == '__main__': # Run isolated # -s for DEBUG mode diff --git a/ldap/servers/slapd/back-ldbm/cache.c b/ldap/servers/slapd/back-ldbm/cache.c index 89f958a35..5ad9ca829 100644 --- a/ldap/servers/slapd/back-ldbm/cache.c +++ b/ldap/servers/slapd/back-ldbm/cache.c @@ -614,6 +614,9 @@ flush_hash(struct cache *cache, struct timespec *start_time, int32_t type) void revert_cache(ldbm_instance *inst, struct timespec *start_time) { + if (inst == NULL) { + return; + } flush_hash(&inst->inst_cache, start_time, ENTRY_CACHE); flush_hash(&inst->inst_dncache, start_time, DN_CACHE); } -- 2.26.2