[exim-dev] Security issues in exim4 local delivery

Top Page
Delete this message
Reply to this message
Author: Dan Rosenberg
Date:  
To: nigel
CC: exim-dev
Subject: [exim-dev] Security issues in exim4 local delivery
Hi. I've discovered two security vulnerabilities in Exim4:

1. When Exim is used with a world-writable mail directory with the
sticky-bit set, local users may create hard links to other non-root
users' files at the expected location of those users' mailboxes,
causing their files to be written to upon mail delivery. This could
be used to create denial-of-service conditions or potentially escalate
privileges to those of targeted users. This issue has been assigned
CVE-2010-2023.

2. When MBX locking is enabled, local users may exploit a race
condition to change permissions of other non-root users' files,
leading to denial-of-service conditions or potentially privilege
escalation, or to create new files owned by other users in
unauthorized locations. This issue has been assigned CVE-2010-2024.

I've attached a patch for these issues. The first issue is resolved
by checking the link count of the file before writing - since we're
assuming a sticky bit, there's no potential for race conditions if the
file is successfully opened (because once a hard link to another
user's file is created, it cannot be removed). The second issue is
resolved by first attempting to open the file without O_CREAT. If it
succeeds, then we use fstat() to check that the file is not a symlink,
etc. If the open doesn't succeed, then we make another attempt using
O_CREAT | O_EXCL. If this fails, something fishy is going on.
Otherwise, we obtain a safe lock file.

Let me know if you have any questions about these issues, or have any
problems with the patch. Even though neither of these two
vulnerabilities affects many downstream distributions by default
(since sticky-bit mail directories are becoming more rare and MBX
locking isn't used by many distributions), I'd like to publish an
advisory for these issues independently once you have released a fix.
I'd appreciate it if you kept me posted on any progress in regards to
these issues.

Regards,
Dan Rosenberg
--- exim-4.71/src/transports/appendfile.c.orig    2010-05-09 15:54:02.000000000 -0400
+++ exim-4.71/src/transports/appendfile.c    2010-05-09 16:38:54.000000000 -0400
@@ -1806,6 +1806,18 @@
         goto RETURN;
         }


+      /* Just in case this is a sticky-bit mail directory, we don't want
+      users to be able to create hard links to other users' files. */
+
+      if (statbuf.st_nlink != 1)
+        {
+        addr->basic_errno = ERRNO_NOTREGULAR;
+        addr->message = string_sprintf("mailbox %s%s has too many links (%d)",
+          filename, islink? " (symlink)" : "", statbuf.st_nlink);
+        goto RETURN;
+
+        }
+
       /* If symlinks are permitted (not recommended), the lstat() above will
       have found the symlink. Its ownership has just been checked; go round
       the loop again, using stat() instead of lstat(). That will never yield a
@@ -2005,8 +2017,25 @@
         sprintf(CS mbx_lockname, "/tmp/.%lx.%lx", (long)statbuf.st_dev,
           (long)statbuf.st_ino);


-        if (Ulstat(mbx_lockname, &statbuf) >= 0)
+        /* First, attempt to open the file without O_CREAT, and call
+           fstat() to check if it's a hard link or symlink.  If this 
+           open() fails because the lockfile does not already exist, 
+           then we can safely try again using O_CREAT | O_EXCL.  If 
+           this fails, something fishy is going on.  This way, the 
+           chmod() call is safe, because we're guaranteed that the 
+           lockfile is not a symlink or hard link.  We assume /tmp 
+           has a sticky-bit, so there's no potential for a race condition 
+           once the file has been properly created. */
+        mbx_lockfd = Uopen(mbx_lockname, O_RDWR, ob->lockfile_mode);
+        if (mbx_lockfd >= 0)
           {
+          if (fstat(mbx_lockfd, &statbuf))
+            {
+            addr->basic_errno = ERRNO_LOCKFAILED;
+            addr->message = string_sprintf("fstat failed on MBX lock file %s",
+              mbx_lockname);
+            goto RETURN;
+            }
           if ((statbuf.st_mode & S_IFMT) == S_IFLNK)
             {
             addr->basic_errno = ERRNO_LOCKFAILED;
@@ -2023,15 +2052,18 @@
             }
           }


-        mbx_lockfd = Uopen(mbx_lockname, O_RDWR | O_CREAT, ob->lockfile_mode);
-        if (mbx_lockfd < 0)
+        /* File did not already exist */
+        else
           {
-          addr->basic_errno = ERRNO_LOCKFAILED;
-          addr->message = string_sprintf("failed to open MBX lock file %s :%s",
-            mbx_lockname, strerror(errno));
-          goto RETURN;
+          mbx_lockfd = Uopen(mbx_lockname, O_RDWR | O_CREAT | O_EXCL, ob->lockfile_mode);
+          if (mbx_lockfd < 0)
+            {
+            addr->basic_errno = ERRNO_LOCKFAILED;
+            addr->message = string_sprintf("failed to open MBX lock file %s :%s",
+              mbx_lockname, strerror(errno));
+            goto RETURN;
+            }
           }
-
         (void)Uchmod(mbx_lockname, ob->lockfile_mode);


         if (apply_lock(mbx_lockfd, F_WRLCK, ob->use_fcntl,