From fa54d0ce631d29343d04b8dcea8ef8d9c235feb3 Mon Sep 17 00:00:00 2001 From: William Brown Date: Dec 16 2016 16:52:22 +0000 Subject: Ticket 48665 - Prevent sefault in ldbm_instance_modify_config_entry Bug Description: python ldap sends a mod_delete then a mod_add to the server. This causes a segfault in ldbm_instance_modify_config_entry which expects the value of the delete operation to exist. Fix Description: If there is no attribute in the delete operation, this now passes a NULL to ldbm_config_set, which then rejects the op as NO_SUCH_ATTRIBUTE There is an attached test case, which also checks that mod_replace is still functional. https://fedorahosted.org/389/ticket/48665 Author: wibrown Review by: nhosoi (Thanks!) (cherry picked from commit 9134b006948d7d8eeb926655640b7f068fc6724d) --- diff --git a/dirsrvtests/tests/tickets/ticket48665_test.py b/dirsrvtests/tests/tickets/ticket48665_test.py new file mode 100644 index 0000000..702319c --- /dev/null +++ b/dirsrvtests/tests/tickets/ticket48665_test.py @@ -0,0 +1,101 @@ +import os +import sys +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 * + +logging.getLogger(__name__).setLevel(logging.DEBUG) +log = logging.getLogger(__name__) + +class TopologyStandalone(object): + def __init__(self, standalone): + standalone.open() + self.standalone = standalone + + +@pytest.fixture(scope="module") +def topology(request): + # Creating standalone instance ... + standalone = DirSrv(verbose=False) + args_instance[SER_HOST] = HOST_STANDALONE + args_instance[SER_PORT] = PORT_STANDALONE + args_instance[SER_SERVERID_PROP] = SERVERID_STANDALONE + args_instance[SER_CREATION_SUFFIX] = DEFAULT_SUFFIX + args_standalone = args_instance.copy() + standalone.allocate(args_standalone) + instance_standalone = standalone.exists() + if instance_standalone: + standalone.delete() + standalone.create() + standalone.open() + + # Delete each instance in the end + def fin(): + # This is useful for analysing the test env. + #standalone.db2ldif(bename=DEFAULT_BENAME, suffixes=[DEFAULT_SUFFIX], excludeSuffixes=[], encrypt=False, \ + # repl_data=True, outputfile='%s/ldif/%s.ldif' % (standalone.dbdir,SERVERID_STANDALONE )) + #standalone.clearBackupFS() + #standalone.backupFS() + standalone.delete() + request.addfinalizer(fin) + + # Clear out the tmp dir + standalone.clearTmpDir(__file__) + + return TopologyStandalone(standalone) + + +def test_ticket48665(topology): + """ + This tests deletion of certain cn=config values. + + First, it should be able to delete, and not crash the server. + + Second, we might be able to delete then add to replace values. + + We should also still be able to mod replace the values and keep the server alive. + """ + #topology.standalone.config.enable_log('audit') + #topology.standalone.config.enable_log('auditfail') + # This will trigger a mod delete then add. + try: + modlist = [(ldap.MOD_DELETE, 'nsslapd-cachememsize', None), (ldap.MOD_ADD, 'nsslapd-cachememsize', '1')] + topology.standalone.modify_s("cn=%s,cn=ldbm database,cn=plugins,cn=config" % DEFAULT_BENAME, + modlist) + except: + pass + # Check the server has not commited seppuku. + result = topology.standalone.whoami_s() + assert(DN_DM.lower() in result.lower()) + + # This has a magic hack to determine if we are in cn=config. + topology.standalone.backend.setProperties(bename=DEFAULT_BENAME, + prop='nsslapd-cachememsize', values='1') + # Check the server has not commited seppuku. + result = topology.standalone.whoami_s() + assert(DN_DM.lower() in result.lower()) + + # Now try with mod_replace. This should be okay. + + modlist = [(ldap.MOD_REPLACE, 'nsslapd-cachememsize', '1')] + topology.standalone.modify_s("cn=%s,cn=ldbm database,cn=plugins,cn=config" % DEFAULT_BENAME, + modlist) + # Check the server has not commited seppuku. + result = topology.standalone.whoami_s() + assert(DN_DM.lower() in result.lower()) + + log.info('Test complete') + + +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/slapd/back-ldbm/ldbm_instance_config.c b/ldap/servers/slapd/back-ldbm/ldbm_instance_config.c index 2037618..c269ab3 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_instance_config.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_instance_config.c @@ -789,9 +789,19 @@ ldbm_instance_modify_config_entry_callback(Slapi_PBlock *pb, Slapi_Entry* entryB } /* This assumes there is only one bval for this mod. */ - rc = ldbm_config_set((void *) inst, attr_name, - ldbm_instance_config, mods[i]->mod_bvalues[0], returntext, - CONFIG_PHASE_RUNNING, apply_mod, mods[i]->mod_op); + if (mods[i]->mod_bvalues == NULL) { + /* This avoids the null pointer deref. + * In ldbm_config.c ldbm_config_set, it checks for the NULL. + * If it's a NULL, we get NO_SUCH_ATTRIBUTE error. + */ + rc = ldbm_config_set((void *) inst, attr_name, + ldbm_instance_config, NULL, returntext, + CONFIG_PHASE_RUNNING, apply_mod, mods[i]->mod_op); + } else { + rc = ldbm_config_set((void *) inst, attr_name, + ldbm_instance_config, mods[i]->mod_bvalues[0], returntext, + CONFIG_PHASE_RUNNING, apply_mod, mods[i]->mod_op); + } } }