CVE-2021-4091 - 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 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>
Origin: upstream, commit:a3c298f8140d3e4fa1bd5a670f1bb965a21a9b7b

Gbp-Pq: Name CVE-2021-4091-double-free-of-virtual-attribute-ctx.patch

diff --git a/ldap/servers/slapd/connection.c b/ldap/servers/slapd/connection.c
index 1883fe711ed6d6e1b84372dda6f4a22e2583499d..d8223ae1c2f5ce6dd3ad6ce39f6acf3c025cfc7c 100644 (file)
--- a/ldap/servers/slapd/connection.c
+++ b/ldap/servers/slapd/connection.c
@@ -1808,9 +1808,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 b991f35bcc8b5ba6e68e41369ec10fabf6e60514..9da90e3fb70b971ede8e453627f66633f2b2e1d2 100644 (file)
--- 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;