It would seem that Mutt's temp file creation mechanisms all suffer from a potentially exploitable race condition. Actually there are two: one in functions that call safe_open(), which only affects users creating temp files on NFS file systems (due to the O_EXCL problem), and one in functions that make use of safe_fopen(), because the resulting file is not adequately checked to determine if other users can modify it before it is written to. On Wed, Oct 04, 2006 at 11:19:26AM -0400, Kyle Wheeler wrote: > char tmpfile[POSIX_PATH_MAX] = "/tmp/muttXXXXXX"; > char tmpfile2[POSIX_PATH_MAX]; > char *extension = "foo"; > // mkstemp should come first > fd = mkstemp(tmpfile); > sprintf(tmpfile2, "%s.%s", tmpfile, extension); > rename(tmpfile, tmpfile2); The trouble with this is that while tmpfile is "guaranteed" unique, tmpfile.foo may well not be. :( Granted, it would be hard to exploit, but theoretically not impossible. You still need to be careful to open the file with O_CREAT|O_EXCL and expect that it could fail (which I believe mutt does, though I didn't verify every possible case). Unfortunately even that isn't really enough given that lots of people set TMPDIR to something in their home directory, and have home directories on NFS. Opening a file this way (with O_EXCL) on NFS is broken (and thus so are any implementations of mkstemp() family functions that rely on it). You can do the link(2) trick too, but AFAICT there's no way to guarantee that the file you're creating is unique on NFS without encountering a race condition. This is a weakness in the NFS protocol IIRC, not in the implementation of mkstemp(). [This may not be the case with NFSv4, I am uncertain.] The man page for open(2) mentions this. That's problem #1. The scheme that mutt currently uses for most of its temp files (_mutt_mktemp()/safe_open()) involves using the host name, UID, pid, and an internal counter maintained by mutt. The trouble is, it's fairly likely that a user will go through the exact same steps when they boot their workstation (i.e. when starting mutt immediately after booting the system), so for any particular user the likelihood is surprisingly high that all of these pieces (including the PID) except for the counter will frequently be the same for any given user, thus very guessable. Only the counter will change, and it's always initialized to zero, so that also is not hard to guess. At a glance, I don't see mutt doing anything to check to see if for such files, the user is the only one who can read and/or write to the file. Certainly functions that call safe_open() use O_EXCL, but due to the problems with that working on NFS, that may not be enough. Also NFS or no NFS, with regard to mutt_adv_mktemp() for attachments, calls to safe_fopen() generally use "w+" which means the file will be truncated if it exists, but that doesn't do anything to ensure that the file so created couldn't be moved aside and replaced with a malicious file should the directory be writable by someone else; nor does it do anything to prevent the race where someone else creates the file in between the calls to mktemp() and safe_fopen(). In the latter case, the file will be truncated, but the file is still writable by the owner and can potentially be modified in a malicious way before mutt runs the MIME type's helper program on it. Is this exploitable? It would seem so, though executing such a maneuver is probably exceedingly difficult. An attacker would need to be able to guess the temp file name in advance, know what kind of data was going into it, and be able to craft malicious data which would exploit some problem in the program it was going to be fed into... Still, mutt really ought to be creating a unique directory in which to create its temp files, and before doing so it should make sure the permissions on that directory are 700 and owned by the user. It should also make sure that the parent directory either allows access only to the user, or that it has the sticky bit set. There's still a possible race condition involving creating a unique directory name, but fortunately mkdir(2) always fails if the directory already exists, which largely solves the problem. Further, checking the permissions and ownership on the directory before creating the temp file should eliminate any remaining doubt. On Wed, Oct 04, 2006 at 11:36:43AM -0400, Kyle Wheeler wrote: > On the other hand, mkstemps() would solve the problem, at least for > the OS's that support it, i.e. all modern Unixes (it works on Tru64 > Unix, Solaris, FreeBSD, OpenBSD, NetBSD, MacOS X, OpenSolaris, > Solaris, and Linux). $ uname -smr Linux 2.4.20-20.9 i686 $ man mkstemps No manual entry for mkstemps $ gcc mkstemps.c /home/ddm/tmp/ccvsG8nh.o(.text+0x17): In function `main': : undefined reference to `mkstemps' collect2: ld returned 1 exit status You were saying? This function is not sufficiently portable to rely upon its presence. At best you'd need to provide an alternate implementation, in which case there's no obvious benefit to using it... mutt_adv_mktemp() basically does the same thing already. Note that mkstemps also relies on O_EXCL so it also suffers from a race condition on NFS. -- Derek D. Martin http://www.pizzashack.org/ GPG Key ID: 0xDFBEAD02 -=-=-=-=- This message is posted from an invalid address. Replying to it will result in undeliverable mail. Sorry for the inconvenience. Thank the spammers.
Attachment:
pgpMy81Io8yil.pgp
Description: PGP signature