[PATCH] Issue 5218 - double-free of the virtual attribute context in persistent searc...

description:
	A search is processed by a worker using a private pblock.
	If the search is persistent, the worker spawn a thread
	and kind of duplicate its private pblock so that the spawn
        thread continue to process the persistent search.
	Then worker ends the initial search, reinit (free) its private pblock,
        and returns monitoring the wait_queue.
	When the persistent search completes, it frees the duplicated
	pblock.
	The problem is that private pblock and duplicated pblock
        are referring to a same structure (pb_vattr_context).
        That can lead to a double free

Fix:
	When cloning the pblock (slapi_pblock_clone) make sure
	to transfert the references inside the original (private)
	pblock to the target (cloned) one
        That includes pb_vattr_context pointer.

Reviewed by: Mark Reynolds, James Chapman, Pierre Rogier (Thanks !)

Co-authored-by: Mark Reynolds <mreynolds@redhat.com>
Gbp-Pq: Name CVE-2021-4091.patch

diff --git a/ldap/servers/slapd/connection.c b/ldap/servers/slapd/connection.c
index 8b885686c8905780ef4f002749cdc42a76ad1108..e18917cbb5038a0b757096879db543c828ab6573 100644 (file)
--- a/ldap/servers/slapd/connection.c
+++ b/ldap/servers/slapd/connection.c
@@ -1827,9 +1827,11 @@ connection_threadmain()
                 PR_ExitMonitor(conn->c_mutex);
             }
             /* ps_add makes a shallow copy of the pb - so we
-                 * can't free it or init it here - just set operation to NULL.
-                 * ps_send_results will call connection_remove_operation_ext to free it
-                 */
+             * can't free it or init it here - just set operation to NULL.
+             * ps_send_results will call connection_remove_operation_ext to free it
+             * The connection_thread private pblock ('pb') has be cloned and should only
+             * be reinit (slapi_pblock_init)
+             */
             slapi_pblock_set(pb, SLAPI_OPERATION, NULL);
             slapi_pblock_init(pb);
         } else {

diff --git a/ldap/servers/slapd/pblock.c b/ldap/servers/slapd/pblock.c
index bc18a7b18bfd40819477858540bafb7fb4d1508f..e81fefba31025e44edafbb5760fad2b82670cc5d 100644 (file)
--- a/ldap/servers/slapd/pblock.c
+++ b/ldap/servers/slapd/pblock.c
@@ -290,6 +290,12 @@ _pblock_assert_pb_deprecated(Slapi_PBlock *pblock)
     }
 }
 
+/* It clones the pblock
+ * the content of the source pblock is transfered
+ * to the target pblock (returned)
+ * The source pblock should not be used for any operation
+ * it needs to be reinit (slapi_pblock_init)
+ */
 Slapi_PBlock *
 slapi_pblock_clone(Slapi_PBlock *pb)
 {
@@ -310,28 +316,32 @@ slapi_pblock_clone(Slapi_PBlock *pb)
     if (pb->pb_task != NULL) {
         _pblock_assert_pb_task(new_pb);
         *(new_pb->pb_task) = *(pb->pb_task);
+        memset(pb->pb_task, 0, sizeof(slapi_pblock_task));
     }
     if (pb->pb_mr != NULL) {
         _pblock_assert_pb_mr(new_pb);
         *(new_pb->pb_mr) = *(pb->pb_mr);
+        memset(pb->pb_mr, 0, sizeof(slapi_pblock_matching_rule));
     }
     if (pb->pb_misc != NULL) {
         _pblock_assert_pb_misc(new_pb);
         *(new_pb->pb_misc) = *(pb->pb_misc);
+        memset(pb->pb_misc, 0, sizeof(slapi_pblock_misc));
     }
     if (pb->pb_intop != NULL) {
         _pblock_assert_pb_intop(new_pb);
         *(new_pb->pb_intop) = *(pb->pb_intop);
-        /* set pwdpolicy to NULL so this clone allocates its own policy */
-        new_pb->pb_intop->pwdpolicy = NULL;
+        memset(pb->pb_intop, 0, sizeof(slapi_pblock_intop));
     }
     if (pb->pb_intplugin != NULL) {
         _pblock_assert_pb_intplugin(new_pb);
         *(new_pb->pb_intplugin) = *(pb->pb_intplugin);
+        memset(pb->pb_intplugin, 0,sizeof(slapi_pblock_intplugin));
     }
     if (pb->pb_deprecated != NULL) {
         _pblock_assert_pb_deprecated(new_pb);
         *(new_pb->pb_deprecated) = *(pb->pb_deprecated);
+        memset(pb->pb_deprecated, 0, sizeof(slapi_pblock_deprecated));
     }
 #ifdef PBLOCK_ANALYTICS
     new_pb->analytics = NULL;