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