rpms/pacemaker
7
0
Fork 0
Code Issues Pull Requests Releases Activity
f12c26a619
pacemaker/SOURCES/005-remote_overflow.patch
AlmaLinux RelEng Bot bd9348bff9 import Oracle_OSS pacemaker-2.1.10-3.0.1.el9_8
2026-07-20 04:26:34 -04:00

414 lines
15 KiB
Diff
Raw Blame History

From 264d94dcdb4c6739355f7251b4e9e81256938dbf Mon Sep 17 00:00:00 2001
From: Chris Lumens <clumens@redhat.com>
Date: Thu, 14 May 2026 12:57:17 -0400
Subject: [PATCH 1/6] Med: libcrmcommon: Add sanity checks to
localized_remote_header.
These checks are present in the main and 3.0 branches, and they make
sense here as well.
---
lib/common/remote.c | 52 ++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 47 insertions(+), 5 deletions(-)
diff --git a/lib/common/remote.c b/lib/common/remote.c
index b25e5ce..a4c8b8a 100644
--- a/lib/common/remote.c
+++ b/lib/common/remote.c
@@ -1,5 +1,5 @@
/*
- * Copyright 2008-2024 the Pacemaker project contributors
+ * Copyright 2008-2026 the Pacemaker project contributors
*
* The version control history for this file may have further details.
*
@@ -23,7 +23,7 @@
#include <netdb.h>
#include <stdlib.h>
#include <errno.h>
-#include <inttypes.h> // PRIx32
+#include <inttypes.h> // PRIu32, PRIx32
#include <glib.h>
#include <bzlib.h>
@@ -97,11 +97,18 @@ struct remote_header_v0 {
static struct remote_header_v0 *
localized_remote_header(pcmk__remote_t *remote)
{
- struct remote_header_v0 *header = (struct remote_header_v0 *)remote->buffer;
- if(remote->buffer_offset < sizeof(struct remote_header_v0)) {
+ struct remote_header_v0 *header = NULL;
+ size_t expected_size = 0;
+
+ if ((remote == NULL) || (remote->buffer == NULL)
+ || (remote->buffer_offset < sizeof(struct remote_header_v0))) {
+
+ // Caller error or we haven't received the full header yet
return NULL;
+ }
- } else if(header->endian != ENDIAN_LOCAL) {
+ header = (struct remote_header_v0 *) remote->buffer;
+ if (header->endian != ENDIAN_LOCAL) {
uint32_t endian = __swab32(header->endian);
CRM_LOG_ASSERT(endian == ENDIAN_LOCAL);
@@ -123,6 +130,41 @@ localized_remote_header(pcmk__remote_t *remote)
header->payload_uncompressed = __swab32(header->payload_uncompressed);
}
+ // Sanity checks
+ if (header->payload_offset != sizeof(struct remote_header_v0)) {
+ crm_err("Header payload offset %" PRIu32 " does not have expected "
+ "size %zu", header->payload_offset,
+ sizeof(struct remote_header_v0));
+ return NULL;
+ }
+
+ if (header->payload_compressed != 0) {
+ if (header->payload_compressed > (SIZE_MAX - header->payload_offset)) {
+ crm_err("Header compressed size %" PRIu32 " is too large",
+ header->payload_compressed);
+ return NULL;
+ }
+
+ expected_size = (size_t) header->payload_offset
+ + header->payload_compressed;
+
+ } else {
+ if (header->payload_uncompressed > (SIZE_MAX - header->payload_offset)) {
+ crm_err("Header uncompressed size %" PRIu32 " is too large",
+ header->payload_uncompressed);
+ return NULL;
+ }
+
+ expected_size = (size_t) header->payload_offset
+ + header->payload_uncompressed;
+ }
+
+ if (expected_size != header->size_total) {
+ crm_err("Header total size %" PRIu32 " does not match calculated "
+ "size %zu", header->size_total, expected_size);
+ return NULL;
+ }
+
return header;
}
--
2.53.0
From 8d21d297629bcc8aebf71bb171c550bcc4532c3f Mon Sep 17 00:00:00 2001
From: Chris Lumens <clumens@redhat.com>
Date: Thu, 14 May 2026 13:06:57 -0400
Subject: [PATCH 2/6] High: libcrmcommon: Fix integer overflow in remote
message code.
It's possible for a client to send a specifically constructed header
packet that will cause our size checks in pcmk__remote_message_xml to
overflow, which could result in crashes (potentially leading to a node
being fenced) or memory corruption problems.
Such a header would have to indicate that the payload is compressed,
which nothing in pacemaker is doing. In fact, we haven't used any of
the remote message compression code since it was introduced in 2013
which explains why we haven't seen any problems with it. Thus, it would
require a malicious client to cause this to happen.
It would also require remote CIB administration to be enabled via
remote-clear-port or remote-tls-port, which we do not do by default.
Additionally, if remote-tls-port is in use, it requires the client to
complete the gnutls handshake procedure (pcmk__remote_message_xml is
only called in cib_remote_msg after the handshake but before the client
has authenticated).
Co-Authored-By: Reid Wahl <nrwahl@protonmail.com>
---
lib/common/remote.c | 49 ++++++++++++++++++++++++++++++++++++++-------
1 file changed, 42 insertions(+), 7 deletions(-)
diff --git a/lib/common/remote.c b/lib/common/remote.c
index a4c8b8a..aa8e762 100644
--- a/lib/common/remote.c
+++ b/lib/common/remote.c
@@ -619,16 +619,51 @@ pcmk__remote_message_xml(pcmk__remote_t *remote)
}
/* Support compression on the receiving end now, in case we ever want to add it later */
- if (header->payload_compressed) {
+ if (header->payload_compressed != 0) {
int rc = 0;
- unsigned int size_u = 1 + header->payload_uncompressed;
- char *uncompressed =
- pcmk__assert_alloc(1, header->payload_offset + size_u);
+ unsigned int size_u = 0;
+ char *uncompressed = NULL;
+
+#if (UINT32_MAX < UINT_MAX)
+ if (header->payload_uncompressed >= UINT_MAX) {
+ crm_err("Couldn't decompress message because uncompressed "
+ "payload size (%" PRIu32 ") is greater than UINT_MAX "
+ "(%u)", header->payload_uncompressed, UINT_MAX);
+ return NULL;
+ }
+#endif
+
+ /* @TODO Is the extra byte for the null terminator?
+ * pcmk__remote_send_xml() also adds one byte to the iov length.
+ * (However, we do need to account for the possibility of receiving a
+ * message from an untrusted sender.)
+ */
+ size_u = 1 + header->payload_uncompressed;
+
+ /* Header and uncompressed payload must fit in the destination buffer.
+ * We do not need to separately check the header size here since
+ * localized_remote_header will return NULL if it's incorrect.
+ */
+#if (UINT_MAX >= SIZE_MAX)
+ if ((size_u >= SIZE_MAX)
+ || (header->payload_offset > (SIZE_MAX - size_u))) {
+#else
+ if (header->payload_offset > (SIZE_MAX - size_u)) {
+#endif
+ crm_err("Couldn't decompress message because the required buffer "
+ "size (%" PRIu32 " + %u) is greater than SIZE_MAX (%zu)",
+ header->payload_offset, size_u, SIZE_MAX);
+ return NULL;
+ }
+
+ crm_trace("Decompressing message data %" PRIu32 " bytes into %u "
+ "bytes", header->payload_compressed, size_u);
- crm_trace("Decompressing message data %d bytes into %d bytes",
- header->payload_compressed, size_u);
+ uncompressed = pcmk__assert_alloc(header->payload_offset + size_u,
+ sizeof(char));
- rc = BZ2_bzBuffToBuffDecompress(uncompressed + header->payload_offset, &size_u,
+ rc = BZ2_bzBuffToBuffDecompress(uncompressed + header->payload_offset,
+ &size_u,
remote->buffer + header->payload_offset,
header->payload_compressed, 1, 0);
rc = pcmk__bzlib2rc(rc);
--
2.53.0
From 77a530d1bb867264fa0d42752da44d75ee063830 Mon Sep 17 00:00:00 2001
From: Chris Lumens <clumens@redhat.com>
Date: Thu, 14 May 2026 13:26:07 -0400
Subject: [PATCH 3/6] High: libcrmcommon: Limit the max size of a remote
message.
Previously, we were just taking the sizes straight from the message
header and allocating however much memory was asked for without
checking. It's possible for a malicious client to ask for a very large
amount of memory (say, 4 GB since that'll fit in the uint32_t value for
header->size_total).
In that case, due to the use of pcmk__assert_alloc, we'll crash due to
an out of memory error and the node could potentially be fenced.
The fix is to simply check that the client isn't asking for too much
memory. This does unfortunately impose a size restriction on the
message. It's doubly unfortunate given how much work we've recently put
into removing restrictions on IPC messages allowing for much larger
clusters.
I think the biggest thing these messages will ever contain is a CIB, so
for now I'm going with a limit of 20 MB. It is not my intention that
this become a tuneable parameter that is exposed to the user.
---
include/crm/common/remote_internal.h | 5 ++++-
lib/common/remote.c | 17 +++++++++++++++--
2 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/include/crm/common/remote_internal.h b/include/crm/common/remote_internal.h
index f8c0bea..00f5918 100644
--- a/include/crm/common/remote_internal.h
+++ b/include/crm/common/remote_internal.h
@@ -1,5 +1,5 @@
/*
- * Copyright 2008-2024 the Pacemaker project contributors
+ * Copyright 2008-2026 the Pacemaker project contributors
*
* The version control history for this file may have further details.
*
@@ -15,6 +15,9 @@
#include <crm/common/nodes.h> // pcmk_node_variant_remote
#include <crm/common/scheduler_types.h> // pcmk_node_t
+// The maximum payload size for a remote message (in bytes)
+#define PCMK__REMOTE_MSG_MAX_SIZE (20 * 1024 * 1024)
+
// internal functions from remote.c
typedef struct pcmk__remote_s pcmk__remote_t;
diff --git a/lib/common/remote.c b/lib/common/remote.c
index aa8e762..08b6988 100644
--- a/lib/common/remote.c
+++ b/lib/common/remote.c
@@ -623,6 +623,7 @@ pcmk__remote_message_xml(pcmk__remote_t *remote)
int rc = 0;
unsigned int size_u = 0;
char *uncompressed = NULL;
+ size_t buffer_size = 0;
#if (UINT32_MAX < UINT_MAX)
if (header->payload_uncompressed >= UINT_MAX) {
@@ -656,11 +657,17 @@ pcmk__remote_message_xml(pcmk__remote_t *remote)
return NULL;
}
+ buffer_size = (size_t) header->payload_offset + size_u;
+ if (buffer_size > PCMK__REMOTE_MSG_MAX_SIZE) {
+ crm_err("Message size %zu is larger than max allowed %u bytes",
+ buffer_size, PCMK__REMOTE_MSG_MAX_SIZE);
+ return NULL;
+ }
+
crm_trace("Decompressing message data %" PRIu32 " bytes into %u "
"bytes", header->payload_compressed, size_u);
- uncompressed = pcmk__assert_alloc(header->payload_offset + size_u,
- sizeof(char));
+ uncompressed = pcmk__assert_alloc(buffer_size, sizeof(char));
rc = BZ2_bzBuffToBuffDecompress(uncompressed + header->payload_offset,
&size_u,
@@ -805,6 +812,12 @@ pcmk__read_available_remote_data(pcmk__remote_t *remote)
read_len = header->size_total;
}
+ if (read_len > PCMK__REMOTE_MSG_MAX_SIZE) {
+ crm_err("Message size %zu is larger than max allowed %u bytes",
+ read_len, PCMK__REMOTE_MSG_MAX_SIZE);
+ return EINVAL;
+ }
+
/* automatically grow the buffer when needed */
if(remote->buffer_size < read_len) {
remote->buffer_size = 2 * read_len;
--
2.53.0
From eaae2d00a2d29c0187832a55a81cf42b39d93a25 Mon Sep 17 00:00:00 2001
From: Chris Lumens <clumens@redhat.com>
Date: Fri, 15 May 2026 10:19:33 -0400
Subject: [PATCH 4/6] High: libcrmcommon: Fix an integer overflow in
pcmk__remote_send_xml.
iov[0].iov_len is a size_t. iov[1].iov_len is also a size_t (which
comes from the length of a GString, which is a gsize, which is also a
size_t). We're adding those together and storing them in a uint32_t, so
make sure the result will fit.
Fixes RHEL-181155
---
lib/common/remote.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/lib/common/remote.c b/lib/common/remote.c
index 08b6988..83f050d 100644
--- a/lib/common/remote.c
+++ b/lib/common/remote.c
@@ -586,6 +586,13 @@ pcmk__remote_send_xml(pcmk__remote_t *remote, const xmlNode *msg)
header->version = REMOTE_MSG_VERSION;
header->payload_offset = iov[0].iov_len;
header->payload_uncompressed = iov[1].iov_len;
+
+ if ((UINT32_MAX - iov[0].iov_len) < iov[1].iov_len) {
+ crm_err("Remote message size %zu + %zu exceeds maximum of %" PRIu32,
+ iov[0].iov_len, iov[1].iov_len, UINT32_MAX);
+ goto done;
+ }
+
header->size_total = iov[0].iov_len + iov[1].iov_len;
rc = remote_send_iovs(remote, iov, 2);
@@ -594,6 +601,7 @@ pcmk__remote_send_xml(pcmk__remote_t *remote, const xmlNode *msg)
pcmk_rc_str(rc), rc);
}
+done:
free(iov[0].iov_base);
g_free((gchar *) iov[1].iov_base);
return rc;
--
2.53.0
From 2e097d03f1d93f95162a37a5c21ec46d911af176 Mon Sep 17 00:00:00 2001
From: Chris Lumens <clumens@redhat.com>
Date: Thu, 28 May 2026 12:56:57 -0400
Subject: [PATCH 5/6] Refactor: libcib: Remove an unnecessary coverity
suppression.
With all the previous changes, this suppression is no longer necessary
and can be removed.
---
lib/cib/cib_remote.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/lib/cib/cib_remote.c b/lib/cib/cib_remote.c
index baed17b..a64ff95 100644
--- a/lib/cib/cib_remote.c
+++ b/lib/cib/cib_remote.c
@@ -243,7 +243,6 @@ cib_remote_callback_dispatch(gpointer user_data)
return -1;
}
- // coverity[tainted_data] This can't easily be changed right now
msg = pcmk__remote_message_xml(&private->callback);
if (msg == NULL) {
private->start_time = 0;
--
2.53.0
From 2a3c713c3ceab8f5aded9693d09203d18b33bf55 Mon Sep 17 00:00:00 2001
From: Chris Lumens <clumens@redhat.com>
Date: Wed, 17 Jun 2026 15:55:28 -0400
Subject: [PATCH 6/6] Med: libcrmcommon: Add additional checks to
pcmk__remote_message_xml.
coverity flags the existing code for using a tainted expression as an
index to a pointer. While we're fixing other range errors and overflows
in this code, we should fix this one as well by porting the existing
code from the main branch.
---
lib/common/remote.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/lib/common/remote.c b/lib/common/remote.c
index 83f050d..9ad4b17 100644
--- a/lib/common/remote.c
+++ b/lib/common/remote.c
@@ -620,6 +620,7 @@ xmlNode *
pcmk__remote_message_xml(pcmk__remote_t *remote)
{
xmlNode *xml = NULL;
+ size_t data_size = 0;
struct remote_header_v0 *header = localized_remote_header(remote);
if (header == NULL) {
@@ -709,7 +710,18 @@ pcmk__remote_message_xml(pcmk__remote_t *remote)
/* take ownership of the buffer */
remote->buffer_offset = 0;
- CRM_LOG_ASSERT(remote->buffer[sizeof(struct remote_header_v0) + header->payload_uncompressed - 1] == 0);
+ data_size = (size_t) header->payload_offset + header->payload_uncompressed;
+
+ // Ensure the buffer is as big as it should be
+ CRM_CHECK(remote->buffer_size >= data_size, return NULL);
+
+ /* Ensure the buffer is null-terminated (see
+ * pcmk__read_available_remote_data()).
+ *
+ * Note that payload_uncompressed contains the payload size including the
+ * null byte (see pcmk__remote_send_xml()).
+ */
+ CRM_CHECK(remote->buffer[data_size] == '\0', return NULL);
xml = pcmk__xml_parse(remote->buffer + header->payload_offset);
if (xml == NULL && header->version > REMOTE_MSG_VERSION) {
--
2.53.0
Reference in New Issue View Git Blame Copy Permalink