|
 |
51b5b9 |
From d000349089eb15b3476ec302f4279f118336290e Mon Sep 17 00:00:00 2001
|
|
|
51b5b9 |
From: Mark Reynolds <mreynolds@redhat.com>
|
|
|
51b5b9 |
Date: Thu, 16 Dec 2021 16:13:08 -0500
|
|
|
51b5b9 |
Subject: [PATCH 1/2] CVE-2021-4091 (BZ#2030367) double-free of the virtual
|
|
|
51b5b9 |
attribute context in persistent search
|
|
|
51b5b9 |
|
|
|
51b5b9 |
description:
|
|
|
51b5b9 |
A search is processed by a worker using a private pblock.
|
|
|
51b5b9 |
If the search is persistent, the worker spawn a thread
|
|
|
51b5b9 |
and kind of duplicate its private pblock so that the spawn
|
|
|
51b5b9 |
thread continue to process the persistent search.
|
|
|
51b5b9 |
Then worker ends the initial search, reinit (free) its private pblock,
|
|
|
51b5b9 |
and returns monitoring the wait_queue.
|
|
|
51b5b9 |
When the persistent search completes, it frees the duplicated
|
|
|
51b5b9 |
pblock.
|
|
|
51b5b9 |
The problem is that private pblock and duplicated pblock
|
|
|
51b5b9 |
are referring to a same structure (pb_vattr_context).
|
|
|
51b5b9 |
That lead to a double free
|
|
|
51b5b9 |
|
|
|
51b5b9 |
Fix:
|
|
|
51b5b9 |
When cloning the pblock (slapi_pblock_clone) make sure
|
|
|
51b5b9 |
to transfert the references inside the original (private)
|
|
|
51b5b9 |
pblock to the target (cloned) one
|
|
|
51b5b9 |
That includes pb_vattr_context pointer.
|
|
|
51b5b9 |
|
|
|
51b5b9 |
Reviewed by: Mark Reynolds, James Chapman, Pierre Rogier (Thanks !)
|
|
|
51b5b9 |
---
|
|
|
51b5b9 |
ldap/servers/slapd/connection.c | 8 +++++---
|
|
|
51b5b9 |
ldap/servers/slapd/pblock.c | 14 ++++++++++++--
|
|
|
51b5b9 |
2 files changed, 17 insertions(+), 5 deletions(-)
|
|
|
51b5b9 |
|
|
|
51b5b9 |
diff --git a/ldap/servers/slapd/connection.c b/ldap/servers/slapd/connection.c
|
|
|
51b5b9 |
index e0c1a52d2..fc7ed9c4a 100644
|
|
|
51b5b9 |
--- a/ldap/servers/slapd/connection.c
|
|
|
51b5b9 |
+++ b/ldap/servers/slapd/connection.c
|
|
|
51b5b9 |
@@ -1823,9 +1823,11 @@ connection_threadmain()
|
|
|
51b5b9 |
pthread_mutex_unlock(&(conn->c_mutex));
|
|
|
51b5b9 |
}
|
|
|
51b5b9 |
/* ps_add makes a shallow copy of the pb - so we
|
|
|
51b5b9 |
- * can't free it or init it here - just set operation to NULL.
|
|
|
51b5b9 |
- * ps_send_results will call connection_remove_operation_ext to free it
|
|
|
51b5b9 |
- */
|
|
|
51b5b9 |
+ * can't free it or init it here - just set operation to NULL.
|
|
|
51b5b9 |
+ * ps_send_results will call connection_remove_operation_ext to free it
|
|
|
51b5b9 |
+ * The connection_thread private pblock ('pb') has be cloned and should only
|
|
|
51b5b9 |
+ * be reinit (slapi_pblock_init)
|
|
|
51b5b9 |
+ */
|
|
|
51b5b9 |
slapi_pblock_set(pb, SLAPI_OPERATION, NULL);
|
|
|
51b5b9 |
slapi_pblock_init(pb);
|
|
|
51b5b9 |
} else {
|
|
|
51b5b9 |
diff --git a/ldap/servers/slapd/pblock.c b/ldap/servers/slapd/pblock.c
|
|
|
51b5b9 |
index a64986aeb..c78d1250f 100644
|
|
|
51b5b9 |
--- a/ldap/servers/slapd/pblock.c
|
|
|
51b5b9 |
+++ b/ldap/servers/slapd/pblock.c
|
|
|
51b5b9 |
@@ -292,6 +292,12 @@ _pblock_assert_pb_deprecated(Slapi_PBlock *pblock)
|
|
|
51b5b9 |
}
|
|
|
51b5b9 |
}
|
|
|
51b5b9 |
|
|
|
51b5b9 |
+/* It clones the pblock
|
|
|
51b5b9 |
+ * the content of the source pblock is transfered
|
|
|
51b5b9 |
+ * to the target pblock (returned)
|
|
|
51b5b9 |
+ * The source pblock should not be used for any operation
|
|
|
51b5b9 |
+ * it needs to be reinit (slapi_pblock_init)
|
|
|
51b5b9 |
+ */
|
|
|
51b5b9 |
Slapi_PBlock *
|
|
|
51b5b9 |
slapi_pblock_clone(Slapi_PBlock *pb)
|
|
|
51b5b9 |
{
|
|
|
51b5b9 |
@@ -312,28 +318,32 @@ slapi_pblock_clone(Slapi_PBlock *pb)
|
|
|
51b5b9 |
if (pb->pb_task != NULL) {
|
|
|
51b5b9 |
_pblock_assert_pb_task(new_pb);
|
|
|
51b5b9 |
*(new_pb->pb_task) = *(pb->pb_task);
|
|
|
51b5b9 |
+ memset(pb->pb_task, 0, sizeof(slapi_pblock_task));
|
|
|
51b5b9 |
}
|
|
|
51b5b9 |
if (pb->pb_mr != NULL) {
|
|
|
51b5b9 |
_pblock_assert_pb_mr(new_pb);
|
|
|
51b5b9 |
*(new_pb->pb_mr) = *(pb->pb_mr);
|
|
|
51b5b9 |
+ memset(pb->pb_mr, 0, sizeof(slapi_pblock_matching_rule));
|
|
|
51b5b9 |
}
|
|
|
51b5b9 |
if (pb->pb_misc != NULL) {
|
|
|
51b5b9 |
_pblock_assert_pb_misc(new_pb);
|
|
|
51b5b9 |
*(new_pb->pb_misc) = *(pb->pb_misc);
|
|
|
51b5b9 |
+ memset(pb->pb_misc, 0, sizeof(slapi_pblock_misc));
|
|
|
51b5b9 |
}
|
|
|
51b5b9 |
if (pb->pb_intop != NULL) {
|
|
|
51b5b9 |
_pblock_assert_pb_intop(new_pb);
|
|
|
51b5b9 |
*(new_pb->pb_intop) = *(pb->pb_intop);
|
|
|
51b5b9 |
- /* set pwdpolicy to NULL so this clone allocates its own policy */
|
|
|
51b5b9 |
- new_pb->pb_intop->pwdpolicy = NULL;
|
|
|
51b5b9 |
+ memset(pb->pb_intop, 0, sizeof(slapi_pblock_intop));
|
|
|
51b5b9 |
}
|
|
|
51b5b9 |
if (pb->pb_intplugin != NULL) {
|
|
|
51b5b9 |
_pblock_assert_pb_intplugin(new_pb);
|
|
|
51b5b9 |
*(new_pb->pb_intplugin) = *(pb->pb_intplugin);
|
|
|
51b5b9 |
+ memset(pb->pb_intplugin, 0,sizeof(slapi_pblock_intplugin));
|
|
|
51b5b9 |
}
|
|
|
51b5b9 |
if (pb->pb_deprecated != NULL) {
|
|
|
51b5b9 |
_pblock_assert_pb_deprecated(new_pb);
|
|
|
51b5b9 |
*(new_pb->pb_deprecated) = *(pb->pb_deprecated);
|
|
|
51b5b9 |
+ memset(pb->pb_deprecated, 0, sizeof(slapi_pblock_deprecated));
|
|
|
51b5b9 |
}
|
|
|
51b5b9 |
#ifdef PBLOCK_ANALYTICS
|
|
|
51b5b9 |
new_pb->analytics = NULL;
|
|
|
51b5b9 |
--
|
|
|
51b5b9 |
2.31.1
|
|
|
51b5b9 |
|