Source: glib2.0
Version: 2.74.6-2+deb12u1
Severity: minor
Tags: patch fixed-upstream
X-Debbugs-Cc: [email protected]
Control: found -1 2.79.0+git20240110~g38f5ba3c-1
Control: found -1 2.66.8-1+deb11u2
Control: fixed -1 2.80.2-1
While applying the CVE-2024-34397 fixes to glib2.0 in (old)stable,
I backported an upstream bug fix from GLib 2.79.x to fix a possible
use-after-free, which had gone undetected in older versions, but caused
intermittent failures in the new test coverage that I added to reproduce
CVE-2024-34397.
It turns out that this bug fix is not 100% correct, and causes a rare
memory leak: if a program replaces a non-NULL message body with a different
non-NULL message body, the first argument of the old message body is leaked.
This has now been fixed upstream and will be in 2.80.3, and I backported
the fix in 2.80.2-1 (uploaded to unstable today).
My current understanding is that this will not affect
typical applications, and only applications that call
g_dbus_connection_add_filter() and use it to edit incoming or outgoing
messages in-place will normally be affected. This is not a common pattern,
and the only real-world use I'm aware of is that stretch's gdm3 had a hack
that used this to work around an incompatibility between jessie and stretch
versions' D-Bus APIs during upgrade.
This is technically a regression in a security update, so I'm cc'ing
the security team, but I imagine they will not want to update the DSA
for this. My inclination is to wait for a few days to see whether
there are any other regressions; if yes, take the opportunity to
fix this leak at the same time; and otherwise, propose an upload to
(old)stable-proposed-updates fixing the memory leak.
smcv
>From 8966099e9bef3fd3481f87bb7ad933f5cacad885 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <[email protected]>
Date: Wed, 8 May 2024 22:53:51 +0200
Subject: [PATCH] gdbusmessage: Clean the cached arg0 when setting the message
body
We're now caching arg0 but such value is not cleared when a new body is
set as it's in the connection filter test cases where we've a leak as
highlighted by both valgrind and leak sanitizer
---
gio/gdbusmessage.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/gio/gdbusmessage.c b/gio/gdbusmessage.c
index 0b77dc607..331e68d45 100644
--- a/gio/gdbusmessage.c
+++ b/gio/gdbusmessage.c
@@ -1155,10 +1155,12 @@ g_dbus_message_set_body (GDBusMessage *message,
if (message->body != NULL)
g_variant_unref (message->body);
+
+ g_clear_pointer (&message->arg0_cache, g_variant_unref);
+
if (body == NULL)
{
message->body = NULL;
- message->arg0_cache = NULL;
g_dbus_message_set_signature (message, NULL);
}
else
@@ -1172,8 +1174,6 @@ g_dbus_message_set_body (GDBusMessage *message,
if (g_variant_is_of_type (message->body, G_VARIANT_TYPE_TUPLE) &&
g_variant_n_children (message->body) > 0)
message->arg0_cache = g_variant_get_child_value (message->body, 0);
- else
- message->arg0_cache = NULL;
type_string = g_variant_get_type_string (body);
type_string_len = strlen (type_string);
--
2.39.2
