From: Chong Yidong <cyd@stupidchicken.com>
Date: Fri, 2 Apr 2010 15:24:16 +0000 (-0400)
Subject: Fix permissions handling (CVE-2010-0825).
X-Git-Tag: emacs-pretest-23.1.95~1
X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=c8f19d81022a1f17cd14d1553a9c063f4eabe3c2;p=emacs.git

Fix permissions handling (CVE-2010-0825).

* movemail.c (main): Check return values of setuid.  Avoid
possibility of symlink attack when movemail is setgid mail
(CVE-2010-0825).
---

diff --git a/lib-src/ChangeLog b/lib-src/ChangeLog
index e97f2672074..826f5c4e6ad 100644
--- a/lib-src/ChangeLog
+++ b/lib-src/ChangeLog
@@ -1,3 +1,9 @@
+2010-04-02  Dan Rosenberg  <dan.j.rosenberg@gmail.com>  (tiny change)
+
+	* movemail.c (main): Check return values of setuid.  Avoid
+	possibility of symlink attack when movemail is setgid mail
+	(CVE-2010-0825).
+
 2010-03-19  Tetsurou Okazaki  <okazaki@be.to>  (tiny change)
 
 	* Makefile.in (uninstall): Handle the case where archlibdir does
diff --git a/lib-src/movemail.c b/lib-src/movemail.c
index e0eb4d48b89..ae51df3d39c 100644
--- a/lib-src/movemail.c
+++ b/lib-src/movemail.c
@@ -197,6 +197,9 @@ main (argc, argv)
 # define ARGSTR "p"
 #endif /* MAIL_USE_POP */
 
+  uid_t real_gid = getgid();
+  uid_t priv_gid = getegid();
+
 #ifdef WINDOWSNT
   /* Ensure all file i/o is in binary mode. */
   _fmode = _O_BINARY;
@@ -247,25 +250,6 @@ main (argc, argv)
   if (*outname == 0)
     fatal ("Destination file name is empty", 0, 0);
 
-  /* Check access to output file.  */
-  if (access (outname, F_OK) == 0 && access (outname, W_OK) != 0)
-    pfatal_with_name (outname);
-
-  /* Also check that outname's directory is writable to the real uid.  */
-  {
-    char *buf = (char *) xmalloc (strlen (outname) + 1);
-    char *p;
-    strcpy (buf, outname);
-    p = buf + strlen (buf);
-    while (p > buf && !IS_DIRECTORY_SEP (p[-1]))
-      *--p = 0;
-    if (p == buf)
-      *p++ = '.';
-    if (access (buf, W_OK) != 0)
-      pfatal_with_name (buf);
-    free (buf);
-  }
-
 #ifdef MAIL_USE_POP
   if (!strncmp (inname, "po:", 3))
     {
@@ -277,15 +261,12 @@ main (argc, argv)
       exit (status);
     }
 
-  setuid (getuid ());
+  if (setuid (getuid ()) < 0)
+    fatal ("Failed to drop privileges", 0, 0);
+
 #endif /* MAIL_USE_POP */
 
 #ifndef DISABLE_DIRECT_ACCESS
-
-  /* Check access to input file.  */
-  if (access (inname, R_OK | W_OK) != 0)
-    pfatal_with_name (inname);
-
 #ifndef MAIL_USE_MMDF
 #ifndef MAIL_USE_SYSTEM_LOCK
 #ifdef MAIL_USE_MAILLOCK
@@ -379,7 +360,8 @@ main (argc, argv)
       time_t touched_lock, now;
 #endif
 
-      setuid (getuid ());
+      if (setuid (getuid ()) < 0 || setegid (real_gid) < 0)
+	fatal ("Failed to drop privileges", 0, 0);
 
 #ifndef MAIL_USE_MMDF
 #ifdef MAIL_USE_SYSTEM_LOCK
@@ -405,6 +387,9 @@ main (argc, argv)
       if (outdesc < 0)
 	pfatal_with_name (outname);
 
+      if (setegid (priv_gid) < 0)
+	fatal ("Failed to regain privileges", 0, 0);
+
       /* This label exists so we can retry locking
 	 after a delay, if it got EAGAIN or EBUSY.  */
     retry_lock:
@@ -498,6 +483,10 @@ main (argc, argv)
 	pfatal_and_delete (outname);
 #endif
 
+      /* Prevent symlink attacks truncating other users' mailboxes */
+      if (setegid (real_gid) < 0)
+	fatal ("Failed to drop privileges", 0, 0);
+
       /* Check to make sure no errors before we zap the inbox.  */
       if (close (outdesc) != 0)
 	pfatal_and_delete (outname);
@@ -529,6 +518,10 @@ main (argc, argv)
 	}
 #endif /* not MAIL_USE_SYSTEM_LOCK */
 
+      /* End of mailbox truncation */
+      if (setegid (priv_gid) < 0)
+	fatal ("Failed to regain privileges", 0, 0);
+
 #ifdef MAIL_USE_MAILLOCK
       /* This has to occur in the child, i.e., in the process that
          acquired the lock! */