[glib: 6/16] gdbusmessage: Validate type of message header signature field

[Philip Withnall <pwithnall src gnome org>]

commit af712bbce1580455d455aeea214c39648ad70b46
Author: Philip Withnall <withnall endlessm com>
Date:   Thu Aug 16 21:33:52 2018 +0100

    gdbusmessage: Validate type of message header signature field
    
    Parsing a D-Bus message with the signature field in the message header
    of type other than ‘g’ (GVariant type signature) would cause a critical
    warning. Instead, we should return a runtime error.
    
    Includes a test.
    
    oss-fuzz#9825
    
    Signed-off-by: Philip Withnall <withnall endlessm com>

 gio/gdbusmessage.c              | 19 ++++++++++++++
 gio/tests/gdbus-serialization.c | 56 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+)
---
diff --git a/gio/gdbusmessage.c b/gio/gdbusmessage.c
index 8de836bf6..79bc11c16 100644
--- a/gio/gdbusmessage.c
+++ b/gio/gdbusmessage.c
@@ -2115,6 +2115,15 @@ g_dbus_message_new_from_blob (guchar                *blob,
       const gchar *signature_str;
       gsize signature_str_len;
 
+      if (!g_variant_is_of_type (signature, G_VARIANT_TYPE_SIGNATURE))
+        {
+          g_set_error_literal (error,
+                               G_IO_ERROR,
+                               G_IO_ERROR_INVALID_ARGUMENT,
+                               _("Signature header found but is not of type signature"));
+          goto out;
+        }
+
       signature_str = g_variant_get_string (signature, &signature_str_len);
 
       /* signature but no body */
@@ -2695,6 +2704,16 @@ g_dbus_message_to_blob (GDBusMessage          *message,
   body_start_offset = mbuf.valid_len;
 
   signature = g_dbus_message_get_header (message, G_DBUS_MESSAGE_HEADER_FIELD_SIGNATURE);
+
+  if (signature != NULL && !g_variant_is_of_type (signature, G_VARIANT_TYPE_SIGNATURE))
+    {
+      g_set_error_literal (error,
+                           G_IO_ERROR,
+                           G_IO_ERROR_INVALID_ARGUMENT,
+                           _("Signature header found but is not of type signature"));
+      goto out;
+    }
+
   signature_str = NULL;
   if (signature != NULL)
       signature_str = g_variant_get_string (signature, NULL);
diff --git a/gio/tests/gdbus-serialization.c b/gio/tests/gdbus-serialization.c
index 2ab856c48..3cdade6d7 100644
--- a/gio/tests/gdbus-serialization.c
+++ b/gio/tests/gdbus-serialization.c
@@ -873,6 +873,20 @@ message_serialize_header_checks (void)
   g_assert (blob == NULL);
   g_object_unref (message);
 
+  /*
+   * check that we can't serialize messages with SIGNATURE set to a non-signature-typed value
+   */
+  message = g_dbus_message_new_signal ("/the/path", "The.Interface", "TheMember");
+  g_dbus_message_set_header (message, G_DBUS_MESSAGE_HEADER_FIELD_SIGNATURE, g_variant_new_boolean (FALSE));
+  blob = g_dbus_message_to_blob (message, &blob_size, G_DBUS_CAPABILITY_FLAGS_NONE, &error);
+
+  g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT);
+  g_assert_cmpstr (error->message, ==, "Signature header found but is not of type signature");
+  g_assert_null (blob);
+
+  g_clear_error (&error);
+  g_clear_object (&message);
+
   /*
    * check we can't serialize signal messages with INTERFACE, PATH or MEMBER unset / set to reserved value
    */
@@ -1083,6 +1097,46 @@ test_double_array (void)
 
 /* ---------------------------------------------------------------------------------------------------- */
 
+/* Test that an invalid header in a D-Bus message (specifically, with a type
+ * which doesn’t match what’s expected for the given header) 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_non_signature_header (void)
+{
+  const guint8 data[] = {
+    0x6c,  /* byte order */
+    0x04,  /* message type */
+    0x0f,  /* message flags */
+    0x01,  /* major protocol version */
+    0x00, 0x00, 0x00, 0x00,  /* body length */
+    0x00, 0x00, 0x00, 0xbc,  /* message serial */
+    /* a{yv} of header fields:
+     * (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: */
+      0x04, /* signature length */
+      'd', 0x00, 0x00, 'F',  /* signature (invalid) */
+      0x00,  /* nul terminator */
+      /* (GVariant 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);
+}
+
+/* ---------------------------------------------------------------------------------------------------- */
+
 int
 main (int   argc,
       char *argv[])
@@ -1102,6 +1156,8 @@ main (int   argc,
       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/non-signature-header",
+                   test_message_parse_non_signature_header);
 
   return g_test_run();
 }