Merge branch 'backport-1201-gdbus-recursion-glib-2-62' into 'glib-2-62'
Backport !1201 “gdbusmessage: Limit recursion of variants in D-Bus messages” to glib-2-62
See merge request GNOME/glib!1290
diff --git a/gio/gdbusmessage.c b/gio/gdbusmessage.c
index 852a704..6e3bd8b 100644
--- a/gio/gdbusmessage.c
+++ b/gio/gdbusmessage.c
@@ -58,6 +58,10 @@
#include "glibintl.h"
+/* See https://dbus.freedesktop.org/doc/dbus-specification.html#message-protocol-marshaling-signature
+ * This is 64 containers plus 1 value within them. */
+#define G_DBUS_MAX_TYPE_DEPTH (64 + 1)
+
typedef struct _GMemoryBuffer GMemoryBuffer;
struct _GMemoryBuffer
{
@@ -1439,17 +1443,27 @@
static GVariant *
parse_value_from_blob (GMemoryBuffer *buf,
const GVariantType *type,
+ guint max_depth,
gboolean just_align,
guint indent,
GError **error)
{
- GVariant *ret;
- GError *local_error;
+ GVariant *ret = NULL;
+ GError *local_error = NULL;
#ifdef DEBUG_SERIALIZER
gboolean is_leaf;
#endif /* DEBUG_SERIALIZER */
const gchar *type_string;
+ if (max_depth == 0)
+ {
+ g_set_error_literal (&local_error,
+ G_IO_ERROR,
+ G_IO_ERROR_INVALID_ARGUMENT,
+ _("Value nested too deeply"));
+ goto fail;
+ }
+
type_string = g_variant_type_peek_string (type);
#ifdef DEBUG_SERIALIZER
@@ -1465,12 +1479,9 @@
}
#endif /* DEBUG_SERIALIZER */
- ret = NULL;
-
#ifdef DEBUG_SERIALIZER
is_leaf = TRUE;
#endif /* DEBUG_SERIALIZER */
- local_error = NULL;
switch (type_string[0])
{
case 'b': /* G_VARIANT_TYPE_BOOLEAN */
@@ -1690,6 +1701,17 @@
goto fail;
}
+ if (max_depth == 1)
+ {
+ /* If we had recursed into parse_value_from_blob() again to
+ * parse the array values, this would have been emitted. */
+ g_set_error_literal (&local_error,
+ G_IO_ERROR,
+ G_IO_ERROR_INVALID_ARGUMENT,
+ _("Value nested too deeply"));
+ goto fail;
+ }
+
ensure_input_padding (buf, fixed_size);
array_data = read_bytes (buf, array_len, &local_error);
if (array_data == NULL)
@@ -1717,6 +1739,7 @@
GVariant *item G_GNUC_UNUSED /* when compiling with G_DISABLE_ASSERT */;
item = parse_value_from_blob (buf,
element_type,
+ max_depth - 1,
TRUE,
indent + 2,
NULL);
@@ -1731,6 +1754,7 @@
GVariant *item;
item = parse_value_from_blob (buf,
element_type,
+ max_depth - 1,
FALSE,
indent + 2,
&local_error);
@@ -1770,6 +1794,7 @@
key_type = g_variant_type_key (type);
key = parse_value_from_blob (buf,
key_type,
+ max_depth - 1,
FALSE,
indent + 2,
&local_error);
@@ -1778,6 +1803,7 @@
value_type = g_variant_type_value (type);
value = parse_value_from_blob (buf,
value_type,
+ max_depth - 1,
FALSE,
indent + 2,
&local_error);
@@ -1812,6 +1838,7 @@
GVariant *item;
item = parse_value_from_blob (buf,
element_type,
+ max_depth - 1,
FALSE,
indent + 2,
&local_error);
@@ -1858,9 +1885,26 @@
sig);
goto fail;
}
+
+ if (max_depth <= g_variant_type_string_get_depth_ (sig))
+ {
+ /* Catch the type nesting being too deep without having to
+ * parse the data. We don’t have to check this for static
+ * container types (like arrays and tuples, above) because
+ * the g_variant_type_string_is_valid() check performed before
+ * the initial parse_value_from_blob() call should check the
+ * static type nesting. */
+ g_set_error_literal (&local_error,
+ G_IO_ERROR,
+ G_IO_ERROR_INVALID_ARGUMENT,
+ _("Value nested too deeply"));
+ goto fail;
+ }
+
variant_type = g_variant_type_new (sig);
value = parse_value_from_blob (buf,
variant_type,
+ max_depth - 1,
FALSE,
indent + 2,
&local_error);
@@ -2098,6 +2142,7 @@
#endif /* DEBUG_SERIALIZER */
headers = parse_value_from_blob (&mbuf,
G_VARIANT_TYPE ("a{yv}"),
+ G_DBUS_MAX_TYPE_DEPTH + 2 /* for the a{yv} */,
FALSE,
2,
error);
@@ -2169,6 +2214,7 @@
#endif /* DEBUG_SERIALIZER */
message->body = parse_value_from_blob (&mbuf,
variant_type,
+ G_DBUS_MAX_TYPE_DEPTH + 1 /* for the surrounding tuple */,
FALSE,
2,
error);
diff --git a/gio/tests/gdbus-serialization.c b/gio/tests/gdbus-serialization.c
index 89a2e97..2ca28d9 100644
--- a/gio/tests/gdbus-serialization.c
+++ b/gio/tests/gdbus-serialization.c
@@ -514,9 +514,8 @@
}
/* If @value is floating, this assumes ownership. */
-static void
-check_serialization (GVariant *value,
- const gchar *expected_dbus_1_output)
+static gchar *
+get_and_check_serialization (GVariant *value)
{
guchar *blob;
gsize blob_size;
@@ -525,7 +524,7 @@
GDBusMessage *recovered_message;
GError *error;
DBusError dbus_error;
- gchar *s;
+ gchar *s = NULL;
guint n;
message = g_dbus_message_new ();
@@ -597,9 +596,6 @@
s = dbus_1_message_print (dbus_1_message);
dbus_message_unref (dbus_1_message);
- g_assert_cmpstr (s, ==, expected_dbus_1_output);
- g_free (s);
-
/* Then serialize back and check that the body is identical */
error = NULL;
@@ -624,10 +620,22 @@
}
g_object_unref (message);
+
+ return g_steal_pointer (&s);
+}
+
+/* If @value is floating, this assumes ownership. */
+static void
+check_serialization (GVariant *value,
+ const gchar *expected_dbus_1_output)
+{
+ gchar *s = get_and_check_serialization (value);
+ g_assert_cmpstr (s, ==, expected_dbus_1_output);
+ g_free (s);
}
static void
-message_serialize_basic (void)
+test_message_serialize_basic (void)
{
check_serialization (NULL, "");
@@ -661,10 +669,12 @@
/* ---------------------------------------------------------------------------------------------------- */
static void
-message_serialize_complex (void)
+test_message_serialize_complex (void)
{
GError *error;
GVariant *value;
+ guint i;
+ gchar *serialization = NULL;
error = NULL;
@@ -724,6 +734,37 @@
" unix-fd: (not extracted)\n");
g_variant_unref (value);
#endif
+
+ /* Deep nesting of variants (just below the recursion limit). */
+ value = g_variant_new_string ("buried");
+ for (i = 0; i < 64; i++)
+ value = g_variant_new_variant (value);
+ value = g_variant_new_tuple (&value, 1);
+
+ serialization = get_and_check_serialization (value);
+ g_assert_nonnull (serialization);
+ g_assert_true (g_str_has_prefix (serialization,
+ "value 0: variant:\n"
+ " variant:\n"
+ " variant:\n"));
+ g_free (serialization);
+
+ /* Deep nesting of arrays and structs (just below the recursion limit).
+ * See https://dbus.freedesktop.org/doc/dbus-specification.html#message-protocol-marshaling-signature */
+ value = g_variant_new_string ("hello");
+ for (i = 0; i < 32; i++)
+ value = g_variant_new_tuple (&value, 1);
+ for (i = 0; i < 32; i++)
+ value = g_variant_new_array (NULL, &value, 1);
+ value = g_variant_new_tuple (&value, 1);
+
+ serialization = get_and_check_serialization (value);
+ g_assert_nonnull (serialization);
+ g_assert_true (g_str_has_prefix (serialization,
+ "value 0: array:\n"
+ " array:\n"
+ " array:\n"));
+ g_free (serialization);
}
@@ -749,7 +790,7 @@
}
static void
-message_serialize_invalid (void)
+test_message_serialize_invalid (void)
{
guint n;
@@ -842,7 +883,7 @@
/* ---------------------------------------------------------------------------------------------------- */
static void
-message_serialize_header_checks (void)
+test_message_serialize_header_checks (void)
{
GDBusMessage *message;
GDBusMessage *reply;
@@ -996,7 +1037,7 @@
/* ---------------------------------------------------------------------------------------------------- */
static void
-message_parse_empty_arrays_of_arrays (void)
+test_message_parse_empty_arrays_of_arrays (void)
{
GVariant *body;
GError *error = NULL;
@@ -1052,7 +1093,7 @@
/* ---------------------------------------------------------------------------------------------------- */
static void
-test_double_array (void)
+test_message_serialize_double_array (void)
{
GVariantBuilder builder;
GVariant *body;
@@ -1252,6 +1293,126 @@
/* ---------------------------------------------------------------------------------------------------- */
+/* Test that an invalid header in a D-Bus message (specifically, containing too
+ * many levels of nested variant) is gracefully handled with an error rather
+ * than a crash. The set of bytes here come almost directly from fuzzer output. */
+static void
+test_message_parse_deep_header_nesting (void)
+{
+ const guint8 data[] = {
+ 'l', /* little-endian byte order */
+ 0x20, /* message type */
+ 0x20, /* message flags */
+ 0x01, /* major protocol version */
+ 0x20, 0x20, 0x20, 0x00, /* body length (invalid) */
+ 0x20, 0x20, 0x20, 0x20, /* message serial */
+ /* a{yv} of header fields:
+ * (things start to be even more invalid below here) */
+ 0x20, 0x20, 0x20, 0x00, /* array length (in bytes) */
+ 0x20, /* array key (this is not currently a valid header field) */
+ /* Variant array value: */
+ 0x01, /* signature length */
+ 'v', /* one complete type */
+ 0x00, /* nul terminator */
+ /* (Variant array value payload) */
+ /* Critically, this contains 64 nested variants (minus two for the
+ * ‘arbitrary valid content’ below, but ignoring two for the `a{yv}`
+ * above), which in total exceeds %G_DBUS_MAX_TYPE_DEPTH. */
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ /* Some arbitrary valid content inside the innermost variant: */
+ 0x01, 'y', 0x00, 0xcc,
+ /* (message body length missing) */
+ };
+ gsize size = sizeof (data);
+ GDBusMessage *message = NULL;
+ GError *local_error = NULL;
+
+ message = g_dbus_message_new_from_blob ((guchar *) data, size,
+ G_DBUS_CAPABILITY_FLAGS_NONE,
+ &local_error);
+ g_assert_error (local_error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT);
+ g_assert_null (message);
+
+ g_clear_error (&local_error);
+}
+
+/* ---------------------------------------------------------------------------------------------------- */
+
+/* Test that an invalid body in a D-Bus message (specifically, containing too
+ * many levels of nested variant) is gracefully handled with an error rather
+ * than a crash. The set of bytes here are a modified version of the bytes from
+ * test_message_parse_deep_header_nesting(). */
+static void
+test_message_parse_deep_body_nesting (void)
+{
+ const guint8 data[] = {
+ 'l', /* little-endian byte order */
+ 0x20, /* message type */
+ 0x20, /* message flags */
+ 0x01, /* major protocol version */
+ 0x20, 0x20, 0x20, 0x00, /* body length (invalid) */
+ 0x20, 0x20, 0x20, 0x20, /* message serial */
+ /* a{yv} of header fields: */
+ 0x07, 0x00, 0x00, 0x00, /* array length (in bytes) */
+ 0x08, /* array key (signature field) */
+ /* Variant array value: */
+ 0x01, /* signature length */
+ 'g', /* one complete type */
+ 0x00, /* nul terminator */
+ /* (Variant array value payload) */
+ 0x01, 'v', 0x00,
+ /* End-of-header padding to reach an 8-byte boundary: */
+ 0x00,
+ /* Message body: over 64 levels of nested variant, which is not valid: */
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00, 0x01, 'v', 0x00,
+ /* Some arbitrary valid content inside the innermost variant: */
+ 0x01, 'y', 0x00, 0xcc,
+ };
+ gsize size = sizeof (data);
+ GDBusMessage *message = NULL;
+ GError *local_error = NULL;
+
+ message = g_dbus_message_new_from_blob ((guchar *) data, size,
+ G_DBUS_CAPABILITY_FLAGS_NONE,
+ &local_error);
+ g_assert_error (local_error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT);
+ g_assert_null (message);
+
+ g_clear_error (&local_error);
+}
+
+/* ---------------------------------------------------------------------------------------------------- */
+
int
main (int argc,
char *argv[])
@@ -1262,15 +1423,19 @@
g_test_init (&argc, &argv, NULL);
g_test_bug_base ("https://bugzilla.gnome.org/show_bug.cgi?id=");
- g_test_add_func ("/gdbus/message-serialize-basic", message_serialize_basic);
- g_test_add_func ("/gdbus/message-serialize-complex", message_serialize_complex);
- g_test_add_func ("/gdbus/message-serialize-invalid", message_serialize_invalid);
- g_test_add_func ("/gdbus/message-serialize-header-checks", message_serialize_header_checks);
+ g_test_add_func ("/gdbus/message-serialize/basic",
+ test_message_serialize_basic);
+ g_test_add_func ("/gdbus/message-serialize/complex",
+ test_message_serialize_complex);
+ g_test_add_func ("/gdbus/message-serialize/invalid",
+ test_message_serialize_invalid);
+ g_test_add_func ("/gdbus/message-serialize/header-checks",
+ test_message_serialize_header_checks);
+ g_test_add_func ("/gdbus/message-serialize/double-array",
+ test_message_serialize_double_array);
- g_test_add_func ("/gdbus/message-parse-empty-arrays-of-arrays",
- message_parse_empty_arrays_of_arrays);
-
- g_test_add_func ("/gdbus/message-serialize/double-array", test_double_array);
+ g_test_add_func ("/gdbus/message-parse/empty-arrays-of-arrays",
+ test_message_parse_empty_arrays_of_arrays);
g_test_add_func ("/gdbus/message-parse/non-signature-header",
test_message_parse_non_signature_header);
g_test_add_func ("/gdbus/message-parse/empty-signature-header",
@@ -1279,6 +1444,10 @@
test_message_parse_multiple_signature_header);
g_test_add_func ("/gdbus/message-parse/over-long-signature-header",
test_message_parse_over_long_signature_header);
+ g_test_add_func ("/gdbus/message-parse/deep-header-nesting",
+ test_message_parse_deep_header_nesting);
+ g_test_add_func ("/gdbus/message-parse/deep-body-nesting",
+ test_message_parse_deep_body_nesting);
return g_test_run();
}
