mutt.tempfile.race.txt

`Date: Sun, 28 Feb 1999 09:28:43 +0100  
From: Thomas Roessler <[email protected]>  
To: [email protected]  
Subject: [mutt security] tempfile race in mutt  
Parts/Attachments:  
1.1 Shown ~39 lines Text  
1.2 OK ~134 lines Text  
2 475 bytes Application  
----------------------------------------  
  
An anonymous Debian developer forwarded the following message from  
debian-private to me:  
  
> Date: Sun, 28 Feb 1999 01:14:14 +1100  
> From: Hamish Moffatt <[email protected]>  
> To: [email protected]  
> Subject: mutt viewing html  
>  
> On my hamm system, when viewing text/html messages, mutt writes the  
> text to /tmp/mutt.html then calls lynx on that file.  
>   
> I found that if I made a symlink like  
>  
> /tmp/mutt.html -> /home/hamish/blah  
>  
> then viewing an HTML message, the file "blah" (which previously   
> did not exist) contained 4k of nulls. Mutt also deleted the symlink  
> afterwards.  
  
The behaviour Hamish describes exhibits two different problems:  
  
- The temporary file name generator we use when interpreting  
nametemplates from the mailcap file was broken since it used  
access(2) to check for the existance of a file. Obviously, this  
doesn't detect dangling symlinks.  
  
- There are some attachment-handling functions which don't avoid  
race conditions by using our safe_fopen() function, but relied on  
stock libc fopen(3) instead.  
  
The attached patch against mutt 0.95.3 is supposed to fix these  
problems. I plan to release mutt 0.95.4 tomorrow.  
  
As a work-around, you can use a private $TMPDIR with mutt. (This  
may be a good idea in general.)  
  
tlr  
--   
http://home.pages.de/~roessler/  
  
  
Index: attach.c  
===================================================================  
RCS file: /home/roessler/cvsroot/mutt/attach.c,v  
retrieving revision 2.1.4.4  
diff -u -u -r2.1.4.4 attach.c  
--- attach.c 1999/02/10 22:02:02 2.1.4.4  
+++ attach.c 1999/02/28 08:06:08  
@@ -707,9 +707,11 @@  
  
memset (&s, 0, sizeof (s));  
if (flags == M_SAVE_APPEND)  
- s.fpout = safe_fopen (path, "a");  
- else  
+ s.fpout = fopen (path, "a");  
+ else if (flags == M_SAVE_OVERWRITE)  
s.fpout = fopen (path, "w");  
+ else  
+ s.fpout = safe_fopen (path, "w");  
if (s.fpout == NULL)  
{  
mutt_perror ("fopen");  
@@ -771,9 +773,12 @@  
s.flags = displaying ? M_DISPLAY : 0;  
  
if (flags == M_SAVE_APPEND)  
- s.fpout = safe_fopen (path, "a");  
- else  
+ s.fpout = fopen (path, "a");  
+ else if (flags == M_SAVE_OVERWRITE)  
s.fpout = fopen (path, "w");  
+ else  
+ s.fpout = safe_fopen (path, "w");  
+  
if (s.fpout == NULL)  
{  
perror ("fopen");  
Index: lib.c  
===================================================================  
RCS file: /home/roessler/cvsroot/mutt/lib.c,v  
retrieving revision 2.2.4.5  
diff -u -u -r2.2.4.5 lib.c  
--- lib.c 1999/02/10 22:02:04 2.2.4.5  
+++ lib.c 1999/02/28 08:06:08  
@@ -803,8 +803,10 @@  
  
case 2: /* append */  
*append = M_SAVE_APPEND;  
+ break;  
case 1: /* overwrite */  
- ;  
+ *append = M_SAVE_OVERWRITE;  
+ break;  
}  
}  
return 0;  
Index: mutt.h  
===================================================================  
RCS file: /home/roessler/cvsroot/mutt/mutt.h,v  
retrieving revision 2.1.4.6  
diff -u -u -r2.1.4.6 mutt.h  
--- mutt.h 1999/02/10 21:42:31 2.1.4.6  
+++ mutt.h 1999/02/28 08:06:08  
@@ -227,7 +227,8 @@  
M_NEW_SOCKET,  
  
/* Options for mutt_save_attachment */  
- M_SAVE_APPEND  
+ M_SAVE_APPEND,  
+ M_SAVE_OVERWRITE  
};  
  
/* possible arguments to set_quadoption() */  
Index: rfc1524.c  
===================================================================  
RCS file: /home/roessler/cvsroot/mutt/rfc1524.c,v  
retrieving revision 2.0.4.4  
diff -u -u -r2.0.4.4 rfc1524.c  
--- rfc1524.c 1999/02/10 22:02:06 2.0.4.4  
+++ rfc1524.c 1999/02/28 08:06:09  
@@ -29,11 +29,14 @@  
#include "mutt.h"  
#include "rfc1524.h"  
  
-#include <ctype.h>  
+#include <string.h>  
#include <stdlib.h>  
-#include <unistd.h>  
+#include <ctype.h>  
+  
+#include <sys/stat.h>  
#include <sys/wait.h>  
-#include <string.h>  
+#include <errno.h>  
+#include <unistd.h>  
  
/* The command semantics include the following:  
* %s is the filename that contains the mail body data  
@@ -445,6 +448,7 @@  
char tmp[_POSIX_PATH_MAX];  
char *period;  
size_t sl;  
+ struct stat sb;  
  
strfcpy (buf, NONULL (Tempdir), sizeof (buf));  
mutt_expand_path (buf, sizeof (buf));  
@@ -457,7 +461,7 @@  
{  
strfcpy (tmp, s, sizeof (tmp));  
snprintf (s, l, "%s/%s", buf, tmp);  
- if (access (s, F_OK) != 0)  
+ if (lstat (s, &sb) == -1 && errno == ENOENT)  
return;  
if ((period = strrchr (tmp, '.')) != NULL)  
*period = 0;  
@@ -610,6 +614,11 @@  
* This function returns 0 on successful move, 1 on old file doesn't exist,  
* 2 on new file already exists, and 3 on other failure.  
*/  
+  
+/* note on access(2) use: No dangling symlink problems here due to  
+ * safe_fopen().  
+ */  
+  
int mutt_rename_file (char *oldfile, char *newfile)  
{  
FILE *ofp, *nfp;  
  
`