Mitigate UAF crashes due to wl_client_destroy reentrancy

There are situations in which a call into wl_client_destroy() can
result in a reentrant call into wl_client_destroy() - which
results in UAF / double free crashes.

For example, this can occur in the following scenario.

1. Server receives a message notifying it that a client has
   disconnected (WL_EVENT_HANGUP [1])

2. This beings client destruction with a call to wl_client_destroy()

3. wl_client_destroy() kicks off callbacks as client-associated
   resources are cleaned up and their destructors and destruction
   signals are invoked.

4. These callbacks eventually lead to an explicit call to
   wl_display_flush_clients() as the server attempts to flush
   events to other connected clients.

5. Since the client has already begun destruction, when it is
   reached in the iteration the flush fails wl_client_destroy()
   is called again [2].

This patch guards against this reentrant condition by removing
the client from the display's client list when wl_client_destroy()
is first called. This prevents access / iteration over the client
after wl_client_destroy() is called.

In the example above, wl_display_flush_clients() will pass over
the client currently undergoing destruction and the reentrant
call is avoided.

[1] https://gitlab.freedesktop.org/wayland/wayland/-/blob/8f499bf4045f88f3a4b4b0a445befca467bebe20/src/wayland-server.c#L342

[2] https://gitlab.freedesktop.org/wayland/wayland/-/blob/8f499bf4045f88f3a4b4b0a445befca467bebe20/src/wayland-server.c#L1512

Signed-off-by: Thomas Lukaszewicz [thomaslukaszewicz@gmail.com](mailto:thomaslukaszewicz@gmail.com)

1 files changed, 12 insertions, 1 deletions

diff --git a/src/wayland-server.c b/src/wayland-server.c
index 1e079a3..78ff405 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -933,6 +933,18 @@ wl_client_get_destroy_late_listener(struct wl_client *client,
 WL_EXPORT void
 wl_client_destroy(struct wl_client *client)
 {
+
+	/* wl_client_destroy() should not be called twice for the same client. */
+	if (wl_list_empty(&client->link)) {
+		client->error = 1;
+		wl_log("wl_client_destroy: encountered re-entrant client destruction.\n");
+		return;
+	}
+
+	wl_list_remove(&client->link);
+	/* Keep the client link safe to inspect. */
+	wl_list_init(&client->link);
+
 	wl_priv_signal_final_emit(&client->destroy_signal, client);
 
 	wl_client_flush(client);
@@ -943,7 +955,6 @@ wl_client_destroy(struct wl_client *client)
 
 	wl_priv_signal_final_emit(&client->destroy_late_signal, client);
 
-	wl_list_remove(&client->link);
 	wl_list_remove(&client->resource_created_signal.listener_list);
 
 	if (client->data_dtor)