Bugtraq mailing list archives
Re: mktemp() and friends
From: deraadt () cvs openbsd org (Theo de Raadt)
Date: Mon, 23 Dec 1996 18:15:24 -0700
Both do have tmpfile(3S). Why do we need so many variants of this function and provide people writing software with more chances to make bad decisions when cutting code ?
I agree, these interfaces must die. Most programmers will use whatever hammer they find first, so we must get rid of the dangerous hammers. mkstemp() and tmpfile() are the only safe interfaces that do the job. Ok, let me hereby call on all the vendors and other OS's to change the man pages for tmpnam(), tempname(), and mktemp() to indicate that these interfaces are DEPRICATED and something else should be used. As well, I would like them to add the mkstemp() interface if they do not already have it, seeing as it solves the problem. (Sometimes you don't want a FILE *). I don't CARE if these functions are ANSI standard; much like gets() their use should be warned about. In fact, I think I'm going to add linker warnings to them right now... just like there are safe ways to use gets() there are also safe ways to use these functions. But they are going to very rare. NOTE: Yes siree, we've killed every single use of gets() in the src tree. Here is what the 4.xBSD man pages say (or, rather, the OpenBSD man pages since we may have added/changed things after 4.xBSD did): char * tmpnam(char *str); char * tempnam(const char *tmpdir, const char *prefix); ... BUGS These interfaces are provided for System V and ANSI compatibility only. The mkstemp(3) interface is strongly preferred. There are four important problems with these interfaces (as well as with the historic mktemp(3) interface). First, there is an obvious race be- tween file name selection and file creation and deletion. Second, most historic implementations provide only a limited number of possible tempo- rary file names (usually 26) before file names will start being recycled. Third, the System V implementations of these functions (and of mktemp) use the access(2) function to determine whether or not the temporary file may be created. This has obvious ramifications for setuid or setgid pro- grams, complicating the portable use of these interfaces in such pro- grams. Finally, there is no specification of the permissions with which the temporary files are created. This implementation does not have these flaws, but portable software can- not depend on that. In particular, the tmpfile() interface should not be used in software expected to be used on other systems if there is any possibility that the user does not wish the temporary file to be publicly readable and writable. and... then in mktemp(3).... char * mktemp(char *template); BUGS An attacker can guess the filenames produced by mktemp(). Whenever it is possible mkstemp() should be used instead.
Has OpenBSD taken the next step and removed mktemp/tmpnam since they're no longer needed (even if just documentation for them) to discourage new use of them ?
I think the above documentation fairly strongly condemns the ANSI/SYSV interfaces ;-). Ok, in OpenBSD I can solve this even better by coaxing ld to bitch if those nasty interfaces are used. Just takes a few modifications... hang on.... How's this for a first attempt. Oh darn, the lines are longer than I want... Thanks for leading me to actually considering making this change, Darren. I only wish the other vendors would actually do something like this; until they do something proactive like this programmers on their systems will continue to use such bad practices. % cat foo.c #include <sys/param.h> #include <sys/types.h> #include <stdio.h> #include <unistd.h> #define BASE "/tmp/testXXXXXX" int main(argc, argv) int argc; char *argv[]; { char base[MAXPATHLEN]; char *s; strcpy(base, BASE); s = mktemp(base); s = tmpnam(NULL); s = tempnam(NULL, NULL); gets(base); } % cc foo.c /tmp/cc0165011.o: warning: this program uses mktemp(), which is unsafe. consider using mkstemp() /tmp/cc0165011.o: warning: this program uses tmpnam(), which is unsafe. consider using mkstemp() /tmp/cc0165011.o: warning: this program uses tempnam(), which is unsafe. consider using mkstemp() /tmp/cc0165011.o: warning: this program uses gets(), which is unsafe. % exit Note: A warning here and there is irrelevant; the code will still operate correctly.
Current thread:
- Re: mktemp() and friends Theo de Raadt (Dec 23)
- Re: mktemp() and friends Darren Reed (Dec 23)
- Re: mktemp() and friends Uriel Maimon (Dec 23)
- <Possible follow-ups>
- Re: mktemp() and friends Theo de Raadt (Dec 23)
- Re: mktemp() and friends Darren Reed (Dec 23)
- Re: mktemp() and friends Steve \ (Dec 24)
- Re: mktemp() and friends Casper Dik (Dec 24)
- Re: mktemp() and friends Theo de Raadt (Dec 23)
- Re: mktemp() and friends Benedikt Stockebrand (Dec 23)
- Re: mktemp() and friends Theo de Raadt (Dec 24)
- Re: mktemp() and friends D. J. Bernstein (Dec 24)
- Re: mktemp() and friends SGI Security Coordinator (Dec 24)
- Re: mktemp() and friends Darren Reed (Dec 23)