summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorChong Yidong <cyd@stupidchicken.com>2010-04-02 11:24:16 -0400
committerChong Yidong <cyd@stupidchicken.com>2010-04-02 11:24:16 -0400
commitc8f19d81022a1f17cd14d1553a9c063f4eabe3c2 (patch)
treee785f91b914962e7fb8e38553b042903efbfd301
parente4b204d75ff161c1f4823eee5187fd0a64511f60 (diff)
downloademacs-c8f19d81022a1f17cd14d1553a9c063f4eabe3c2.tar.gz
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).
-rw-r--r--lib-src/ChangeLog6
-rw-r--r--lib-src/movemail.c45
2 files changed, 25 insertions, 26 deletions
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! */