Merge lp:~pitti/lightdm/write-user-files-as-user into lp:lightdm

[Martin Pitt]

This drops privileges before writing ~/.dmrc and ~/.Xauthority instead of using
chmod(), to guard against symlink attacks and other potential privilege
escalation.

=== modified file 'NEWS'
--- NEWS	2011-09-13 05:24:24 +0000
+++ NEWS	2011-09-13 13:54:24 +0000
@@ -2,6 +2,7 @@
 
     * Only unlock displays if switched to from greeter
     * Add VNC server support
+    * Do not write ~/.dmrc and ~/.Xauthority as root. [CVE-2011-3349]
 
 Overview of changes in lightdm 0.9.5
 

=== modified file 'src/dmrc.c'
--- src/dmrc.c	2011-08-23 01:39:57 +0000
+++ src/dmrc.c	2011-09-13 13:54:24 +0000
@@ -9,6 +9,8 @@
  * license.
  */
 
+/* for setres*id() */
+#define _GNU_SOURCE
 #include <errno.h>
 #include <string.h>
 #include <unistd.h>
@@ -80,11 +82,22 @@
     /* Update the users .dmrc */
     if (user)
     {
+	gboolean drop_privs = (geteuid () == 0);
+
+	/* Guard against privilege escalation through symlinks, etc. */
+	if (drop_privs)
+	{
+	    g_assert (setresgid (user_get_gid (user), user_get_gid (user), -1) == 0);
+	    g_assert (setresuid (user_get_uid (user), user_get_uid (user), -1) == 0);
+	}
         path = g_build_filename (user_get_home_directory (user), ".dmrc", NULL);
         g_file_set_contents (path, data, length, NULL);
-        if (getuid () == 0 && chown (path, user_get_uid (user), user_get_gid (user)) < 0)
-            g_warning ("Error setting ownership on %s: %s", path, strerror (errno));
         g_free (path);
+	if (drop_privs)
+	{
+	    g_assert (setresuid (0, 0, -1) == 0);
+	    g_assert (setresgid (0, 0, -1) == 0);
+	}
     }
 
     /* Update the .dmrc cache */

=== modified file 'src/xauthority.c'
--- src/xauthority.c	2011-09-07 05:37:55 +0000
+++ src/xauthority.c	2011-09-13 13:54:24 +0000
@@ -9,6 +9,8 @@
  * license.
  */
 
+/* for setres*id() */
+#define _GNU_SOURCE
 #include <string.h>
 #include <errno.h>
 #include <unistd.h>
@@ -244,6 +246,16 @@
     XAuthority *a;
     gboolean result;
     gboolean matched = FALSE;
+    gboolean drop_privs = (user && geteuid () == 0);
+    gboolean retval = FALSE;
+
+    /* Guard against privilege escalation through symlinks, etc. */
+    if (drop_privs)
+    {
+	g_debug ("Dropping privileges to uid %i", user_get_uid (user));
+	g_assert (setresgid (user_get_gid (user), user_get_gid (user), -1) == 0);
+	g_assert (setresuid (user_get_uid (user), user_get_uid (user), -1) == 0);
+    }
 
     /* Read out existing records */
     if (mode != XAUTH_WRITE_MODE_SET)
@@ -317,7 +329,7 @@
 
     output_stream = g_file_replace (file, NULL, FALSE, G_FILE_CREATE_PRIVATE, NULL, error);
     if (!output_stream)
-        return FALSE;
+        goto out;
 
     /* Workaround because g_file_replace () generates a file does not exist error even though it can replace it */
     g_clear_error (error);
@@ -345,18 +357,18 @@
     g_object_unref (output_stream);
 
     if (!result)
-        return FALSE;
+        goto out;
 
-    /* NOTE: Would like to do:
-     * g_file_set_attribute_string (file, G_FILE_ATTRIBUTE_OWNER_USER, username, G_FILE_QUERY_INFO_NONE, NULL, error))
-     * but not supported. */
-    if (user && getuid () == 0)
+    retval = TRUE;
+  
+out:
+    /* reclaim privileges */
+    if (drop_privs)
     {
-        if (chown (g_file_get_path (file), user_get_uid (user), user_get_gid (user)) < 0)
-            g_warning ("Failed to set authorization owner: %s", strerror (errno));
+	g_assert (setresuid (0, 0, -1) == 0);
+	g_assert (setresgid (0, 0, -1) == 0);
     }
-  
-    return TRUE;
+    return retval;
 }
 
 static void