Blob Blame History Raw
From d000349089eb15b3476ec302f4279f118336290e Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@redhat.com>
Date: Thu, 16 Dec 2021 16:13:08 -0500
Subject: [PATCH 1/2] CVE-2021-4091 (BZ#2030367) double-free of the virtual
 attribute context in persistent search

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 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 !)
---
 ldap/servers/slapd/connection.c |  8 +++++---
 ldap/servers/slapd/pblock.c     | 14 ++++++++++++--
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/ldap/servers/slapd/connection.c b/ldap/servers/slapd/connection.c
index e0c1a52d2..fc7ed9c4a 100644
--- a/ldap/servers/slapd/connection.c
+++ b/ldap/servers/slapd/connection.c
@@ -1823,9 +1823,11 @@ connection_threadmain()
                 pthread_mutex_unlock(&(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 a64986aeb..c78d1250f 100644
--- a/ldap/servers/slapd/pblock.c
+++ b/ldap/servers/slapd/pblock.c
@@ -292,6 +292,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)
 {
@@ -312,28 +318,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;
-- 
2.31.1