From 7b7217538908ae58df864ef5cd82e1d3303c189f Mon Sep 17 00:00:00 2001

From: Mark Reynolds <mreynolds@redhat.com>

Date: Mon, 7 Jun 2021 12:58:42 -0400

Subject: [PATCH] Issue 4447 - Crash when the Referential Integrity log is

 manually edited

Bug Description:  If the referint log is manually edited with a string

                  that is not a DN the server will crash when processing

                  the log.

Fix Description:  Check for NULL pointers when strtoking the file line.

relates: https://github.com/389ds/389-ds-base/issues/4447

Reviewed by: firstyear(Thanks!)

---

 .../tests/suites/plugins/referint_test.py     | 72 +++++++++++++++----

 ldap/servers/plugins/referint/referint.c      |  7 ++

 src/lib389/lib389/plugins.py                  | 15 ++++

 3 files changed, 80 insertions(+), 14 deletions(-)

diff --git a/dirsrvtests/tests/suites/plugins/referint_test.py b/dirsrvtests/tests/suites/plugins/referint_test.py

index 02b985767..fda602545 100644

--- a/dirsrvtests/tests/suites/plugins/referint_test.py

+++ b/dirsrvtests/tests/suites/plugins/referint_test.py

@@ -1,5 +1,5 @@

 # --- BEGIN COPYRIGHT BLOCK ---

-# Copyright (C) 2016 Red Hat, Inc.

+# Copyright (C) 2021 Red Hat, Inc.

 # All rights reserved.

 #

 # License: GPL (version 3 or any later version).

@@ -12,13 +12,11 @@ Created on Dec 12, 2019

 @author: tbordaz

 '''

 import logging

-import subprocess

 import pytest

 from lib389 import Entry

-from lib389.utils import *

-from lib389.plugins import *

-from lib389._constants import *

-from lib389.idm.user import UserAccounts, UserAccount

+from lib389.plugins import ReferentialIntegrityPlugin

+from lib389._constants import DEFAULT_SUFFIX

+from lib389.idm.user import UserAccounts

 from lib389.idm.group import Groups

 from lib389.topologies import topology_st as topo

@@ -29,21 +27,27 @@ log = logging.getLogger(__name__)

 ESCAPED_RDN_BASE = "foo\\,oo"

 def _user_get_dn(no):

     uid = '%s%d' % (ESCAPED_RDN_BASE, no)

-    dn = 'uid=%s,%s' % (uid, SUFFIX)

+    dn = 'uid=%s,%s' % (uid, DEFAULT_SUFFIX)

     return (uid, dn)

 def add_escaped_user(server, no):

     (uid, dn) = _user_get_dn(no)

     log.fatal('Adding user (%s): ' % dn)

-    server.add_s(Entry((dn, {'objectclass': ['top', 'person', 'organizationalPerson', 'inetOrgPerson'],

-                             'uid': [uid],

-                             'sn' : [uid],

-                             'cn' : [uid]})))

+    users = UserAccounts(server, DEFAULT_SUFFIX, None)

+    user_properties = {

+        'objectclass': ['top', 'person', 'organizationalPerson', 'inetOrgPerson', 'posixAccount'],

+        'uid': uid,

+        'cn' : uid,

+        'sn' : uid,

+        'uidNumber' : '1000',

+        'gidNumber' : '2000',

+        'homeDirectory' : '/home/testuser',

+    }

+    users.create(properties=user_properties)

     return dn

-@pytest.mark.ds50020

 def test_referential_false_failure(topo):

-    """On MODRDN referential integrity can erronously fail

+    """On MODRDN referential integrity can erroneously fail

     :id: f77aeb80-c4c4-471b-8c1b-4733b714778b

     :setup: Standalone Instance

@@ -100,6 +104,46 @@ def test_referential_false_failure(topo):

     inst.restart()

     # Here if the bug is fixed, referential is able to update the member value

-    inst.rename_s(user1.dn, 'uid=new_test_user_1001', newsuperior=SUFFIX, delold=0)

+    user1.rename('uid=new_test_user_1001', newsuperior=DEFAULT_SUFFIX, deloldrdn=False)

+def test_invalid_referint_log(topo):

+    """If there is an invalid log line in the referint log, make sure the server

+    does not crash at startup

+

+    :id: 34807b5a-ab17-4281-ae48-4e3513e19145

+    :setup: Standalone Instance

+    :steps:

+        1. Set the referint log delay

+        2. Create invalid log

+        3. Start the server (no crash)

+    :expectedresults:

+        1. Success

+        2. Success

+        3. Success

+    """

+

+    inst = topo.standalone

+

+    # Set delay - required for log parsing at server startup

+    plugin = ReferentialIntegrityPlugin(inst)

+    plugin.enable()

+    plugin.set_update_delay('2')

+    logfile = plugin.get_log_file()

+    inst.restart()

+

+    # Create invalid log

+    inst.stop()

+    with open(logfile, 'w') as log_fh:

+        log_fh.write("CRASH\n")

+

+    # Start the instance

+    inst.start()

+    assert inst.status()

+

+

+if __name__ == '__main__':

+    # Run isolated

+    # -s for DEBUG mode

+    CURRENT_FILE = os.path.realpath(__file__)

+    pytest.main("-s %s" % CURRENT_FILE)

diff --git a/ldap/servers/plugins/referint/referint.c b/ldap/servers/plugins/referint/referint.c

index fd5356d72..28240c1f6 100644

--- a/ldap/servers/plugins/referint/referint.c

+++ b/ldap/servers/plugins/referint/referint.c

@@ -1447,6 +1447,13 @@ referint_thread_func(void *arg __attribute__((unused)))

             sdn = slapi_sdn_new_normdn_byref(ptoken);

             ptoken = ldap_utf8strtok_r(NULL, delimiter, &iter);

+            if (ptoken == NULL) {

+                /* Invalid line in referint log, skip it */

+                slapi_log_err(SLAPI_LOG_ERR, REFERINT_PLUGIN_SUBSYSTEM,

+                        "Skipping invalid referint log line: (%s)\n", thisline);

+                slapi_sdn_free(&sdn;;

+                continue;

+            }

             if (!strcasecmp(ptoken, "NULL")) {

                 tmprdn = NULL;

             } else {

diff --git a/src/lib389/lib389/plugins.py b/src/lib389/lib389/plugins.py

index 2d88e60bd..b07e80022 100644

--- a/src/lib389/lib389/plugins.py

+++ b/src/lib389/lib389/plugins.py

@@ -518,6 +518,21 @@ class ReferentialIntegrityPlugin(Plugin):

         self.set('referint-update-delay', str(value))

+    def get_log_file(self):

+        """Get referint log file"""

+

+        return self.get_attr_val_utf8('referint-logfile')

+

+    def get_log_file_formatted(self):

+        """Get referint log file"""

+

+        return self.display_attr('referint-logfile')

+

+    def set_log_file(self, value):

+        """Set referint log file"""

+

+        self.set('referint-logfile', value)

+

     def get_membership_attr(self, formatted=False):

         """Get referint-membership-attr attribute"""

-- 

2.31.1