On 2018-08-28 19:31:27, Markus Koschany wrote: > Hello Chris, > > the Debian LTS team would like to fix CVE-2018-14424, gdm3 in Jessie. We > have prepared a patch [1] based on your work which you have attached to > the Gnome issue tracker. [2] We have noticed [3] that it is still > possible to "crash" gdm3 in Jessie with your POC although we cannot get > a meaningful stacktrace to find out why.
Hi! For the record, I cannot reproduce those results. With the packages provided in [3], everything works well in my test environment. I am testing in Vagrant 2.0.2 backed by VirtualBox 5.2.18 on Debian buster, with an up-to-date official "jessie" box available here: https://app.vagrantup.com/debian/boxes/jessie64 To reproduce, on the host: sudo apt install vagrant vagrant init debian/jessie64 vagrant up vagrant ssh Then, in the guest, this should reproduce the crash in jessie: sudo apt update ; sudo apt upgrade -y sudo apt install -y gdm3 wget https://gitlab.gnome.org/GNOME/gdm/uploads/70fb90ddb64ea3b4697be3e93438dc2c/crash-gdm.sh cat crash-gdm.sh # quick audit sh crash-gdm.sh # reproduce the crash sudo dmesg | tail # should show segfault Then, in the guest again, upgrade to my test packages: sudo apt install devscripts dget https://people.debian.org/~anarcat/debian/jessie-lts/gdm3_3.14.1-7+deb7u1_amd64.changes sudo apt install ./gdm3_3.14.1-7+deb7u1_amd64.deb sudo service restart gdm3 sh crash-gdm.sh sudo dmesg | tail # shows no segfault I believe the confusion might be coming from the signal that is sent in the proof of concept (PoC): because it sends a transient screen call, it *looks* like the session is crashed. But here's a test procedure that will show it does not, actually crash the session and that the provided packages behave correctly: 1. start gdm3 2. login (user: vagrant, password: vagrant) 3. start any application (say the file manager) 4. launch the PoC (sh crash-gdm.sh) 5. a login dialog comes up. login again 6. session returns with the application still running Markus, are you sure the session actually crashes? > Do you plan to fix this issue for Ubuntu 16.04 too? The version in > 16.04 is closer to ours, so you may experience the same result. For what it's worth, the patch should be available here: https://people.debian.org/~anarcat/debian/jessie-lts/gdm3_3.14.1-7+deb7u1.debian.tar.xz I also attach the two patches backported by Markus for review. > Do you think there is an additional patch required or is GDM actually > doing what it is asked for? >From my point of view, it's doing exactly what it is asked for. :) I hope that clarifies things! A. -- Those who would give up essential Liberty, to purchase a little temporary Safety, deserve neither Liberty nor Safety. - Benjamin Franklin, 1755
From: Markus Koschany <a...@debian.org> Date: Thu, 16 Aug 2018 11:48:54 +0200 Subject: CVE-2018-14424_part1 Bug-Upstream: https://gitlab.gnome.org/GNOME/gdm/issues/401 Origin: https://gitlab.gnome.org/GNOME/gdm/commit/6060db704a19b0db68f2e9e6a2d020c0c78b6bba --- daemon/gdm-display-store.c | 11 +++-------- daemon/gdm-display-store.h | 2 +- daemon/gdm-manager.c | 19 +++++++++---------- daemon/gdm-manager.h | 3 ++- 4 files changed, 15 insertions(+), 20 deletions(-) diff --git a/daemon/gdm-display-store.c b/daemon/gdm-display-store.c index af76f51..fd24334 100644 --- a/daemon/gdm-display-store.c +++ b/daemon/gdm-display-store.c @@ -76,15 +76,10 @@ stored_display_new (GdmDisplayStore *store, static void stored_display_free (StoredDisplay *stored_display) { - char *id; - - gdm_display_get_id (stored_display->display, &id, NULL); - g_signal_emit (G_OBJECT (stored_display->store), signals[DISPLAY_REMOVED], 0, - id); - g_free (id); + stored_display->display); g_debug ("GdmDisplayStore: Unreffing display: %p", stored_display->display); @@ -281,9 +276,9 @@ gdm_display_store_class_init (GdmDisplayStoreClass *klass) G_STRUCT_OFFSET (GdmDisplayStoreClass, display_removed), NULL, NULL, - g_cclosure_marshal_VOID__STRING, + g_cclosure_marshal_VOID__OBJECT, G_TYPE_NONE, - 1, G_TYPE_STRING); + 1, G_TYPE_OBJECT); g_type_class_add_private (klass, sizeof (GdmDisplayStorePrivate)); } diff --git a/daemon/gdm-display-store.h b/daemon/gdm-display-store.h index 2835993..0aff8ee 100644 --- a/daemon/gdm-display-store.h +++ b/daemon/gdm-display-store.h @@ -49,7 +49,7 @@ typedef struct void (* display_added) (GdmDisplayStore *display_store, const char *id); void (* display_removed) (GdmDisplayStore *display_store, - const char *id); + GdmDisplay *display); } GdmDisplayStoreClass; typedef enum diff --git a/daemon/gdm-manager.c b/daemon/gdm-manager.c index a464ba6..33ed7fe 100644 --- a/daemon/gdm-manager.c +++ b/daemon/gdm-manager.c @@ -1286,19 +1286,18 @@ on_display_status_changed (GdmDisplay *display, static void on_display_removed (GdmDisplayStore *display_store, - const char *id, + GdmDisplay *display, GdmManager *manager) { - GdmDisplay *display; + char *id; - display = gdm_display_store_lookup (display_store, id); - if (display != NULL) { - g_dbus_object_manager_server_unexport (manager->priv->object_manager, id); + gdm_display_get_id (display, &id, NULL); + g_dbus_object_manager_server_unexport (manager->priv->object_manager, id); + g_free (id); - g_signal_handlers_disconnect_by_func (display, G_CALLBACK (on_display_status_changed), manager); + g_signal_handlers_disconnect_by_func (display, G_CALLBACK (on_display_status_changed), manager); - g_signal_emit (manager, signals[DISPLAY_REMOVED], 0, id); - } + g_signal_emit (manager, signals[DISPLAY_REMOVED], 0, display); } typedef struct @@ -2240,9 +2239,9 @@ gdm_manager_class_init (GdmManagerClass *klass) G_STRUCT_OFFSET (GdmManagerClass, display_removed), NULL, NULL, - g_cclosure_marshal_VOID__STRING, + g_cclosure_marshal_VOID__OBJECT, G_TYPE_NONE, - 1, G_TYPE_STRING); + 1, G_TYPE_OBJECT); g_object_class_install_property (object_class, PROP_XDMCP_ENABLED, diff --git a/daemon/gdm-manager.h b/daemon/gdm-manager.h index 4482bdd..ad0eecf 100644 --- a/daemon/gdm-manager.h +++ b/daemon/gdm-manager.h @@ -24,6 +24,7 @@ #include <glib-object.h> +#include "gdm-display.h" #include "gdm-manager-glue.h" G_BEGIN_DECLS @@ -50,7 +51,7 @@ typedef struct void (* display_added) (GdmManager *manager, const char *id); void (* display_removed) (GdmManager *manager, - const char *id); + GdmDisplay *display); } GdmManagerClass; typedef enum
From: Markus Koschany <a...@debian.org> Date: Thu, 16 Aug 2018 13:22:13 +0200 Subject: CVE-2018-14424_part2 Bug-Upstream: https://gitlab.gnome.org/GNOME/gdm/issues/401 Origin: https://gitlab.gnome.org/GNOME/gdm/commit/765b306c364885dd89d47fe9fe8618ce6a467bc1 --- daemon/gdm-display.c | 48 ++++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/daemon/gdm-display.c b/daemon/gdm-display.c index 727d270..ef08bc1 100644 --- a/daemon/gdm-display.c +++ b/daemon/gdm-display.c @@ -1209,30 +1209,30 @@ register_display (GdmDisplay *display) display->priv->object_skeleton = g_dbus_object_skeleton_new (display->priv->id); display->priv->display_skeleton = GDM_DBUS_DISPLAY (gdm_dbus_display_skeleton_new ()); - g_signal_connect (display->priv->display_skeleton, "handle-get-id", - G_CALLBACK (handle_get_id), display); - g_signal_connect (display->priv->display_skeleton, "handle-get-remote-hostname", - G_CALLBACK (handle_get_remote_hostname), display); - g_signal_connect (display->priv->display_skeleton, "handle-get-seat-id", - G_CALLBACK (handle_get_seat_id), display); - g_signal_connect (display->priv->display_skeleton, "handle-get-timed-login-details", - G_CALLBACK (handle_get_timed_login_details), display); - g_signal_connect (display->priv->display_skeleton, "handle-get-x11-authority-file", - G_CALLBACK (handle_get_x11_authority_file), display); - g_signal_connect (display->priv->display_skeleton, "handle-get-x11-cookie", - G_CALLBACK (handle_get_x11_cookie), display); - g_signal_connect (display->priv->display_skeleton, "handle-get-x11-display-name", - G_CALLBACK (handle_get_x11_display_name), display); - g_signal_connect (display->priv->display_skeleton, "handle-get-x11-display-number", - G_CALLBACK (handle_get_x11_display_number), display); - g_signal_connect (display->priv->display_skeleton, "handle-is-local", - G_CALLBACK (handle_is_local), display); - g_signal_connect (display->priv->display_skeleton, "handle-is-initial", - G_CALLBACK (handle_is_initial), display); - g_signal_connect (display->priv->display_skeleton, "handle-add-user-authorization", - G_CALLBACK (handle_add_user_authorization), display); - g_signal_connect (display->priv->display_skeleton, "handle-remove-user-authorization", - G_CALLBACK (handle_remove_user_authorization), display); + g_signal_connect_object (display->priv->display_skeleton, "handle-get-id", + G_CALLBACK (handle_get_id), display, 0); + g_signal_connect_object (display->priv->display_skeleton, "handle-get-remote-hostname", + G_CALLBACK (handle_get_remote_hostname), display, 0); + g_signal_connect_object (display->priv->display_skeleton, "handle-get-seat-id", + G_CALLBACK (handle_get_seat_id), display, 0); + g_signal_connect_object (display->priv->display_skeleton, "handle-get-timed-login-details", + G_CALLBACK (handle_get_timed_login_details), display, 0); + g_signal_connect_object (display->priv->display_skeleton, "handle-get-x11-authority-file", + G_CALLBACK (handle_get_x11_authority_file), display, 0); + g_signal_connect_object (display->priv->display_skeleton, "handle-get-x11-cookie", + G_CALLBACK (handle_get_x11_cookie), display, 0); + g_signal_connect_object (display->priv->display_skeleton, "handle-get-x11-display-name", + G_CALLBACK (handle_get_x11_display_name), display, 0); + g_signal_connect_object (display->priv->display_skeleton, "handle-get-x11-display-number", + G_CALLBACK (handle_get_x11_display_number), display, 0); + g_signal_connect_object (display->priv->display_skeleton, "handle-is-local", + G_CALLBACK (handle_is_local), display, 0); + g_signal_connect_object (display->priv->display_skeleton, "handle-is-initial", + G_CALLBACK (handle_is_initial), display, 0); + g_signal_connect_object (display->priv->display_skeleton, "handle-add-user-authorization", + G_CALLBACK (handle_add_user_authorization), display, 0); + g_signal_connect_object (display->priv->display_skeleton, "handle-remove-user-authorization", + G_CALLBACK (handle_remove_user_authorization), display, 0); g_dbus_object_skeleton_add_interface (display->priv->object_skeleton, G_DBUS_INTERFACE_SKELETON (display->priv->display_skeleton));
