From 89a1b571adde644664093dd4763fb9aa077dfafc Mon Sep 17 00:00:00 2001 From: David Zeuthen Date: Wed, 4 Aug 2010 14:38:51 -0400 Subject: [PATCH] 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 --- gio/gdbusconnection.c | 5 -- gio/gdbusmessage.c | 119 ++++++++++++++++++++++++++++-- gio/tests/gdbus-serialization.c | 155 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 268 insertions(+), 11 deletions(-) diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c index 9551b49..6974510 100644 --- a/gio/gdbusconnection.c +++ b/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 diff --git a/gio/gdbusmessage.c b/gio/gdbusmessage.c index e8a468e..0d2e15e 100644 --- a/gio/gdbusmessage.c +++ b/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); - - if (interface_ != NULL) - g_dbus_message_set_interface (message, interface_); + g_dbus_message_set_interface (message, interface_); return message; } @@ -746,6 +744,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, GError **error) @@ -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)) diff --git a/gio/tests/gdbus-serialization.c b/gio/tests/gdbus-serialization.c index 12abb77..38227c3 100644 --- a/gio/tests/gdbus-serialization.c +++ b/gio/tests/gdbus-serialization.c @@ -20,6 +20,7 @@ * Author: David Zeuthen */ +#include #include #include @@ -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(); } -- 2.7.4