[mutt security] tempfile race in mutt

[roessler () GUUG DE (Thomas Roessler)]

--6sX45UoQRIJXqkqR
Content-Type: multipart/mixed; boundary="lrZ03NoBR/3+SXJZ"


--lrZ03NoBR/3+SXJZ
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: quoted-printable

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 <hamish () debian org>
> 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.
> =20
> 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=20
> 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
--=20
http://home.pages.de/~roessler/

--lrZ03NoBR/3+SXJZ
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="patch-0.95.3.tlr.tmp_race.1"
Content-Transfer-Encoding: quoted-printable

Index: attach.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
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 @@
      =20
       memset (&s, 0, sizeof (s));
       if (flags =3D=3D M_SAVE_APPEND)
-       s.fpout =3D safe_fopen (path, "a");
-      else
+       s.fpout =3D fopen (path, "a");
+      else if (flags =3D=3D M_SAVE_OVERWRITE)
        s.fpout =3D fopen (path, "w");
+      else
+       s.fpout =3D safe_fopen (path, "w");
       if (s.fpout =3D=3D NULL)
       {
        mutt_perror ("fopen");
@@ -771,9 +773,12 @@
   s.flags =3D displaying ? M_DISPLAY : 0;
=20
   if (flags =3D=3D M_SAVE_APPEND)
-    s.fpout =3D safe_fopen (path, "a");
-  else
+    s.fpout =3D fopen (path, "a");
+  else if (flags =3D=3D M_SAVE_OVERWRITE)
     s.fpout =3D fopen (path, "w");
+  else
+    s.fpout =3D safe_fopen (path, "w");
+
   if (s.fpout =3D=3D NULL)
   {
     perror ("fopen");
Index: lib.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
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 @@
=20
       case 2: /* append */
         *append =3D M_SAVE_APPEND;
+        break;
       case 1: /* overwrite */
-       ;
+        *append =3D M_SAVE_OVERWRITE;
+        break;
     }
   }
   return 0;
Index: mutt.h
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
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,
=20
   /* Options for mutt_save_attachment */
-  M_SAVE_APPEND
+  M_SAVE_APPEND,
+  M_SAVE_OVERWRITE
 };
=20
 /* possible arguments to set_quadoption() */
Index: rfc1524.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
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"
=20
-#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>
=20
 /* 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;
  =20
   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) !=3D 0)
+    if (lstat (s, &sb) =3D=3D -1 && errno =3D=3D ENOENT)
       return;
     if ((period =3D strrchr (tmp, '.')) !=3D NULL)
       *period =3D 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;

--lrZ03NoBR/3+SXJZ--

--6sX45UoQRIJXqkqR
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: 2.6.3in

iQEVAwUBNtj+OdImKUTOasbBAQHDIwf+LYaFvSAlYjiTJQqt0TpvORvcDDkdOgul
FszJZ5BjImMKWW+ROjrogW19yhBBvjujie3u5srRwWLOa0hutijfFzQEkpaBF+IX
kQ3+eJ+DuGxvlruQiMyU5wrTIvLASPE9DBfjeRoJSVj/cD3vCu8/1ud7SGIuDPtc
HaI0res0WmzrhKS+EXr3Wpvef6d+DP/KlkunwgDP7vOlX0yKvVHwZ5ZXDfTKhUrK
6a+ckyndrBUjViINOuNF6nLSEtsJvUW7Ueph8y/I6P51llVyZepwgAbVOsCzMpHy
qX/MgLwT/8sNejUyzAKTwO8MUjJwmp/wP2H48drB19GuCFsh+wFJRA==
=uKmW
-----END PGP SIGNATURE-----

--6sX45UoQRIJXqkqR--