gmenuexporter: Fix a NULL pointer dereference on an error handling path

This latent bug wasn’t triggered until commit 3f30ec86c (or its
cherry-pick onto `glib-2-80`, 747e3af99, which was first released in
2.80.1).

That change means that `g_menu_exporter_free()` is now called on the
registration failure path by `g_dbus_connection_register_object()`
before it returns. The caller then tries to call `g_slice_free()` on the
exporter again. The call to `g_menu_exporter_free()` tries to
dereference/free members of the exporter which it expects to be
initialised — but because this is happening in an error handling path,
they are not initialised.

If it were to get any further, the `g_slice_free()` would then be a
double-free on the exporter allocation.

Fix that by making `g_menu_exporter_free()` robust to some of the
exporter members being `NULL`, and moving some of the initialisation
code higher in `g_dbus_connection_export_menu_model()`, and removing the
duplicate free code on the error handling path.

This includes a unit test.

Signed-off-by: Philip Withnall <pwithnall@gnome.org>

Fixes: #3366
This commit is contained in:
Philip Withnall
2024-05-15 12:26:36 +01:00
parent 34626188aa
commit df2c5d925a
2 changed files with 45 additions and 15 deletions

View File

@@ -1159,6 +1159,42 @@ test_dbus_peer_subscriptions (void)
#endif
}
static void
test_dbus_export_error_handling (void)
{
GRand *rand = NULL;
RandomMenu *menu = NULL;
GDBusConnection *bus;
GError *local_error = NULL;
guint id1, id2;
g_test_summary ("Test that error handling of menu model export failure works");
g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/3366");
bus = g_bus_get_sync (G_BUS_TYPE_SESSION, NULL, NULL);
rand = g_rand_new_with_seed (g_test_rand_int ());
menu = random_menu_new (rand, 2);
id1 = g_dbus_connection_export_menu_model (bus, "/", G_MENU_MODEL (menu), &local_error);
g_assert_no_error (local_error);
g_assert_cmpuint (id1, !=, 0);
/* Trigger a failure by trying to export on a path which is already in use */
id2 = g_dbus_connection_export_menu_model (bus, "/", G_MENU_MODEL (menu), &local_error);
g_assert_error (local_error, G_IO_ERROR, G_IO_ERROR_EXISTS);
g_assert_cmpuint (id2, ==, 0);
g_clear_error (&local_error);
g_dbus_connection_unexport_menu_model (bus, id1);
while (g_main_context_iteration (NULL, FALSE));
g_clear_object (&menu);
g_rand_free (rand);
g_clear_object (&bus);
}
static gpointer
do_modify (gpointer data)
{
@@ -1658,6 +1694,7 @@ main (int argc, char **argv)
g_test_add_func ("/gmenu/dbus/threaded", test_dbus_threaded);
g_test_add_func ("/gmenu/dbus/peer/roundtrip", test_dbus_peer_roundtrip);
g_test_add_func ("/gmenu/dbus/peer/subscriptions", test_dbus_peer_subscriptions);
g_test_add_func ("/gmenu/dbus/export/error-handling", test_dbus_export_error_handling);
g_test_add_func ("/gmenu/attributes", test_attributes);
g_test_add_func ("/gmenu/attributes/iterate", test_attribute_iter);
g_test_add_func ("/gmenu/links", test_links);