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