Blob Blame Raw
From f03bfc51387fcfe15122ee994626738f71b1935c Mon Sep 17 00:00:00 2001
From: Noriko Hosoi <nhosoi@redhat.com>
Date: Sun, 12 Feb 2017 17:26:46 -0800
Subject: [PATCH 70/71] Ticket #49121 - ns-slapd crashes in ldif_sput due to
 the output buf size is less than the real size.

Description: There were missing pieces in the entry size calculation
when an attribute had no a_present_values nor a_deleted_values.
1) There was no chance to add the size of the attribute type name since
   preceding entry2str_internal_size_valueset did not add any size if
   the value was empty.  The type name size is now explicitly added.
2) a_deletioncsn is added in entry2str_internal_put_attrlist by calling
   valueset_add_string with empty value.  The size was not included in
   the allocated memory to store the entire entry as a string. Now the
   size is added.

Adding CI test ticket49121_test.py.

https://pagure.io/389-ds-base/issue/49121

Reviewed by wibrown@redhat.com (Thank you, William!!)

(cherry picked from commit 543fe89edb0a6410a740a4fff738cace7bc57078)
---
 dirsrvtests/tests/data/ticket49121/utf8str.txt |   1 +
 dirsrvtests/tests/tickets/ticket49121_test.py  | 211 +++++++++++++++++++++++++
 ldap/servers/slapd/entry.c                     |  55 ++++---
 3 files changed, 244 insertions(+), 23 deletions(-)
 create mode 100644 dirsrvtests/tests/data/ticket49121/utf8str.txt
 create mode 100644 dirsrvtests/tests/tickets/ticket49121_test.py

diff --git a/dirsrvtests/tests/data/ticket49121/utf8str.txt b/dirsrvtests/tests/data/ticket49121/utf8str.txt
new file mode 100644
index 0000000..0005c4e
--- /dev/null
+++ b/dirsrvtests/tests/data/ticket49121/utf8str.txt
@@ -0,0 +1 @@
+あいうえお
diff --git a/dirsrvtests/tests/tickets/ticket49121_test.py b/dirsrvtests/tests/tickets/ticket49121_test.py
new file mode 100644
index 0000000..6450297
--- /dev/null
+++ b/dirsrvtests/tests/tickets/ticket49121_test.py
@@ -0,0 +1,211 @@
+# --- BEGIN COPYRIGHT BLOCK ---
+# Copyright (C) 2017 Red Hat, Inc.
+# All rights reserved.
+#
+# License: GPL (version 3 or any later version).
+# See LICENSE for details.
+# --- END COPYRIGHT BLOCK ---
+#
+import pytest
+import sys
+import codecs
+from lib389.tasks import *
+from lib389.utils import *
+from lib389.topologies import topology_m2
+
+DEBUGGING = os.getenv('DEBUGGING', False)
+
+if DEBUGGING:
+    logging.getLogger(__name__).setLevel(logging.DEBUG)
+else:
+    logging.getLogger(__name__).setLevel(logging.INFO)
+log = logging.getLogger(__name__)
+
+
+def test_ticket49121(topology_m2):
+    """
+    Creating some users.
+    Deleting quite a number of attributes which may or may not be in the entry.
+    The attribute type names are to be long.
+    Under the conditions, it did not estimate the size of string format entry
+    shorter than the real size and caused the Invalid write / server crash.
+    """
+    reload(sys)
+    sys.setdefaultencoding('utf-8')
+    log.info('DefaultEncoding: %s' % sys.getdefaultencoding())
+    
+    utf8file = os.path.join(topology_m2.ms["master1"].getDir(__file__, DATA_DIR), "ticket49121/utf8str.txt")
+    utf8obj = codecs.open(utf8file, 'r', 'utf-8')
+    utf8strorig = utf8obj.readline()
+    utf8str = utf8strorig.encode('utf-8').rstrip('\n')
+    utf8obj.close()
+    assert(utf8str)
+
+    # Get the sbin directory so we know where to replace 'ns-slapd'
+    sbin_dir = topology_m2.ms["master1"].get_sbin_dir()
+    log.info('sbin_dir: %s' % sbin_dir)
+
+    # stop M1 to do the next updates
+    topology_m2.ms["master1"].stop(30)
+    topology_m2.ms["master2"].stop(30)
+
+    # wait for the servers shutdown
+    time.sleep(5)
+
+    # Enable valgrind
+    if not topology_m2.ms["master1"].has_asan():
+        valgrind_enable(sbin_dir)
+
+    # start M1 to do the next updates
+    topology_m2.ms["master1"].start()
+    topology_m2.ms["master2"].start()
+
+    for idx in range(1, 10):
+        try:
+            USER_DN = 'CN=user%d,ou=People,%s' % (idx, DEFAULT_SUFFIX)
+            log.info('adding user %s...' % (USER_DN))
+            topology_m2.ms["master1"].add_s(Entry((USER_DN,
+                            {'objectclass': 'top person extensibleObject'.split(' '),
+                             'cn': 'user%d' % idx,
+                             'sn': 'SN%d-%s' % (idx, utf8str)})))
+        except ldap.LDAPError as e:
+            log.fatal('Failed to add user (%s): error %s' % (USER_DN, e.message['desc']))
+            assert False
+
+    for i in range(1, 3):
+        time.sleep(3)
+        for idx in range(1, 10):
+            try:
+                USER_DN = 'CN=user%d,ou=People,%s' % (idx, DEFAULT_SUFFIX)
+                log.info('[%d] modify user %s - replacing attrs...' % (i, USER_DN))
+                topology_m2.ms["master1"].modify_s(
+                        USER_DN, [(ldap.MOD_REPLACE, 'cn', 'user%d' % idx),
+                                  (ldap.MOD_REPLACE, 'ABCDEFGH_ID', ['239001ad-06dd-e011-80fa-c00000ad5174',
+                                                                     '240f0878-c552-e411-b0f3-000006040037']),
+                                  (ldap.MOD_REPLACE, 'attr1', 'NEW_ATTR'),
+                                  (ldap.MOD_REPLACE, 'attr20000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr30000000000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr40000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr50000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr600000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr7000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr8000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr900000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr1000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr110000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr120000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr130000000000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr140000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr150000000000000000000000000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr1600000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr17000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr18000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr1900000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr2000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr210000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr220000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr230000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr240000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr25000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr260000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr270000000000000000000000000000000000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr280000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr29000000000000000000000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr3000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr310000000000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr320000000000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr330000000000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr340000000000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr350000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr360000000000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr370000000000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr380000000000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr390000000000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr4000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr410000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr420000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr430000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr440000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr4500000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr460000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr470000000000000000000000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr480000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr49000000000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr5000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr510000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr520000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr530000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr540000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr550000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr5600000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr57000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr58000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr5900000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr6000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr6100000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr6200000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr6300000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr6400000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr65000000000000000000000000000000000000000000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr6600000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr6700000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr6800000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr690000000000000000000000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr7000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr71000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr72000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr73000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr74000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr750000000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr7600000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr77000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr78000000000000000000000000000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr79000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr800000000000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr81000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr82000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr83000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr84000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr85000000000000000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr8600000000000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr87000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr88000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr89000000000000000000000000000000000', None),
+                                  (ldap.MOD_REPLACE, 'attr9000000000000000000000000000000000000000000000000000', None)])
+            except ldap.LDAPError as e:
+                log.fatal('Failed to modify user - deleting attrs (%s): error %s' % (USER_DN, e.message['desc']))
+
+    if not topology_m2.ms["master1"].has_asan():
+        results_file = valgrind_get_results_file(topology_m2.ms["master1"])
+
+    # Stop master2
+    topology_m2.ms["master1"].stop(30)
+    topology_m2.ms["master2"].stop(30)
+
+    # Check for leak
+    if not topology_m2.ms["master1"].has_asan():
+        # Check for invalid read/write
+        if valgrind_check_file(results_file, VALGRIND_INVALID_STR):
+            log.info('Valgrind reported invalid!')
+            assert False
+        else:
+            log.info('Valgrind is happy!')
+
+    # Disable valgrind
+    if not topology_m2.ms["master1"].has_asan():
+        valgrind_disable(sbin_dir)
+
+    # start M1 to do the next updates
+    topology_m2.ms["master1"].start()
+    topology_m2.ms["master2"].start()
+
+    log.info('Testcase PASSED')
+    if DEBUGGING:
+        # Add debugging steps(if any)...
+        pass
+
+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/entry.c b/ldap/servers/slapd/entry.c
index 0cd3b60..ed99a38 100644
--- a/ldap/servers/slapd/entry.c
+++ b/ldap/servers/slapd/entry.c
@@ -1472,7 +1472,8 @@ bail:
 }
 
 static size_t
-entry2str_internal_size_valueset( const char *attrtype, const Slapi_ValueSet *vs, int entry2str_ctrl, int attribute_state, int value_state )
+entry2str_internal_size_valueset( const Slapi_Attr *a, const char *attrtype, const Slapi_ValueSet *vs,
+                                  int entry2str_ctrl, int attribute_state, int value_state )
 {
 	size_t elen= 0;
 	if(!valueset_isempty(vs))
@@ -1485,6 +1486,12 @@ entry2str_internal_size_valueset( const char *attrtype, const Slapi_ValueSet *vs
 												 attribute_state, value_state );
 		}
 	}
+	if(entry2str_ctrl & SLAPI_DUMP_STATEINFO) {
+		/* ";adcsn-" + a->a_deletioncsn */
+		if ( a && a->a_deletioncsn ) {
+			elen += 1 + LDIF_CSNPREFIX_MAXLENGTH + CSN_STRSIZE;
+		}
+	}
 	return elen;
 }
 
@@ -1501,30 +1508,34 @@ entry2str_internal_size_attrlist( const Slapi_Attr *attrlist, int entry2str_ctrl
 			continue;
 
 		/* Count the space required for the present and deleted values */
-		elen+= entry2str_internal_size_valueset(a->a_type, &a->a_present_values,
-												entry2str_ctrl, attribute_state,
-												VALUE_PRESENT);
-		if(entry2str_ctrl & SLAPI_DUMP_STATEINFO)
-		{
-			elen+= entry2str_internal_size_valueset(a->a_type, &a->a_deleted_values,
-													entry2str_ctrl, attribute_state,
-													VALUE_DELETED);
-			/* ";adcsn-" + a->a_deletioncsn */
-			if ( a->a_deletioncsn )
-			{
-				elen += 1 + LDIF_CSNPREFIX_MAXLENGTH + CSN_STRSIZE;
-			}
-			if ( valueset_isempty(&a->a_deleted_values)) {
+		elen += entry2str_internal_size_valueset(a, a->a_type, &a->a_present_values,
+		                                         entry2str_ctrl, attribute_state, VALUE_PRESENT);
+		if (entry2str_ctrl & SLAPI_DUMP_STATEINFO) {
+			elen += entry2str_internal_size_valueset(a, a->a_type, &a->a_deleted_values,
+			                                         entry2str_ctrl, attribute_state, VALUE_DELETED);
+			if (valueset_isempty(&a->a_deleted_values) && valueset_isempty(&a->a_present_values)) {
 				/* this means the entry is deleted and has no more attributes,
 				 * when writing the attr to disk we would loose the AD-csn.
-				 * Add an empty value to the set of deleted values. This will 
-				 * never be seen by any client. It will never be moved to the 
+				 * Add an empty value to the set of deleted values. This will
+				 * never be seen by any client. It will never be moved to the
 				 * present values and is only used to preserve the AD-csn
 				 * We need to add the size for that.
 				 */
 				elen += 1 + LDIF_CSNPREFIX_MAXLENGTH + CSN_STRSIZE;
-                                /* need also space for ";deletedattribute;deleted" */
-                                elen += DELETED_ATTR_STRSIZE + DELETED_VALUE_STRSIZE;
+				/* need also space for ";deletedattribute;deleted" */
+				elen += DELETED_ATTR_STRSIZE + DELETED_VALUE_STRSIZE;
+				/*
+				 * If a_deleted_values is empty && if a_deletioncsn is NULL,
+				 * a_deletioncsn is initialized via valueset_add_string.
+				 * The size needs to be added.
+				 */
+				/* ";adcsn-" + a->a_deletioncsn */
+				elen += 1 + LDIF_CSNPREFIX_MAXLENGTH + CSN_STRSIZE;
+				/*
+				 * When both a_present_values & a_deleted_values are empty,
+				 * the type size is not added.
+				 */
+				elen += PL_strlen(a->a_type);
 			}
 		}
 	}
@@ -1811,10 +1822,8 @@ entry2str_internal_ext( Slapi_Entry *e, int *len, int entry2str_ctrl)
         if (NULL != slapi_entry_get_rdn_const(e))
         {
             slapi_value_set_string(&rdnvalue, slapi_entry_get_rdn_const(e));
-            elen += entry2str_internal_size_value("rdn", &rdnvalue,
-                                                  entry2str_ctrl,
-                                                  ATTRIBUTE_PRESENT,
-                                                  VALUE_PRESENT);
+            elen += entry2str_internal_size_value("rdn", &rdnvalue, entry2str_ctrl,
+                                                  ATTRIBUTE_PRESENT, VALUE_PRESENT);
         }
     
         /* Count the space required for the present attributes */
-- 
2.7.4