Re: status of the gdm3 security update
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 <apo@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 <apo@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));
Reply to: