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.
OWL has a large patch reworking tmp file handling in mutt: http://cvsweb.openwall.com/cgi/cvsweb.cgi/Owl/packages/mutt/mutt-1.4.2.1-owl-tmp.diff
opening the bug since the mail can be found at <http://marc.theaimsgroup.com/?l=mutt-dev&m=115999486426292&w=2> <http://secunia.com/advisories/22613/>
CC'ing maintainers Ubuntu announcement can be found at http://www.ubuntu.com/usn/usn-373-1 so there should be patches available too
Pulling in herd for advise/bumping.
Ok, I've digged in the recent mutt commits and the only reference to temp files / security is the attached one. I'll apply it later today if any of you can ACK the patch. - ferdy
Created attachment 102446 [details, diff] paranoid-temp-file.patch Proposed patch from upstream.
That patch is being applied in mail-client/mutt-1.5.13-r2. If it is not enough to fix this bug please state so and I'll try to get in touch with upstream. - ferdy
Vorlon is this patch enough?
could someone pls verify the patch? (tavis/...?)
mutt-1.5.14 is in the tree now (with this patch included too). I haven't seen more patches related to this bug. Is this bug still considered valid? Do I have to do anything else? - ferdy
Tavis please advise.
> 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); rename() is not vulnerable to a symlink attack, unlike open(). The race conditions explained in the mail needs other successful attacks, or administrator misconfigurations. Non-issue for me. For the O_EXCL which doesn't work on NFS, that may be a weakness, but this would affect all other apps using O_EXCL. It's not mutt specific and it can't simply be solved. Furthermore, /tmp on a NFS... sounds... eccentric. (why not, but in that case it is also a misconfiguration since that will affect many applications). Non-issue for me. I'm an addicted mutt user and despite of that, I think we can close this bug as Invalid.
mmm after a deeper look to the mailing list, the syscall to rename() should be checked (return value equals 0 or -1). Indeed, if the files are: -rw------- 1 falco falco 0 Jun 9 23:46 muttjlNOGW -rw-r--r-- 1 foo foo 5 Jun 9 23:46 muttjlNOGW.foo Then the rename() call fails, mutt continues, and read/write from/to the crafted muttjlNOGW.foo file. Yes, that's a bug (another user could view the mails). Not a security issue (DoS, code exec, or so).
according to the CVE refs, this affects versions 1.5.12 and earlier, and we have version .13 stable and versions .14 and .15 unstable in the tree, so what should we do here? Ferdy/Aggrifis/net-mail, please advise.
I just commited mutt-1.5.16. There is no point in stabling .15 so I'd go for .16 myself once arch people deem it appropiate. We can start removing vulnerable unused versions though. - ferdy
Handling stable marking on bug #178003
finally closing without GLSA wrt the discussion on bug 178003, feel free to reopen if you disagree.
