[glib/glib-2-58: 10/21] gdbusmessage: Check for valid GVariantType when parsing a variant blob

[Philip Withnall <pwithnall src gnome org>]

commit 4624fc06ba74531073ef9540a55b7470efadf01d
Author: Philip Withnall <withnall endlessm com>
Date:   Tue Sep 18 15:17:44 2018 +0100

    gdbusmessage: Check for valid GVariantType when parsing a variant blob
    
    The code was checking whether the signature provided by the blob was a
    valid D-Bus signature — but that’s a superset of a valid GVariant type
    string, since a D-Bus signature is zero or more complete types. A
    GVariant type string is exactly one complete type.
    
    This meant that a D-Bus message with a header field containing a variant
    with an empty type signature (for example) could cause a critical
    warning in the code parsing it.
    
    Fix that by checking whether the string is a valid type string too.
    
    Unit test included.
    
    oss-fuzz#9810
    
    Signed-off-by: Philip Withnall <withnall endlessm com>

 gio/gdbusmessage.c              |  5 ++-
 gio/tests/gdbus-serialization.c | 89 +++++++++++++++++++++++++++++++++++++++--
 2 files changed, 90 insertions(+), 4 deletions(-)
---
diff --git a/gio/gdbusmessage.c b/gio/gdbusmessage.c
index b1d73fad5..169e6fd15 100644
--- a/gio/gdbusmessage.c
+++ b/gio/gdbusmessage.c
@@ -1846,8 +1846,11 @@ parse_value_from_blob (GMemoryBuffer       *buf,
               sig = read_string (buf, (gsize) siglen, &local_error);
               if (sig == NULL)
                 goto fail;
-              if (!g_variant_is_signature (sig))
+              if (!g_variant_is_signature (sig) ||
+                  !g_variant_type_string_is_valid (sig))
                 {
+                  /* A D-Bus signature can contain zero or more complete types,
+                   * but a GVariant has to be exactly one complete type. */
                   g_set_error (&local_error,
                                G_IO_ERROR,
                                G_IO_ERROR_INVALID_ARGUMENT,
diff --git a/gio/tests/gdbus-serialization.c b/gio/tests/gdbus-serialization.c
index 3cdade6d7..5848442e0 100644
--- a/gio/tests/gdbus-serialization.c
+++ b/gio/tests/gdbus-serialization.c
@@ -1105,7 +1105,7 @@ static void
 test_message_parse_non_signature_header (void)
 {
   const guint8 data[] = {
-    0x6c,  /* byte order */
+    'l',  /* little-endian byte order */
     0x04,  /* message type */
     0x0f,  /* message flags */
     0x01,  /* major protocol version */
@@ -1115,11 +1115,90 @@ test_message_parse_non_signature_header (void)
      * (things start to be invalid below here) */
     0x02, 0x00, 0x00, 0x00,  /* array length (in bytes) */
       G_DBUS_MESSAGE_HEADER_FIELD_SIGNATURE, /* array key */
-      /* GVariant array value: */
+      /* Variant array value: */
       0x04, /* signature length */
       'd', 0x00, 0x00, 'F',  /* signature (invalid) */
       0x00,  /* nul terminator */
-      /* (GVariant array value payload missing) */
+      /* (Variant array value payload missing) */
+    /* (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 header in a D-Bus message (specifically, containing a
+ * variant with an empty type signature) is gracefully handled with an error
+ * rather than a crash. The set of bytes here come directly from fuzzer
+ * output. */
+static void
+test_message_parse_empty_signature_header (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 */
+      /* Variant array value: */
+      0x00, /* signature length */
+      0x00,  /* nul terminator */
+      /* (Variant array value payload missing) */
+    /* (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 header in a D-Bus message (specifically, containing a
+ * variant with a type signature containing multiple complete types) is
+ * gracefully handled with an error rather than a crash. The set of bytes here
+ * come directly from fuzzer output. */
+static void
+test_message_parse_multiple_signature_header (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 */
+      /* Variant array value: */
+      0x02, /* signature length */
+      'b', 'b',  /* two complete types */
+      0x00,  /* nul terminator */
+      /* (Variant array value payload missing) */
     /* (message body length missing) */
   };
   gsize size = sizeof (data);
@@ -1158,6 +1237,10 @@ main (int   argc,
   g_test_add_func ("/gdbus/message-serialize/double-array", test_double_array);
   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",
+                   test_message_parse_empty_signature_header);
+  g_test_add_func ("/gdbus/message-parse/multiple-signature-header",
+                   test_message_parse_multiple_signature_header);
 
   return g_test_run();
 }