On Sat, Apr 20, 2013 at 12:09:49PM -0000, Mutt wrote:
>  mk[sd]temp() exist as handy temp creators that avoid foot shooting.
>  But if you care about the resultant filename, you can make your
>  own. That's mentioned in the open(.. O_CREAT|O_EXCL ..) in the spec
>  below.

Sure, you can make your own custom temp file creator, but the whole
point of having a library function is that getting this right is much
harder than it seems at first glance, and the OS should make it easy
for you.  But besides, that IS what Mutt has... its own custom temp
file creator.  Which uses mktemp() to generate file names.  Which is
totally fine.

>  # mkstemp() - secure family
>  http://pubs.opengroup.org/onlinepubs/9699919799/functions/mkstemp.html
>  # mktemp() - insecure, removed from spec

mktemp() IS NOT insecure.  The way it has been used over the years is.
The warning is, in fact, a lie; it's an overreaction to a related but
DIFFERENT problem.   Writing your own random filename generator would
essentially amount to re-writing mktemp(), except that it would likely
be less secure (potentially due to insufficient randomness, etc.),
unless you knew what you were doing.  So replacing mktemp() is
pointless.  So long as the weakness is documented (which it is), and
the way to use it correctly is understood (which it is), there's no
problem with mktemp().

It's unfortunate if there are redundant code blocks, but that is not a
bug, per se.  It might be worth opening a bug to get them cleaned up.

If there are calls to fopen() which are for writing, these should be
looked at, and if they're correct, a comment to that effect is
probably worthwhile, in similar fashion to the sprintf() comments...

Regardless, concerning the mktemp compiler warning, the easiest and
best solution is to ignore the warning.  It is patently wrong.

> It's the O_EXCL that protects you, not the stats. 

If you mean stat(), I don't think I ever said anything about stat(),
unless you're talking about the stat() which mktemp() does to try to
make sure it does not give you a file name that already exists.
You're correct that this does not protect you...  No one here ever
said it did.

If you're talking about the file modes, then you're mistaken.  It's
the combination of O_EXCL AND the file mode that protects you.  YOU
NEED BOTH.  An attacker could otherwise simply wait until you've
created your temp file, open it for write, and write their exploit
into it, right over whatever you wrote.  Is this easy?  No (the
writing is if your file modes allow it; the guessing of the correct
file name is not).  But it has been demonstrated that brute-forcing it
IS possible.

>  'When you go to create it' implies beforehand, which is wrong.
>  Only the O_EXCL can save you from the race.

No, it doesn't imply that at all. When you go to create it: 

  open(..., O_CREAT|O_EXCL, 0600);

This is what Mutt does.
 

-- 
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 due to spam prevention.  Sorry for the inconvenience.

Attachment: pgpMpmiohXWjz.pgp
Description: PGP signature

Reply via email to