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
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, , 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);
