GNU bug report logs - #8836
mktemp-related race condition in movemail

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Paul Eggert <eggert <at> cs.ucla.edu>
Subject: bug#8836: closed (Re: bug#8836: Acknowledgement (mktemp-related
 race condition in movemail))
Date: Fri, 10 Jun 2011 17:52:02 +0000

[Message part 1 (text/plain, inline)]

Your bug report

#8836: mktemp-related race condition in movemail

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to 8836 <at> debbugs.gnu.org.

-- 
8836: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=8836
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: 8836-done <at> debbugs.gnu.org
Subject: Re: bug#8836: Acknowledgement (mktemp-related race condition in
	movemail)
Date: Fri, 10 Jun 2011 10:50:58 -0700

I committed the fix in bzr 104555 on the trunk.

[Message part 3 (message/rfc822, inline)]

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: bug-gnu-emacs <at> gnu.org
Subject: mktemp-related race condition in movemail
Date: Fri, 10 Jun 2011 10:46:47 -0700

There is a race condition in lib-src/movemail.c, and some
related bugs.  I plan to commit the following patch.
This one has security implications, so I'm filing a bug
report to give others a bigger heads-up.

I found this one via GCC 4.6.0's static analysis.

* movemail.c: Fix race condition and related bugs.
(main) [!MAIL_USE_SYSTEM_LOCK]: Prefer mkstemp to mktemp, as this
fixes some race conditions.  Report mkstemp/mktemp errno rather
than a possibly-garbage errno.  Reinitialize the template each
time through the loop, as earlier mkstemp/mktemp calls could have
trashed it.  Pass 0600 (not 0666) to mktemp, for consistency
with mkstemp; the permissions don't matter anyway.
=== modified file 'lib-src/movemail.c'
--- lib-src/movemail.c	2011-04-16 21:20:25 +0000
+++ lib-src/movemail.c	2011-06-10 17:30:52 +0000
@@ -168,8 +168,9 @@
 #ifndef MAIL_USE_SYSTEM_LOCK
   struct stat st;
   int tem;
-  char *lockname, *p;
+  char *lockname;
   char *tempname;
+  size_t inname_dirlen;
   int desc;
 #endif /* not MAIL_USE_SYSTEM_LOCK */

@@ -298,26 +299,38 @@
 	 to bug-gnu-emacs <at> prep.ai.mit.edu so we can fix it.  */

       lockname = concat (inname, ".lock", "");
-      tempname = (char *) xmalloc (strlen (inname) + strlen ("EXXXXXX") + 1);
-      strcpy (tempname, inname);
-      p = tempname + strlen (tempname);
-      while (p != tempname && !IS_DIRECTORY_SEP (p[-1]))
-	p--;
-      *p = 0;
-      strcpy (p, "EXXXXXX");
-      mktemp (tempname);
-      unlink (tempname);
+      for (inname_dirlen = strlen (inname);
+	   inname_dirlen && !IS_DIRECTORY_SEP (inname[inname_dirlen - 1]);
+	   inname_dirlen--)
+	continue;
+      tempname = (char *) xmalloc (inname_dirlen + sizeof "EXXXXXX");

       while (1)
 	{
 	  /* Create the lock file, but not under the lock file name.  */
 	  /* Give up if cannot do that.  */
-	  desc = open (tempname, O_WRONLY | O_CREAT | O_EXCL, 0666);
+
+	  memcpy (tempname, inname, inname_dirlen);
+	  strcpy (tempname + inname_dirlen, "EXXXXXX");
+#ifdef HAVE_MKSTEMP
+	  desc = mkstemp (tempname);
+#else
+	  mktemp (tempname);
+	  if (!*tempname)
+	    desc = -1;
+	  else
+	    {
+	      unlink (tempname);
+	      desc = open (tempname, O_WRONLY | O_CREAT | O_EXCL, 0600);
+	    }
+#endif
 	  if (desc < 0)
 	    {
+	      int mkstemp_errno = errno;
 	      char *message = (char *) xmalloc (strlen (tempname) + 50);
 	      sprintf (message, "creating %s, which would become the lock file",
 		       tempname);
+	      errno = mkstemp_errno;
 	      pfatal_with_name (message);
 	    }
 	  close (desc);

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.