From eaabecd70f79c89f6bfd912557c0cbb7718d4c63 Mon Sep 17 00:00:00 2001

From: Ray Strode <rstrode@redhat.com>

Date: Mon, 5 Nov 2012 17:07:05 -0500

Subject: [PATCH] daemon: allow NULs in X11 cookie

We currently allow the slave access to its X server via two

mechanisms:

1) we set XAUTHORITY to point to the X servers Xauthority file

2) we call XSetAuthorization with the cookie from the Xauthority file

1) may fail if the user's hostname changes at the wrong moment, and

a bug in the code meant that 2 would fail if NULs are encoded in the

auth cookie.

This commit fixes 2) to work with embedded NUL bytes.

https://bugzilla.gnome.org/show_bug.cgi?id=687691

---

 daemon/gdm-display.c   |  7 ++++++-

 daemon/gdm-display.xml |  4 +++-

 daemon/gdm-slave.c     | 38 ++++++++++++++++++++++++++++----------

 3 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/daemon/gdm-display.c b/daemon/gdm-display.c

index 42f5990..435dc1c 100644

--- a/daemon/gdm-display.c

+++ b/daemon/gdm-display.c

@@ -1106,10 +1106,15 @@ handle_get_x11_cookie (GdmDBusDisplay        *skeleton,

                        GdmDisplay            *display)

 {

         GArray *cookie = NULL;

+        GVariant *variant;

         gdm_display_get_x11_cookie (display, &cookie, NULL);

-        gdm_dbus_display_complete_get_x11_cookie (skeleton, invocation, cookie->data);

+        variant = g_variant_new_fixed_array (G_VARIANT_TYPE_BYTE,

+                                             cookie->data,

+                                             cookie->len,

+                                             sizeof (char));

+        gdm_dbus_display_complete_get_x11_cookie (skeleton, invocation, variant);

         g_array_unref (cookie);

         return TRUE;

diff --git a/daemon/gdm-display.xml b/daemon/gdm-display.xml

index 904e0ae..48d03db 100644

--- a/daemon/gdm-display.xml

+++ b/daemon/gdm-display.xml

@@ -11,7 +11,9 @@

       <arg name="name" direction="out" type="i"/>

     </method>

     <method name="GetX11Cookie">

-      <arg name="x11_cookie" direction="out" type="ay"/>

+      <arg name="x11_cookie" direction="out" type="ay">

+        <annotation name="org.gtk.GDBus.C.ForceGVariant" value="true"/>

+      </arg>

     </method>

     <method name="GetX11AuthorityFile">

       <arg name="filename" direction="out" type="s"/>

diff --git a/daemon/gdm-slave.c b/daemon/gdm-slave.c

index 948406a..15df03a 100644

--- a/daemon/gdm-slave.c

+++ b/daemon/gdm-slave.c

@@ -98,7 +98,7 @@ struct GdmSlavePrivate

         char            *parent_display_name;

         char            *parent_display_x11_authority_file;

         char            *windowpath;

-        char            *display_x11_cookie;

+        GBytes          *display_x11_cookie;

         gboolean         display_is_initial;

         GdmDBusDisplay  *display_proxy;

@@ -665,10 +665,13 @@ gdm_slave_connect_to_x11_display (GdmSlave *slave)

         sigprocmask (SIG_BLOCK, &mask, &omask);

         /* Give slave access to the display independent of current hostname */

-        XSetAuthorization ("MIT-MAGIC-COOKIE-1",

-                           strlen ("MIT-MAGIC-COOKIE-1"),

-                           slave->priv->display_x11_cookie,

-                           strlen (slave->priv->display_x11_cookie));

+        if (slave->priv->display_x11_cookie != NULL) {

+                XSetAuthorization ("MIT-MAGIC-COOKIE-1",

+                                   strlen ("MIT-MAGIC-COOKIE-1"),

+                                   (gpointer)

+                                   g_bytes_get_data (slave->priv->display_x11_cookie, NULL),

+                                   g_bytes_get_size (slave->priv->display_x11_cookie));

+        }

         slave->priv->server_display = XOpenDisplay (slave->priv->display_name);

@@ -736,9 +739,12 @@ gdm_slave_set_slave_bus_name (GdmSlave *slave)

 static gboolean

 gdm_slave_real_start (GdmSlave *slave)

 {

-        gboolean res;

-        char    *id;

-        GError  *error;

+        gboolean    res;

+        char       *id;

+        GError     *error;

+        GVariant   *x11_cookie;

+        const char *x11_cookie_bytes;

+        gsize       x11_cookie_size;

         g_debug ("GdmSlave: Starting slave");

@@ -826,7 +832,7 @@ gdm_slave_real_start (GdmSlave *slave)

         error = NULL;

         res = gdm_dbus_display_call_get_x11_cookie_sync (slave->priv->display_proxy,

-                                                         &slave->priv->display_x11_cookie,

+                                                         &x11_cookie,

                                                          NULL,

                                                          &error);

         if (! res) {

@@ -835,6 +841,18 @@ gdm_slave_real_start (GdmSlave *slave)

                 return FALSE;

         }

+        x11_cookie_bytes = g_variant_get_fixed_array (x11_cookie,

+                                                      &x11_cookie_size,

+                                                      sizeof (char));

+

+        if (x11_cookie_bytes != NULL && x11_cookie_size > 0) {

+                g_bytes_unref (slave->priv->display_x11_cookie);

+                slave->priv->display_x11_cookie = g_bytes_new (x11_cookie_bytes,

+                                                               x11_cookie_size);

+        }

+

+        g_variant_unref (x11_cookie);

+

         error = NULL;

         res = gdm_dbus_display_call_get_x11_authority_file_sync (slave->priv->display_proxy,

                                                                  &slave->priv->display_x11_authority_file,

@@ -2175,7 +2193,7 @@ gdm_slave_finalize (GObject *object)

         g_free (slave->priv->parent_display_name);

         g_free (slave->priv->parent_display_x11_authority_file);

         g_free (slave->priv->windowpath);

-        g_free (slave->priv->display_x11_cookie);

+        g_bytes_unref (slave->priv->display_x11_cookie);

         G_OBJECT_CLASS (gdm_slave_parent_class)->finalize (object);

 }

-- 

1.7.12.1