zrhoffman / rpms / 389-ds-base

Forked from rpms/389-ds-base 3 years ago
Clone
Blob Blame History Raw
From bc8bdaa57ba9b57671e2921705b99eaa70729ce7 Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@redhat.com>
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