06_drop-privileges-before-writing-user-files cherry-picked from Martin

Pitt merge request. Don't write user files as root to prevent symlinks
attacks [CVE-2011-3349]                                   closes: #639151

git-svn-id: svn://anonscm.debian.org/pkg-xfce/goodies/trunk/lightdm@6021 0c9b3bff-18ee-0310-b944-d1aa2700132f

diff --git a/debian/changelog b/debian/changelog
index 03391768bec05eaed729639dce1ace85f8e0c069..958f1e92d6ab30687a05b5dffffbac80c3eaf327 100644 (file)
--- a/debian/changelog
+++ b/debian/changelog
@@ -7,6 +7,9 @@ lightdm (0.9.5-1) UNRELEASED; urgency=low
     - 05_always-export-XAUTHORITY dropped, included upstream. 
     - 05_dont-add-pkglibexecdir-path added, don't add /usr/lib/lightdm/lightdm
       to the PATH, it's ugly.
+    - 06_drop-privileges-before-writing-user-files cherry-picked from Martin
+      Pitt merge request. Don't write user files as root to prevent symlinks
+      attacks [CVE-2011-3349]                                   closes: #639151
   * debian/rules:
     - don't install gdmflexiserver script for now until the PATH issue is
       solved.

diff --git a/debian/patches/06_drop-privileges-before-writing-user-files.patch b/debian/patches/06_drop-privileges-before-writing-user-files.patch
new file mode 100644 (file)
index 0000000..ad30a3f
--- /dev/null
+++ b/debian/patches/06_drop-privileges-before-writing-user-files.patch
@@ -0,0 +1,106 @@
+=== modified file 'src/dmrc.c'
+Index: lightdm-0.9.5/src/dmrc.c
+===================================================================
+--- lightdm-0.9.5.orig/src/dmrc.c      2011-07-20 05:54:37.000000000 +0200
++++ lightdm-0.9.5/src/dmrc.c   2011-09-13 16:20:50.731421337 +0200
+@@ -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 */
+Index: lightdm-0.9.5/src/xauthority.c
+===================================================================
+--- lightdm-0.9.5.orig/src/xauthority.c        2011-09-07 07:16:54.000000000 +0200
++++ lightdm-0.9.5/src/xauthority.c     2011-09-13 16:20:50.731421337 +0200
+@@ -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

diff --git a/debian/patches/series b/debian/patches/series
index ca7e224ac6570956d2c306935936f5835f5db079..e72af25abec25f43a943336e7d0180164dea06d5 100644 (file)
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -3,3 +3,4 @@
 03_quit-plymouth.patch
 04_default-gtk-greeter-config.patch
 05_dont-add-pkglibexecdir-path.patch
+06_drop-privileges-before-writing-user-files.patch