<<< Date Index >>>     <<< Thread Index >>>

security problem with temp files [was Re: mutt_adv_mktemp() ?]



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