GDBusMessage: Validate header fields when serializing/deserializing

The D-Bus spec mentions exactly what header fields are required for
various message types. Add tests for this as well.

Also disallow empty interfaces for signals since the D-Bus spec says
this is Verboten already.

Signed-off-by: David Zeuthen <davidz@redhat.com>

gio/gdbusconnection.c

@@ -23,11 +23,6 @@
 /*
  * TODO for GDBus:
  *
- * - Validate all data (e.g. UTF-8) and check all the required D-Bus headers
- *   are present and forbidden ones aren't
- *   - When writing: g_dbus_message_to_blob()
- *   - When reading: g_dbus_message_new_from_blob()
- *
  * - would be nice to expose GDBusAuthMechanism and an extension point
  *
  * - Need to rewrite GDBusAuth and rework GDBusAuthMechanism. In particular

gio/gdbusmessage.c

@@ -210,7 +210,7 @@ g_dbus_message_new_method_call (const gchar *name,
 /**
  * g_dbus_message_new_signal:
  * @path: A valid object path.
- * @interface_: A valid D-Bus interface name or %NULL.
+ * @interface_: A valid D-Bus interface name.
  * @signal: A valid signal name.
  *
  * Creates a new #GDBusMessage for a signal emission.
@@ -228,7 +228,7 @@ g_dbus_message_new_signal (const gchar  *path,
 
   g_return_val_if_fail (g_variant_is_object_path (path), NULL);
   g_return_val_if_fail (g_dbus_is_member_name (signal), NULL);
-  g_return_val_if_fail (interface_ == NULL || g_dbus_is_interface_name (interface_), NULL);
+  g_return_val_if_fail (g_dbus_is_interface_name (interface_), NULL);
 
   message = g_dbus_message_new ();
   message->type = G_DBUS_MESSAGE_TYPE_SIGNAL;
@@ -236,9 +236,7 @@ g_dbus_message_new_signal (const gchar  *path,
 
   g_dbus_message_set_path (message, path);
   g_dbus_message_set_member (message, signal);
-
+  g_dbus_message_set_interface (message, interface_);
-  if (interface_ != NULL)
-    g_dbus_message_set_interface (message, interface_);
 
   return message;
 }
@@ -745,6 +743,105 @@ g_dbus_message_set_unix_fd_list (GDBusMessage  *message,
 
 /* ---------------------------------------------------------------------------------------------------- */
 
+static gboolean
+validate_headers (GDBusMessage  *message,
+                  GError       **error)
+{
+  gboolean ret;
+
+  g_return_val_if_fail (G_IS_DBUS_MESSAGE (message), FALSE);
+  g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
+
+  ret = FALSE;
+
+  switch (message->type)
+    {
+    case G_DBUS_MESSAGE_TYPE_INVALID:
+      g_set_error_literal (error,
+                           G_IO_ERROR,
+                           G_IO_ERROR_INVALID_ARGUMENT,
+                           _("type is INVALID"));
+      goto out;
+      break;
+
+    case G_DBUS_MESSAGE_TYPE_METHOD_CALL:
+      if (g_dbus_message_get_header (message, G_DBUS_MESSAGE_HEADER_FIELD_PATH) == NULL ||
+          g_dbus_message_get_header (message, G_DBUS_MESSAGE_HEADER_FIELD_MEMBER) == NULL)
+        {
+          g_set_error_literal (error,
+                               G_IO_ERROR,
+                               G_IO_ERROR_INVALID_ARGUMENT,
+                               _("METHOD_CALL message: PATH or MEMBER header field is missing"));
+          goto out;
+        }
+      break;
+
+    case G_DBUS_MESSAGE_TYPE_METHOD_RETURN:
+      if (g_dbus_message_get_header (message, G_DBUS_MESSAGE_HEADER_FIELD_REPLY_SERIAL) == NULL)
+        {
+          g_set_error_literal (error,
+                               G_IO_ERROR,
+                               G_IO_ERROR_INVALID_ARGUMENT,
+                               _("METHOD_RETURN message: REPLY_SERIAL header field is missing"));
+          goto out;
+        }
+      break;
+
+    case G_DBUS_MESSAGE_TYPE_ERROR:
+      if (g_dbus_message_get_header (message, G_DBUS_MESSAGE_HEADER_FIELD_ERROR_NAME) == NULL ||
+          g_dbus_message_get_header (message, G_DBUS_MESSAGE_HEADER_FIELD_REPLY_SERIAL) == NULL)
+        {
+          g_set_error_literal (error,
+                               G_IO_ERROR,
+                               G_IO_ERROR_INVALID_ARGUMENT,
+                               _("ERROR message: REPLY_SERIAL or ERROR_NAME header field is missing"));
+          goto out;
+        }
+      break;
+
+    case G_DBUS_MESSAGE_TYPE_SIGNAL:
+      if (g_dbus_message_get_header (message, G_DBUS_MESSAGE_HEADER_FIELD_PATH) == NULL ||
+          g_dbus_message_get_header (message, G_DBUS_MESSAGE_HEADER_FIELD_INTERFACE) == NULL ||
+          g_dbus_message_get_header (message, G_DBUS_MESSAGE_HEADER_FIELD_MEMBER) == NULL)
+        {
+          g_set_error_literal (error,
+                               G_IO_ERROR,
+                               G_IO_ERROR_INVALID_ARGUMENT,
+                               _("SIGNAL message: PATH, INTERFACE or MEMBER header field is missing"));
+          goto out;
+        }
+      if (g_strcmp0 (g_dbus_message_get_path (message), "/org/freedesktop/DBus/Local") == 0)
+        {
+          g_set_error_literal (error,
+                               G_IO_ERROR,
+                               G_IO_ERROR_INVALID_ARGUMENT,
+                               _("SIGNAL message: The PATH header field is using the reserved value /org/freedesktop/DBus/Local"));
+          goto out;
+        }
+      if (g_strcmp0 (g_dbus_message_get_interface (message), "org.freedesktop.DBus.Local") == 0)
+        {
+          g_set_error_literal (error,
+                               G_IO_ERROR,
+                               G_IO_ERROR_INVALID_ARGUMENT,
+                               _("SIGNAL message: The INTERFACE header field is using the reserved value org.freedesktop.DBus.Local"));
+          goto out;
+        }
+      break;
+
+    default:
+      /* hitherto unknown type - nothing to check */
+      break;
+    }
+
+  ret = TRUE;
+
+ out:
+  g_assert (ret || (error == NULL || *error != NULL));
+  return ret;
+}
+
+/* ---------------------------------------------------------------------------------------------------- */
+
 static gboolean
 ensure_input_padding (GMemoryInputStream   *mis,
                       gsize                 padding_size,
@@ -1611,6 +1708,11 @@ g_dbus_message_new_from_blob (guchar                *blob,
         }
     }
 
+  if (!validate_headers (message, error))
+    {
+      g_prefix_error (error, _("Cannot deserialize message: "));
+      goto out;
+    }
 
   ret = TRUE;
 
@@ -2068,7 +2170,6 @@ g_dbus_message_to_blob (GDBusMessage          *message,
     num_fds_in_message = g_unix_fd_list_get_length (message->fd_list);
 #endif
   num_fds_according_to_header = g_dbus_message_get_num_unix_fds (message);
-  /* TODO: check we have all the right header fields and that they are the correct value etc etc */
   if (num_fds_in_message != num_fds_according_to_header)
     {
       g_set_error (error,
@@ -2080,6 +2181,12 @@ g_dbus_message_to_blob (GDBusMessage          *message,
       goto out;
     }
 
+  if (!validate_headers (message, error))
+    {
+      g_prefix_error (error, _("Cannot serialize message: "));
+      goto out;
+    }
+
   g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{yv}"));
   g_hash_table_iter_init (&hash_iter, message->headers);
   while (g_hash_table_iter_next (&hash_iter, &key, (gpointer) &header_value))

gio/tests/gdbus-serialization.c

@@ -20,6 +20,7 @@
  * Author: David Zeuthen <davidz@redhat.com>
  */
 
+#include <locale.h>
 #include <gio/gio.h>
 
 #include <string.h>
@@ -821,16 +822,170 @@ message_serialize_invalid (void)
 
 /* ---------------------------------------------------------------------------------------------------- */
 
+static void
+message_serialize_header_checks (void)
+{
+  GDBusMessage *message;
+  GDBusMessage *reply;
+  GError *error;
+  guchar *blob;
+  gsize blob_size;
+
+  /*
+   * check we can't serialize messages with INVALID type
+   */
+  message = g_dbus_message_new ();
+  error = NULL;
+  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, ==, "Cannot serialize message: type is INVALID");
+  g_error_free (error);
+  g_assert (blob == NULL);
+  g_object_unref (message);
+
+  /*
+   * check we can't serialize signal messages with INTERFACE, PATH or MEMBER unset / set to reserved value
+   */
+  message = g_dbus_message_new_signal ("/the/path", "The.Interface", "TheMember");
+  /* ----- */
+  /* interface NULL => error */
+  g_dbus_message_set_interface (message, NULL);
+  error = NULL;
+  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, ==, "Cannot serialize message: SIGNAL message: PATH, INTERFACE or MEMBER header field is missing");
+  g_error_free (error);
+  g_assert (blob == NULL);
+  /* interface reserved value => error */
+  g_dbus_message_set_interface (message, "org.freedesktop.DBus.Local");
+  error = NULL;
+  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, ==, "Cannot serialize message: SIGNAL message: The INTERFACE header field is using the reserved value org.freedesktop.DBus.Local");
+  g_error_free (error);
+  g_assert (blob == NULL);
+  /* reset interface */
+  g_dbus_message_set_interface (message, "The.Interface");
+  /* ----- */
+  /* path NULL => error */
+  g_dbus_message_set_path (message, NULL);
+  error = NULL;
+  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, ==, "Cannot serialize message: SIGNAL message: PATH, INTERFACE or MEMBER header field is missing");
+  g_error_free (error);
+  g_assert (blob == NULL);
+  /* path reserved value => error */
+  g_dbus_message_set_path (message, "/org/freedesktop/DBus/Local");
+  error = NULL;
+  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, ==, "Cannot serialize message: SIGNAL message: The PATH header field is using the reserved value /org/freedesktop/DBus/Local");
+  g_error_free (error);
+  g_assert (blob == NULL);
+  /* reset path */
+  g_dbus_message_set_path (message, "/the/path");
+  /* ----- */
+  /* member NULL => error */
+  g_dbus_message_set_member (message, NULL);
+  error = NULL;
+  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, ==, "Cannot serialize message: SIGNAL message: PATH, INTERFACE or MEMBER header field is missing");
+  g_error_free (error);
+  g_assert (blob == NULL);
+  /* reset member */
+  g_dbus_message_set_member (message, "TheMember");
+  /* ----- */
+  /* done */
+  g_object_unref (message);
+
+  /*
+   * check that we can't serialize method call messages with PATH or MEMBER unset
+   */
+  message = g_dbus_message_new_method_call (NULL, "/the/path", NULL, "TheMember");
+  /* ----- */
+  /* path NULL => error */
+  g_dbus_message_set_path (message, NULL);
+  error = NULL;
+  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, ==, "Cannot serialize message: METHOD_CALL message: PATH or MEMBER header field is missing");
+  g_error_free (error);
+  g_assert (blob == NULL);
+  /* reset path */
+  g_dbus_message_set_path (message, "/the/path");
+  /* ----- */
+  /* member NULL => error */
+  g_dbus_message_set_member (message, NULL);
+  error = NULL;
+  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, ==, "Cannot serialize message: METHOD_CALL message: PATH or MEMBER header field is missing");
+  g_error_free (error);
+  g_assert (blob == NULL);
+  /* reset member */
+  g_dbus_message_set_member (message, "TheMember");
+  /* ----- */
+  /* done */
+  g_object_unref (message);
+
+  /*
+   * check that we can't serialize method reply messages with REPLY_SERIAL unset
+   */
+  message = g_dbus_message_new_method_call (NULL, "/the/path", NULL, "TheMember");
+  g_dbus_message_set_serial (message, 42);
+  /* method reply */
+  reply = g_dbus_message_new_method_reply (message);
+  g_assert_cmpint (g_dbus_message_get_reply_serial (reply), ==, 42);
+  g_dbus_message_set_header (reply, G_DBUS_MESSAGE_HEADER_FIELD_REPLY_SERIAL, NULL);
+  error = NULL;
+  blob = g_dbus_message_to_blob (reply, &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, ==, "Cannot serialize message: METHOD_RETURN message: REPLY_SERIAL header field is missing");
+  g_error_free (error);
+  g_assert (blob == NULL);
+  g_object_unref (reply);
+  /* method error - first nuke ERROR_NAME, then REPLY_SERIAL */
+  reply = g_dbus_message_new_method_error (message, "Some.Error.Name", "the message");
+  g_assert_cmpint (g_dbus_message_get_reply_serial (reply), ==, 42);
+  /* nuke ERROR_NAME */
+  g_dbus_message_set_error_name (reply, NULL);
+  error = NULL;
+  blob = g_dbus_message_to_blob (reply, &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, ==, "Cannot serialize message: ERROR message: REPLY_SERIAL or ERROR_NAME header field is missing");
+  g_error_free (error);
+  g_assert (blob == NULL);
+  /* reset ERROR_NAME */
+  g_dbus_message_set_error_name (reply, "Some.Error.Name");
+  /* nuke REPLY_SERIAL */
+  g_dbus_message_set_header (reply, G_DBUS_MESSAGE_HEADER_FIELD_REPLY_SERIAL, NULL);
+  error = NULL;
+  blob = g_dbus_message_to_blob (reply, &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, ==, "Cannot serialize message: ERROR message: REPLY_SERIAL or ERROR_NAME header field is missing");
+  g_error_free (error);
+  g_assert (blob == NULL);
+  g_object_unref (reply);
+  g_object_unref (message);
+}
+
+/* ---------------------------------------------------------------------------------------------------- */
+
 int
 main (int   argc,
       char *argv[])
 {
+  setlocale (LC_ALL, "C");
+
   g_type_init ();
   g_test_init (&argc, &argv, NULL);
 
   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);
   return g_test_run();
 }