MSVSphere Packaging Team
1 year ago
parent
3de200ec16
commit
bc5c2ea1da
@ -0,0 +1,278 @@
|
||||
From 764f071909df70622e79ee71323973c18c055c8c Mon Sep 17 00:00:00 2001
|
||||
From: Giuseppe Scrivano <giuseppe@scrivano.org>
|
||||
Date: Mon, 14 Sep 2020 16:28:10 +0200
|
||||
Subject: [PATCH 1/5] gdbusauth: empty DATA does not need a trailing space
|
||||
|
||||
This is an interoperability fix. If the line is exactly "DATA\r\n",
|
||||
the reference implementation of D-Bus treats this as equivalent to
|
||||
"DATA \r\n", meaning the data block consists of zero hex-encoded bytes.
|
||||
In practice, D-Bus clients send empty data blocks as "DATA\r\n", and
|
||||
in fact sd-bus only accepts that, rejecting "DATA \r\n".
|
||||
|
||||
[Originally part of a larger commit; commit message added by smcv]
|
||||
|
||||
Signed-off-by: Giuseppe Scrivano <giuseppe@scrivano.org>
|
||||
Co-authored-by: Simon McVittie <smcv@collabora.com>
|
||||
Signed-off-by: Simon McVittie <smcv@collabora.com>
|
||||
---
|
||||
gio/gdbusauth.c | 8 ++++----
|
||||
1 file changed, 4 insertions(+), 4 deletions(-)
|
||||
|
||||
diff --git a/gio/gdbusauth.c b/gio/gdbusauth.c
|
||||
index ede21c8514..d2ca41a201 100644
|
||||
--- a/gio/gdbusauth.c
|
||||
+++ b/gio/gdbusauth.c
|
||||
@@ -783,13 +783,13 @@ _g_dbus_auth_run_client (GDBusAuth *auth,
|
||||
if (line == NULL)
|
||||
goto out;
|
||||
debug_print ("CLIENT: WaitingForData, read='%s'", line);
|
||||
- if (g_str_has_prefix (line, "DATA "))
|
||||
+ if (g_str_equal (line, "DATA") || g_str_has_prefix (line, "DATA "))
|
||||
{
|
||||
gchar *encoded;
|
||||
gchar *decoded_data;
|
||||
gsize decoded_data_len = 0;
|
||||
|
||||
- encoded = g_strdup (line + 5);
|
||||
+ encoded = g_strdup (line + 4);
|
||||
g_free (line);
|
||||
g_strstrip (encoded);
|
||||
decoded_data = hexdecode (encoded, &decoded_data_len, error);
|
||||
@@ -1255,13 +1255,13 @@ _g_dbus_auth_run_server (GDBusAuth *auth,
|
||||
debug_print ("SERVER: WaitingForData, read '%s'", line);
|
||||
if (line == NULL)
|
||||
goto out;
|
||||
- if (g_str_has_prefix (line, "DATA "))
|
||||
+ if (g_str_equal (line, "DATA") || g_str_has_prefix (line, "DATA "))
|
||||
{
|
||||
gchar *encoded;
|
||||
gchar *decoded_data;
|
||||
gsize decoded_data_len = 0;
|
||||
|
||||
- encoded = g_strdup (line + 5);
|
||||
+ encoded = g_strdup (line + 4);
|
||||
g_free (line);
|
||||
g_strstrip (encoded);
|
||||
decoded_data = hexdecode (encoded, &decoded_data_len, error);
|
||||
--
|
||||
GitLab
|
||||
|
||||
|
||||
From a7d2e727eefcf883bb463ad559f5632e8e448757 Mon Sep 17 00:00:00 2001
|
||||
From: Giuseppe Scrivano <giuseppe@scrivano.org>
|
||||
Date: Mon, 14 Sep 2020 16:28:10 +0200
|
||||
Subject: [PATCH 2/5] GDBusServer: If no initial response for EXTERNAL, send a
|
||||
challenge
|
||||
|
||||
Sending an "initial response" along with the AUTH command is meant
|
||||
to be an optional optimization, and clients are allowed to omit it.
|
||||
We must reply with our initial challenge, which in the case of EXTERNAL
|
||||
is an empty string: the client responds to that with the authorization
|
||||
identity.
|
||||
|
||||
If we do not reply to the AUTH command, then the client will wait
|
||||
forever for our reply, while we wait forever for the reply that we
|
||||
expect the client to send, resulting in deadlock.
|
||||
|
||||
D-Bus does not have a way to distinguish between an empty initial
|
||||
response and the absence of an initial response, so clients that want
|
||||
to use an empty authorization identity, such as systed's sd-bus,
|
||||
cannot use the initial-response optimization and will fail to connect
|
||||
to a GDBusServer that does not have this change.
|
||||
|
||||
[Originally part of a larger commit; commit message added by smcv.]
|
||||
|
||||
Signed-off-by: Simon McVittie <smcv@collabora.com>
|
||||
---
|
||||
gio/gdbusauthmechanismexternal.c | 23 ++++++++++++++++++-----
|
||||
1 file changed, 18 insertions(+), 5 deletions(-)
|
||||
|
||||
diff --git a/gio/gdbusauthmechanismexternal.c b/gio/gdbusauthmechanismexternal.c
|
||||
index 617fe1d0e5..ddd06cbd5e 100644
|
||||
--- a/gio/gdbusauthmechanismexternal.c
|
||||
+++ b/gio/gdbusauthmechanismexternal.c
|
||||
@@ -40,6 +40,7 @@ struct _GDBusAuthMechanismExternalPrivate
|
||||
gboolean is_client;
|
||||
gboolean is_server;
|
||||
GDBusAuthMechanismState state;
|
||||
+ gboolean empty_data_sent;
|
||||
};
|
||||
|
||||
static gint mechanism_get_priority (void);
|
||||
@@ -253,7 +254,9 @@ mechanism_server_initiate (GDBusAuthMechanism *mechanism,
|
||||
}
|
||||
else
|
||||
{
|
||||
- m->priv->state = G_DBUS_AUTH_MECHANISM_STATE_WAITING_FOR_DATA;
|
||||
+ /* The initial-response optimization was not used, so we need to
|
||||
+ * send an empty challenge to prompt the client to respond. */
|
||||
+ m->priv->state = G_DBUS_AUTH_MECHANISM_STATE_HAVE_DATA_TO_SEND;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -288,12 +291,22 @@ mechanism_server_data_send (GDBusAuthMechanism *mechanism,
|
||||
|
||||
g_return_val_if_fail (G_IS_DBUS_AUTH_MECHANISM_EXTERNAL (mechanism), NULL);
|
||||
g_return_val_if_fail (m->priv->is_server && !m->priv->is_client, NULL);
|
||||
- g_return_val_if_fail (m->priv->state == G_DBUS_AUTH_MECHANISM_STATE_HAVE_DATA_TO_SEND, NULL);
|
||||
|
||||
- /* can never end up here because we are never in the HAVE_DATA_TO_SEND state */
|
||||
- g_assert_not_reached ();
|
||||
+ if (out_data_len)
|
||||
+ *out_data_len = 0;
|
||||
|
||||
- return NULL;
|
||||
+ if (m->priv->empty_data_sent)
|
||||
+ {
|
||||
+ /* We have already sent an empty data response.
|
||||
+ Reject the connection. */
|
||||
+ m->priv->state = G_DBUS_AUTH_MECHANISM_STATE_REJECTED;
|
||||
+ return NULL;
|
||||
+ }
|
||||
+
|
||||
+ m->priv->state = G_DBUS_AUTH_MECHANISM_STATE_WAITING_FOR_DATA;
|
||||
+ m->priv->empty_data_sent = TRUE;
|
||||
+
|
||||
+ return g_strdup ("");
|
||||
}
|
||||
|
||||
static gchar *
|
||||
--
|
||||
GitLab
|
||||
|
||||
|
||||
From b51e3ab09e39c590c65a7be6228ecfa48a6189f6 Mon Sep 17 00:00:00 2001
|
||||
From: Giuseppe Scrivano <giuseppe@scrivano.org>
|
||||
Date: Mon, 14 Sep 2020 16:28:10 +0200
|
||||
Subject: [PATCH 3/5] GDBusServer: Accept empty authorization identity for
|
||||
EXTERNAL mechanism
|
||||
|
||||
RFC 4422 appendix A defines the empty authorization identity to mean
|
||||
the identity that the server associated with its authentication
|
||||
credentials. In this case, this means whatever uid is in the
|
||||
GCredentials object.
|
||||
|
||||
In particular, this means that clients in a different Linux user
|
||||
namespace can authenticate against our server and will be authorized
|
||||
as the version of their uid that is visible in the server's namespace,
|
||||
even if the corresponding numeric uid returned by geteuid() in the
|
||||
client's namespace was different. systemd's sd-bus has relied on this
|
||||
since commit
|
||||
https://github.com/systemd/systemd/commit/1ed4723d38cd0d1423c8fe650f90fa86007ddf55.
|
||||
|
||||
[Originally part of a larger commit; commit message added by smcv]
|
||||
|
||||
Signed-off-by: Simon McVittie <smcv@collabora.com>
|
||||
---
|
||||
gio/gdbusauthmechanismexternal.c | 16 +++++++++++++---
|
||||
1 file changed, 13 insertions(+), 3 deletions(-)
|
||||
|
||||
diff --git a/gio/gdbusauthmechanismexternal.c b/gio/gdbusauthmechanismexternal.c
|
||||
index ddd06cbd5e..a465862d12 100644
|
||||
--- a/gio/gdbusauthmechanismexternal.c
|
||||
+++ b/gio/gdbusauthmechanismexternal.c
|
||||
@@ -201,14 +201,24 @@ data_matches_credentials (const gchar *data,
|
||||
if (credentials == NULL)
|
||||
goto out;
|
||||
|
||||
- if (data == NULL || data_len == 0)
|
||||
- goto out;
|
||||
-
|
||||
#if defined(G_OS_UNIX)
|
||||
{
|
||||
gint64 alleged_uid;
|
||||
gchar *endp;
|
||||
|
||||
+ /* If we were unable to find out the uid, then nothing
|
||||
+ * can possibly match it. */
|
||||
+ if (g_credentials_get_unix_user (credentials, NULL) == (uid_t) -1)
|
||||
+ goto out;
|
||||
+
|
||||
+ /* An empty authorization identity means we want to be
|
||||
+ * whatever identity the out-of-band credentials say we have
|
||||
+ * (RFC 4422 appendix A.1). This effectively matches any uid. */
|
||||
+ if (data == NULL || data_len == 0)
|
||||
+ {
|
||||
+ match = TRUE;
|
||||
+ goto out;
|
||||
+ }
|
||||
/* on UNIX, this is the uid as a string in base 10 */
|
||||
alleged_uid = g_ascii_strtoll (data, &endp, 10);
|
||||
if (*endp == '\0')
|
||||
--
|
||||
GitLab
|
||||
|
||||
|
||||
From 3f532af65c98e4ba8426c53f26c9ee15d3692f9c Mon Sep 17 00:00:00 2001
|
||||
From: Simon McVittie <smcv@collabora.com>
|
||||
Date: Mon, 18 Jul 2022 17:14:44 +0100
|
||||
Subject: [PATCH 4/5] gdbusauth: Represent empty data block as DATA\r\n, with
|
||||
no space
|
||||
|
||||
This is an interoperability fix. The reference implementation of D-Bus
|
||||
treats "DATA\r\n" as equivalent to "DATA \r\n", but sd-bus does not,
|
||||
and only accepts the former.
|
||||
|
||||
Signed-off-by: Simon McVittie <smcv@collabora.com>
|
||||
---
|
||||
gio/gdbusauth.c | 34 ++++++++++++++++++++++++++--------
|
||||
1 file changed, 26 insertions(+), 8 deletions(-)
|
||||
|
||||
diff --git a/gio/gdbusauth.c b/gio/gdbusauth.c
|
||||
index d2ca41a201..89cbbf67c6 100644
|
||||
--- a/gio/gdbusauth.c
|
||||
+++ b/gio/gdbusauth.c
|
||||
@@ -807,11 +807,21 @@ _g_dbus_auth_run_client (GDBusAuth *auth,
|
||||
{
|
||||
gchar *data;
|
||||
gsize data_len;
|
||||
- gchar *encoded_data;
|
||||
+
|
||||
data = _g_dbus_auth_mechanism_client_data_send (mech, &data_len);
|
||||
- encoded_data = _g_dbus_hexencode (data, data_len);
|
||||
- s = g_strdup_printf ("DATA %s\r\n", encoded_data);
|
||||
- g_free (encoded_data);
|
||||
+
|
||||
+ if (data_len == 0)
|
||||
+ {
|
||||
+ s = g_strdup ("DATA\r\n");
|
||||
+ }
|
||||
+ else
|
||||
+ {
|
||||
+ gchar *encoded_data = _g_dbus_hexencode (data, data_len);
|
||||
+
|
||||
+ s = g_strdup_printf ("DATA %s\r\n", encoded_data);
|
||||
+ g_free (encoded_data);
|
||||
+ }
|
||||
+
|
||||
g_free (data);
|
||||
debug_print ("CLIENT: writing '%s'", s);
|
||||
if (!g_data_output_stream_put_string (dos, s, cancellable, error))
|
||||
@@ -1209,13 +1219,21 @@ _g_dbus_auth_run_server (GDBusAuth *auth,
|
||||
gsize data_len;
|
||||
|
||||
data = _g_dbus_auth_mechanism_server_data_send (mech, &data_len);
|
||||
+
|
||||
if (data != NULL)
|
||||
{
|
||||
- gchar *encoded_data;
|
||||
+ if (data_len == 0)
|
||||
+ {
|
||||
+ s = g_strdup ("DATA\r\n");
|
||||
+ }
|
||||
+ else
|
||||
+ {
|
||||
+ gchar *encoded_data = _g_dbus_hexencode (data, data_len);
|
||||
+
|
||||
+ s = g_strdup_printf ("DATA %s\r\n", encoded_data);
|
||||
+ g_free (encoded_data);
|
||||
+ }
|
||||
|
||||
- encoded_data = _g_dbus_hexencode (data, data_len);
|
||||
- s = g_strdup_printf ("DATA %s\r\n", encoded_data);
|
||||
- g_free (encoded_data);
|
||||
g_free (data);
|
||||
|
||||
debug_print ("SERVER: writing '%s'", s);
|
||||
--
|
||||
GitLab
|
||||
File diff suppressed because it is too large
Load Diff
@ -0,0 +1,65 @@
|
||||
From ba2137b0d9ea3744155be81a5ba770c6535b46f3 Mon Sep 17 00:00:00 2001
|
||||
From: Simon McVittie <smcv@collabora.com>
|
||||
Date: Thu, 15 Dec 2022 12:51:37 +0000
|
||||
Subject: [PATCH] gvariant-serialiser: Convert endianness of offsets
|
||||
MIME-Version: 1.0
|
||||
Content-Type: text/plain; charset=UTF-8
|
||||
Content-Transfer-Encoding: 8bit
|
||||
|
||||
The array of offsets is little-endian, even on big-endian architectures
|
||||
like s390x.
|
||||
|
||||
Fixes: ade71fb5 "gvariant: Don’t allow child elements to overlap with each other"
|
||||
Resolves: https://gitlab.gnome.org/GNOME/glib/-/issues/2839
|
||||
Signed-off-by: Simon McVittie <smcv@collabora.com>
|
||||
---
|
||||
glib/gvariant-serialiser.c | 19 +++++++++++--------
|
||||
1 file changed, 11 insertions(+), 8 deletions(-)
|
||||
|
||||
diff --git a/glib/gvariant-serialiser.c b/glib/gvariant-serialiser.c
|
||||
index fadefab659..f443c2eb85 100644
|
||||
--- a/glib/gvariant-serialiser.c
|
||||
+++ b/glib/gvariant-serialiser.c
|
||||
@@ -714,17 +714,19 @@ gvs_variable_sized_array_n_children (GVariantSerialised value)
|
||||
/* Find the index of the first out-of-order element in @data, assuming that
|
||||
* @data is an array of elements of given @type, starting at index @start and
|
||||
* containing a further @len-@start elements. */
|
||||
-#define DEFINE_FIND_UNORDERED(type) \
|
||||
+#define DEFINE_FIND_UNORDERED(type, le_to_native) \
|
||||
static gsize \
|
||||
find_unordered_##type (const guint8 *data, gsize start, gsize len) \
|
||||
{ \
|
||||
gsize off; \
|
||||
- type current, previous; \
|
||||
+ type current_le, previous_le, current, previous; \
|
||||
\
|
||||
- memcpy (&previous, data + start * sizeof (current), sizeof (current)); \
|
||||
+ memcpy (&previous_le, data + start * sizeof (current), sizeof (current)); \
|
||||
+ previous = le_to_native (previous_le); \
|
||||
for (off = (start + 1) * sizeof (current); off < len * sizeof (current); off += sizeof (current)) \
|
||||
{ \
|
||||
- memcpy (¤t, data + off, sizeof (current)); \
|
||||
+ memcpy (¤t_le, data + off, sizeof (current)); \
|
||||
+ current = le_to_native (current_le); \
|
||||
if (current < previous) \
|
||||
break; \
|
||||
previous = current; \
|
||||
@@ -732,10 +734,11 @@ gvs_variable_sized_array_n_children (GVariantSerialised value)
|
||||
return off / sizeof (current) - 1; \
|
||||
}
|
||||
|
||||
-DEFINE_FIND_UNORDERED (guint8);
|
||||
-DEFINE_FIND_UNORDERED (guint16);
|
||||
-DEFINE_FIND_UNORDERED (guint32);
|
||||
-DEFINE_FIND_UNORDERED (guint64);
|
||||
+#define NO_CONVERSION(x) (x)
|
||||
+DEFINE_FIND_UNORDERED (guint8, NO_CONVERSION);
|
||||
+DEFINE_FIND_UNORDERED (guint16, GUINT16_FROM_LE);
|
||||
+DEFINE_FIND_UNORDERED (guint32, GUINT32_FROM_LE);
|
||||
+DEFINE_FIND_UNORDERED (guint64, GUINT64_FROM_LE);
|
||||
|
||||
static GVariantSerialised
|
||||
gvs_variable_sized_array_get_child (GVariantSerialised value,
|
||||
--
|
||||
GitLab
|
||||
|
||||
@ -0,0 +1,199 @@
|
||||
From 78da5faccb3e065116b75b3ff87ff55381da6c76 Mon Sep 17 00:00:00 2001
|
||||
From: Philip Withnall <pwithnall@endlessos.org>
|
||||
Date: Thu, 15 Dec 2022 13:00:39 +0000
|
||||
Subject: [PATCH 1/2] =?UTF-8?q?gvariant:=20Check=20offset=20table=20doesn?=
|
||||
=?UTF-8?q?=E2=80=99t=20fall=20outside=20variant=20bounds?=
|
||||
MIME-Version: 1.0
|
||||
Content-Type: text/plain; charset=UTF-8
|
||||
Content-Transfer-Encoding: 8bit
|
||||
|
||||
When dereferencing the first entry in the offset table for a tuple,
|
||||
check that it doesn’t fall outside the bounds of the variant first.
|
||||
|
||||
This prevents an out-of-bounds read from some non-normal tuples.
|
||||
|
||||
This bug was introduced in commit 73d0aa81c2575a5c9ae77d.
|
||||
|
||||
Includes a unit test, although the test will likely only catch the
|
||||
original bug if run with asan enabled.
|
||||
|
||||
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
|
||||
|
||||
Fixes: #2840
|
||||
oss-fuzz#54302
|
||||
---
|
||||
glib/gvariant-serialiser.c | 12 ++++++--
|
||||
glib/tests/gvariant.c | 63 ++++++++++++++++++++++++++++++++++++++
|
||||
2 files changed, 72 insertions(+), 3 deletions(-)
|
||||
|
||||
diff --git a/glib/gvariant-serialiser.c b/glib/gvariant-serialiser.c
|
||||
index f443c2eb85..4e4a73ad17 100644
|
||||
--- a/glib/gvariant-serialiser.c
|
||||
+++ b/glib/gvariant-serialiser.c
|
||||
@@ -984,7 +984,8 @@ gvs_tuple_get_member_bounds (GVariantSerialised value,
|
||||
|
||||
member_info = g_variant_type_info_member_info (value.type_info, index_);
|
||||
|
||||
- if (member_info->i + 1)
|
||||
+ if (member_info->i + 1 &&
|
||||
+ offset_size * (member_info->i + 1) <= value.size)
|
||||
member_start = gvs_read_unaligned_le (value.data + value.size -
|
||||
offset_size * (member_info->i + 1),
|
||||
offset_size);
|
||||
@@ -995,7 +996,8 @@ gvs_tuple_get_member_bounds (GVariantSerialised value,
|
||||
member_start &= member_info->b;
|
||||
member_start |= member_info->c;
|
||||
|
||||
- if (member_info->ending_type == G_VARIANT_MEMBER_ENDING_LAST)
|
||||
+ if (member_info->ending_type == G_VARIANT_MEMBER_ENDING_LAST &&
|
||||
+ offset_size * (member_info->i + 1) <= value.size)
|
||||
member_end = value.size - offset_size * (member_info->i + 1);
|
||||
|
||||
else if (member_info->ending_type == G_VARIANT_MEMBER_ENDING_FIXED)
|
||||
@@ -1006,11 +1008,15 @@ gvs_tuple_get_member_bounds (GVariantSerialised value,
|
||||
member_end = member_start + fixed_size;
|
||||
}
|
||||
|
||||
- else /* G_VARIANT_MEMBER_ENDING_OFFSET */
|
||||
+ else if (member_info->ending_type == G_VARIANT_MEMBER_ENDING_OFFSET &&
|
||||
+ offset_size * (member_info->i + 2) <= value.size)
|
||||
member_end = gvs_read_unaligned_le (value.data + value.size -
|
||||
offset_size * (member_info->i + 2),
|
||||
offset_size);
|
||||
|
||||
+ else /* invalid */
|
||||
+ member_end = G_MAXSIZE;
|
||||
+
|
||||
if (out_member_start != NULL)
|
||||
*out_member_start = member_start;
|
||||
if (out_member_end != NULL)
|
||||
diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c
|
||||
index b360888e4d..98c51a1d75 100644
|
||||
--- a/glib/tests/gvariant.c
|
||||
+++ b/glib/tests/gvariant.c
|
||||
@@ -5576,6 +5576,67 @@ test_normal_checking_tuple_offsets4 (void)
|
||||
g_variant_unref (variant);
|
||||
}
|
||||
|
||||
+/* This is a regression test that dereferencing the first element in the offset
|
||||
+ * table doesn’t dereference memory before the start of the GVariant. The first
|
||||
+ * element in the offset table gives the offset of the final member in the
|
||||
+ * tuple (the offset table is stored in reverse), and the position of this final
|
||||
+ * member is needed to check that none of the tuple members overlap with the
|
||||
+ * offset table
|
||||
+ *
|
||||
+ * See https://gitlab.gnome.org/GNOME/glib/-/issues/2840 */
|
||||
+static void
|
||||
+test_normal_checking_tuple_offsets5 (void)
|
||||
+{
|
||||
+ /* A tuple of type (sss) in normal form would have an offset table with two
|
||||
+ * entries:
|
||||
+ * - The first entry (lowest index in the table) gives the offset of the
|
||||
+ * third `s` in the tuple, as the offset table is reversed compared to the
|
||||
+ * tuple members.
|
||||
+ * - The second entry (highest index in the table) gives the offset of the
|
||||
+ * second `s` in the tuple.
|
||||
+ * - The offset of the first `s` in the tuple is always 0.
|
||||
+ *
|
||||
+ * See §2.5.4 (Structures) of the GVariant specification for details, noting
|
||||
+ * that the table is only layed out this way because all three members of the
|
||||
+ * tuple have non-fixed sizes.
|
||||
+ *
|
||||
+ * It’s not clear whether the 0xaa data of this variant is part of the strings
|
||||
+ * in the tuple, or part of the offset table. It doesn’t really matter. This
|
||||
+ * is a regression test to check that the code to validate the offset table
|
||||
+ * doesn’t unconditionally try to access the first entry in the offset table
|
||||
+ * by subtracting the table size from the end of the GVariant data.
|
||||
+ *
|
||||
+ * In this non-normal case, that would result in an address off the start of
|
||||
+ * the GVariant data, and an out-of-bounds read, because the GVariant is one
|
||||
+ * byte long, but the offset table is calculated as two bytes long (with 1B
|
||||
+ * sized entries) from the tuple’s type.
|
||||
+ */
|
||||
+ const GVariantType *data_type = G_VARIANT_TYPE ("(sss)");
|
||||
+ const guint8 data[] = { 0xaa };
|
||||
+ gsize size = sizeof (data);
|
||||
+ GVariant *variant = NULL;
|
||||
+ GVariant *normal_variant = NULL;
|
||||
+ GVariant *expected = NULL;
|
||||
+
|
||||
+ g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/2840");
|
||||
+
|
||||
+ variant = g_variant_new_from_data (data_type, data, size, FALSE, NULL, NULL);
|
||||
+ g_assert_nonnull (variant);
|
||||
+
|
||||
+ g_assert_false (g_variant_is_normal_form (variant));
|
||||
+
|
||||
+ normal_variant = g_variant_get_normal_form (variant);
|
||||
+ g_assert_nonnull (normal_variant);
|
||||
+
|
||||
+ expected = g_variant_new_parsed ("('', '', '')");
|
||||
+ g_assert_cmpvariant (expected, variant);
|
||||
+ g_assert_cmpvariant (expected, normal_variant);
|
||||
+
|
||||
+ g_variant_unref (expected);
|
||||
+ g_variant_unref (normal_variant);
|
||||
+ g_variant_unref (variant);
|
||||
+}
|
||||
+
|
||||
/* Test that an otherwise-valid serialised GVariant is considered non-normal if
|
||||
* its offset table entries are too wide.
|
||||
*
|
||||
@@ -5827,6 +5888,8 @@ main (int argc, char **argv)
|
||||
test_normal_checking_tuple_offsets3);
|
||||
g_test_add_func ("/gvariant/normal-checking/tuple-offsets4",
|
||||
test_normal_checking_tuple_offsets4);
|
||||
+ g_test_add_func ("/gvariant/normal-checking/tuple-offsets5",
|
||||
+ test_normal_checking_tuple_offsets5);
|
||||
g_test_add_func ("/gvariant/normal-checking/tuple-offsets/minimal-sized",
|
||||
test_normal_checking_tuple_offsets_minimal_sized);
|
||||
g_test_add_func ("/gvariant/normal-checking/empty-object-path",
|
||||
--
|
||||
GitLab
|
||||
|
||||
|
||||
From 21a204147b16539b3eda3143b32844c49e29f4d4 Mon Sep 17 00:00:00 2001
|
||||
From: Philip Withnall <pwithnall@endlessos.org>
|
||||
Date: Thu, 15 Dec 2022 16:49:28 +0000
|
||||
Subject: [PATCH 2/2] gvariant: Propagate trust when getting a child of a
|
||||
serialised variant
|
||||
|
||||
If a variant is trusted, that means all its children are trusted, so
|
||||
ensure that their checked offsets are set as such.
|
||||
|
||||
This allows a lot of the offset table checks to be avoided when getting
|
||||
children from trusted serialised tuples, which speeds things up.
|
||||
|
||||
No unit test is included because this is just a performance fix. If
|
||||
there are other slownesses, or regressions, in serialised `GVariant`
|
||||
performance, the fuzzing setup will catch them like it did this one.
|
||||
|
||||
This change does reduce the time to run the oss-fuzz reproducer from 80s
|
||||
to about 0.7s on my machine.
|
||||
|
||||
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
|
||||
|
||||
Fixes: #2841
|
||||
oss-fuzz#54314
|
||||
---
|
||||
glib/gvariant-core.c | 4 ++--
|
||||
1 file changed, 2 insertions(+), 2 deletions(-)
|
||||
|
||||
diff --git a/glib/gvariant-core.c b/glib/gvariant-core.c
|
||||
index f441c4757e..4778022829 100644
|
||||
--- a/glib/gvariant-core.c
|
||||
+++ b/glib/gvariant-core.c
|
||||
@@ -1198,8 +1198,8 @@ g_variant_get_child_value (GVariant *value,
|
||||
child->contents.serialised.bytes =
|
||||
g_bytes_ref (value->contents.serialised.bytes);
|
||||
child->contents.serialised.data = s_child.data;
|
||||
- child->contents.serialised.ordered_offsets_up_to = s_child.ordered_offsets_up_to;
|
||||
- child->contents.serialised.checked_offsets_up_to = s_child.checked_offsets_up_to;
|
||||
+ child->contents.serialised.ordered_offsets_up_to = (value->state & STATE_TRUSTED) ? G_MAXSIZE : s_child.ordered_offsets_up_to;
|
||||
+ child->contents.serialised.checked_offsets_up_to = (value->state & STATE_TRUSTED) ? G_MAXSIZE : s_child.checked_offsets_up_to;
|
||||
|
||||
return child;
|
||||
}
|
||||
--
|
||||
GitLab
|
||||
|
||||
@ -0,0 +1,141 @@
|
||||
From 059f4f3999f1de506417611318c6f27db57fb689 Mon Sep 17 00:00:00 2001
|
||||
From: Marius Vollmer <mvollmer@redhat.com>
|
||||
Date: Mon, 13 Feb 2023 14:12:52 +0200
|
||||
Subject: [PATCH] gdbus: Never buffer reads during server authentication
|
||||
|
||||
Otherwise, the content of the buffer is thrown away when switching
|
||||
from reading via a GDataInputStream to unbuffered reads when waiting
|
||||
for the "BEGIN" line.
|
||||
|
||||
(The code already tried to protect against over-reading like this by
|
||||
using unbuffered reads for the last few lines of the auth protocol,
|
||||
but it might already be too late at that point. The buffer of the
|
||||
GDataInputStream might already contain the "BEGIN" line for example.)
|
||||
|
||||
This matters when connecting a sd-bus client directly to a GDBus
|
||||
client. A sd-bus client optimistically sends the whole auth
|
||||
conversation in one go without waiting for intermediate replies. This
|
||||
is done to improve performance for the many short-lived connections
|
||||
that are typically made.
|
||||
---
|
||||
gio/gdbusauth.c | 50 ++++++++++++++++++++++++++++++-------------------
|
||||
1 file changed, 31 insertions(+), 19 deletions(-)
|
||||
|
||||
diff --git a/gio/gdbusauth.c b/gio/gdbusauth.c
|
||||
index c430f0cf0..17c7d47b7 100644
|
||||
--- a/gio/gdbusauth.c
|
||||
+++ b/gio/gdbusauth.c
|
||||
@@ -933,7 +933,6 @@ _g_dbus_auth_run_server (GDBusAuth *auth,
|
||||
{
|
||||
gboolean ret;
|
||||
ServerState state;
|
||||
- GDataInputStream *dis;
|
||||
GDataOutputStream *dos;
|
||||
GError *local_error;
|
||||
gchar *line;
|
||||
@@ -949,7 +948,6 @@ _g_dbus_auth_run_server (GDBusAuth *auth,
|
||||
_g_dbus_auth_add_mechs (auth, observer);
|
||||
|
||||
ret = FALSE;
|
||||
- dis = NULL;
|
||||
dos = NULL;
|
||||
mech = NULL;
|
||||
negotiated_capabilities = 0;
|
||||
@@ -965,13 +963,18 @@ _g_dbus_auth_run_server (GDBusAuth *auth,
|
||||
goto out;
|
||||
}
|
||||
|
||||
- dis = G_DATA_INPUT_STREAM (g_data_input_stream_new (g_io_stream_get_input_stream (auth->priv->stream)));
|
||||
+ /* We use an extremely slow (but reliable) line reader for input
|
||||
+ * instead of something buffered - this basically does a recvfrom()
|
||||
+ * system call per character
|
||||
+ *
|
||||
+ * (the problem with using GDataInputStream's read_line is that
|
||||
+ * because of buffering it might start reading into the first D-Bus
|
||||
+ * message that appears after "BEGIN\r\n"....)
|
||||
+ */
|
||||
+
|
||||
dos = G_DATA_OUTPUT_STREAM (g_data_output_stream_new (g_io_stream_get_output_stream (auth->priv->stream)));
|
||||
- g_filter_input_stream_set_close_base_stream (G_FILTER_INPUT_STREAM (dis), FALSE);
|
||||
g_filter_output_stream_set_close_base_stream (G_FILTER_OUTPUT_STREAM (dos), FALSE);
|
||||
|
||||
- g_data_input_stream_set_newline_type (dis, G_DATA_STREAM_NEWLINE_TYPE_CR_LF);
|
||||
-
|
||||
/* read the NUL-byte, possibly with credentials attached */
|
||||
#ifdef G_OS_UNIX
|
||||
#ifndef G_CREDENTIALS_PREFER_MESSAGE_PASSING
|
||||
@@ -1010,11 +1013,22 @@ _g_dbus_auth_run_server (GDBusAuth *auth,
|
||||
}
|
||||
else
|
||||
{
|
||||
+ gchar c;
|
||||
+ gssize num_read;
|
||||
+
|
||||
local_error = NULL;
|
||||
- (void)g_data_input_stream_read_byte (dis, cancellable, &local_error);
|
||||
- if (local_error != NULL)
|
||||
+ num_read = g_input_stream_read (g_io_stream_get_input_stream (auth->priv->stream),
|
||||
+ &c, 1,
|
||||
+ cancellable, &local_error);
|
||||
+ if (num_read != 1 || local_error != NULL)
|
||||
{
|
||||
- g_propagate_error (error, local_error);
|
||||
+ if (local_error == NULL)
|
||||
+ g_set_error_literal (error,
|
||||
+ G_IO_ERROR,
|
||||
+ G_IO_ERROR_FAILED,
|
||||
+ _ ("Unexpected lack of content trying to read a byte"));
|
||||
+ else
|
||||
+ g_propagate_error (error, local_error);
|
||||
goto out;
|
||||
}
|
||||
}
|
||||
@@ -1050,7 +1064,10 @@ _g_dbus_auth_run_server (GDBusAuth *auth,
|
||||
{
|
||||
case SERVER_STATE_WAITING_FOR_AUTH:
|
||||
debug_print ("SERVER: WaitingForAuth");
|
||||
- line = _my_g_data_input_stream_read_line (dis, &line_length, cancellable, error);
|
||||
+ line = _my_g_input_stream_read_line_safe (g_io_stream_get_input_stream (auth->priv->stream),
|
||||
+ &line_length,
|
||||
+ cancellable,
|
||||
+ error);
|
||||
debug_print ("SERVER: WaitingForAuth, read '%s'", line);
|
||||
if (line == NULL)
|
||||
goto out;
|
||||
@@ -1260,7 +1277,10 @@ _g_dbus_auth_run_server (GDBusAuth *auth,
|
||||
|
||||
case SERVER_STATE_WAITING_FOR_DATA:
|
||||
debug_print ("SERVER: WaitingForData");
|
||||
- line = _my_g_data_input_stream_read_line (dis, &line_length, cancellable, error);
|
||||
+ line = _my_g_input_stream_read_line_safe (g_io_stream_get_input_stream (auth->priv->stream),
|
||||
+ &line_length,
|
||||
+ cancellable,
|
||||
+ error);
|
||||
debug_print ("SERVER: WaitingForData, read '%s'", line);
|
||||
if (line == NULL)
|
||||
goto out;
|
||||
@@ -1299,13 +1319,6 @@ _g_dbus_auth_run_server (GDBusAuth *auth,
|
||||
|
||||
case SERVER_STATE_WAITING_FOR_BEGIN:
|
||||
debug_print ("SERVER: WaitingForBegin");
|
||||
- /* Use extremely slow (but reliable) line reader - this basically
|
||||
- * does a recvfrom() system call per character
|
||||
- *
|
||||
- * (the problem with using GDataInputStream's read_line is that because of
|
||||
- * buffering it might start reading into the first D-Bus message that
|
||||
- * appears after "BEGIN\r\n"....)
|
||||
- */
|
||||
line = _my_g_input_stream_read_line_safe (g_io_stream_get_input_stream (auth->priv->stream),
|
||||
&line_length,
|
||||
cancellable,
|
||||
@@ -1364,7 +1377,6 @@ _g_dbus_auth_run_server (GDBusAuth *auth,
|
||||
|
||||
out:
|
||||
g_clear_object (&mech);
|
||||
- g_clear_object (&dis);
|
||||
g_clear_object (&dos);
|
||||
g_clear_object (&own_credentials);
|
||||
|
||||
--
|
||||
2.41.0
|
||||
|
||||
Loading…
Reference in new issue