`Date: Sun, 28 Feb 1999 09:28:43 +0100 From: Thomas Roessler To: BUGTRAQ@netspace.org 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 > To: security@debian.org > 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 +#include #include -#include +#include + +#include #include -#include +#include +#include /* 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; `

Query Builder