On Wed, May 24, 2006 at 02:07:19AM +0200, Michael Kerrisk wrote:
> Justin,
>
> > > > tmpnam.3 (and tmpnam_r.3)
> > > > I think this is one of the classically-buggy functions. Since it
> > > > generates a filename, but doesn't ask the kernel to create that file
> > > > atomically, it is easy to pass its return value to fopen() and be
> > > > done
> > > > with it; but, again, this is insecure if you don't use "exclusive"
> > > > mode. It should be fine if you do use it, though.
> > >
> > > No! The problem is that between creation of the name and opening
> > > it (in /tmp, a world writable directory), some other program could
> > > create that file or create it as a symlink, causing the original
> > > program to do the wrong thing.
> > Not in exclusive mode.. In my test it didn't matter if it was a
> > dangling symlink, a real symlink, or a file. Exclusive mode fails if
> > the pathname exists in any way:
>
> <blush> Yes, you are right. I should have read and thought harder
> before I wrote that... My apologies.
>
> In 2.34, the tmpnam.3 man page will include the following
> para:
>
> Although tmpnam(3) generates names that are difficult to
> guess, it is nevertheless possible that between the time
> that tmpnam(3) returns a pathname, and the time that the
> program opens it, another program might create that pathnam
> using open(2), or create it as a symbolic link. This can
> lead to security holes. To avoid such possibilities, use
> the open(2) O_EXCL flag to open the pathname. Or better
> yet, use mkstemp(3) or tmpfile(3).
>
> I added similar text to temnam.3.
>
> I also removed the text "POSIX dictates tmpnam(3)" from mktemp.3.
I would like to propose the following additional changes.
Replace all occurances of the phrase "Never use >XXXXXX<" with "Gcc
warns about the use of >XXXXXX<". This applies to the following:
mktemp
tmpnam
tempnam
This is enough of a practical reason to not use those functions, IMO;
the functions themselves aren't broken, just people's use or
misunderstanding of them. If the functions themselves shouldn't be
used for some other reason, then let's document that.
This is done for mktemp, but I think the real, necessary information
should be documented, instead of implying that 4.3BSD somehow
generates an "insecure temporary file name". It is true that the name
is poor (since it is possible to need more than 26 temp files), but
lets get the real reasons right. "every use of mktemp() is a security
risk." Can we drop that?
Perhaps each function generating a name, (and not opening the file?)
should also include some explicit mention of O_EXCL:
Temporary files must be opened with O_EXCL, and error conditions
detected; to do otherwise introduces a race condition and a security
risk. If a temporary file is ever removed and later reopened,
O_EXCL must be used again.
This applies to:
mktemp
tmpnam
tempnam
?mkdtemp
?mkstemp
tmpfile seems to be the most portable; even mingw32 has it. But, it
doesn't expose the filename. This is easy to work around with some
simple loop:
FILE *fp;
char buf[]="tmp99";
for (int i=0; i<=99; i++) {
int ret;
ret=snprintf(buf, "tmp%0d", i);
assert_perror();
if (NULL!=(fp=fopen(buf, "wx"))) {
if (i==99) {
fprintf(stderr, "Failed to generate temp file\n");
exit(EXIT_FAILURE);
}
break;
}
}
BTW. It occurs to me that using fopen(buf, "wx") is only safe if the
"x" is recognized; if fopen ignores unknown options, you're SOL.
libc6 ignores unknown fopen flags, (and only processes the first 5
mode characters), and I don't when "x" was introduced..
Let's rehash.
tmpfile() is portable, always thread-safe, and, dare I say it, always
secure. Use it whenever you can; it isn't useful if you want a
temporary *name*, though.
mkdtemp creates a directory. It isn't provided by mingw32 (my new
portability guage..), but it would be simple, obvious and intuitive to
write a replacement to return the name of a directory which was just
created by us, and NULL otherwise.
mkstemp creates a file, and returns the filename. It isn't provided
by mingw32. Older libc versions didn't set the file mode in a safe
way.
mktemp, tmpnam, and tempnam all generate filenames which you might
carelessly pass to open(), which is guaranteed to be at least a little
bit unsafe if you don't use O_EXCL, but secure if you do. gcc is
obnoxious and warns about them all anyway (though some implementations
did stupidly limiting things, these functions themselves were not the
source of the problem..). There is apparently no library function I
can call to generate a filename. These all suggest tmpfile as a
replacement; while this isn't itself bad, I think it sucks that I have
to suffer through gcc warnings for functions which could otherwise be
useful.
There must also be some conclusion to the indirection mentioned in the
title of this report:
"tempnam.3, mktemp.3 says to use mkstemp.3 which says to use tmpfile.3"
I would suggest that tempnam, mktemp, and tempnam suggest either
function:
"Use tmpfile or mkstemp instead".
And I would suggest to follow Michael's advice and not discourage the
use of mkstemp. Otherwise, change the existing pages to point
directly to tmpfile.
I would also suggest that the following (presently in the Debian
.diff) is updated:
-The \fBtmpfile\fP() function generates a unique temporary filename.
-The temporary file is then opened in binary read/write (w+b) mode.
+The \fBtmpfile\fP() function opens a unique temporary file in binary
+read/write (w+b) mode.
I would suggest to replace this with "a uniquely named temporary file"
^^^^^^^^
since the file itself will be empty and other empty files exist, even
if not on the same host :)
In fact, the name itself is never exposed to the program, and is only
visible on the filesystem during the tmpfile() call; it is unlinked
immediately and the stream returned.
(This was a bug previously known as #363518; I don't know what
combination of Joey/Michael should make the change).
Justin
--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
