Re: Another day, another race - lynx 2.7.1

Theo de Raadt (deraadt@CVS.OPENBSD.ORG)
Tue, 17 Mar 1998 17:05:30 -0700

Here's a nice big list. Actually, this is getting to be pretty dull
stuff.

Now, we all know that using grep to find security problems is bogus,
since code should be audited significantly more carefully than that.
But... here we go:

% cd /home/netbsd/src
% find . -name \*.[chyl] -exec fgrep 'tempnam(\
tmpnam(\
mktemp(' {} /dev/null \;

I'm not picking on them for any particular reason. Our source tree
has basically all of these fixed (as I've mentioned before, some are
very hard to fix). It would be silly to run such a grep against our
own source tree. I could do it against 4.4lite2, but that's got far
more serious security problems.

./bin/df/df.c: mntpt = mktemp(strdup("/tmp/df.XXXXXX"));

This one is fascinating. It asks mktemp() to create a name, then it
creates a _directory_. Sounds safe, right? Well, mostly. It only
tries to create the directory once. A proper repair job will loop and
create a new name if mkdir returns EEXIST.

./gnu/usr.bin/diff/sdiff.c:#define private_tempnam() tmpnam ((char *) 0)
./gnu/usr.bin/gawk/io.c: if ((name = tempnam(".", "pip")) == NULL)
./gnu/usr.bin/rcs/lib/rcsedit.c: * and hack to use mkstemp() instead of mktemp(). This *does* cause the
./gnu/usr.bin/rcs/lib/rcsedit.c: if (!mktemp(np) || !*np)
./gnu/usr.bin/rcs/lib/rcsfnms.c: * and hack to use mkstemp() instead of mktemp(). This *does* cause the
./gnu/usr.bin/rcs/lib/rcsfnms.c: if (!mktemp(p) || !*p)
./gnu/usr.bin/rcs/lib/rcsfnms.c: if (!tmpnam(p) || !*p)
./gnu/usr.bin/rcs/lib/conf.h:#define has_mktemp 1 /* Does mktemp() work? */
./gnu/dist/gdb/remote-nindy.c:extern char *mktemp();
./gnu/dist/sim/arm/armos.c: (void)tmpnam(OSptr->tempnames[temp]) ;

The trend I have seen is this: All GNU software does /tmp handling
completely incorrectly.

./sbin/mount_portal/mount_portal.c: mktemp(un.sun_path);
./usr.bin/m4/eval.c: pbstr(mktemp(argv[2]));
./usr.bin/m4/main.c: m4temp = mktemp(xstrdup(_PATH_DIVNAME));
./usr.bin/mail/quit.c: tempname = tempnam(tmpdir, "mbox");
./usr.bin/msgs/msgs.c: mktemp(fname);
./usr.bin/rdist/main.c: mktemp(tempfile);
./usr.bin/tn3270/sys_curses/system.c:extern char *mktemp();
./usr.bin/tn3270/sys_curses/system.c: keyname = mktemp(strdup("/tmp/apiXXXXXX"));
./usr.bin/vi/common/recover.c: if ((fd = rcv_mktemp(sp, path, dp, S_IRWXU)) == -1)
./usr.bin/vi/common/recover.c: if ((fd = rcv_mktemp(sp, buf, dp, S_IRUSR | S_IWUSR)) == -1)
./usr.bin/vi/common/recover.c: if ((fd = rcv_mktemp(sp, mpath, dp, S_IRUSR | S_IWUSR)) == -1)
./usr.bin/vi/common/recover.c:rcv_mktemp(sp, path, dname, perms)
./usr.bin/window/wwterminfo.c: mktemp(wwterminfopath);
./usr.bin/xlint/xlint/xlint.c: if (mktemp(cppout) == NULL) {
./usr.bin/xlint/xlint/xlint.c: if (mktemp(ofn) == NULL) {
./usr.bin/xstr/xstr.c: strings = mktemp(strdup(_PATH_TMP));
./usr.sbin/amd/mk-amd-map/mk-amd-map.c: mktemp(maptmp);

All of the above are a concern, if I remember.

./usr.sbin/lpr/lpd/printjob.c: (void)mktemp(tempfile);

This lpd mktemp isn't a huge concern, because it is creating a
temporary file in a directory only lpd owns. One of those rare
cases where it's mostly OK.

./usr.sbin/lpr/lpr/lpr.c: tfname = lmktemp("tf", n, len);
./usr.sbin/lpr/lpr/lpr.c: cfname = lmktemp("cf", n, len);
./usr.sbin/lpr/lpr/lpr.c: dfname = lmktemp("df", n, len);
./usr.sbin/lpr/lpr/lpr.c:lmktemp(id, num, len)
./usr.sbin/ypserv/makedbm/makedbm.c: mktemp(db_tempname);
./usr.sbin/ypserv/ypxfr/ypxfr.c: mktemp(mapname);
./usr.sbin/ypserv/mkalias/mkalias.c: mktemp(db_tempname);
./usr.sbin/xntp/include/config.h:/* mktemp()? */
./usr.sbin/xntp/xntpd/ntp_config.c: (void) mktemp(res_file);
./usr.sbin/pkg_install/lib/pen.c: if (!mktemp(pen)) {
./usr.sbin/sup/source/libc.h:extern char *mktemp(char *);
./usr.sbin/sup/source/libc.h:extern char *mktemp();
./usr.sbin/sup/source/supfilesrv.c: tmpnam(rcs_file);
./usr.sbin/sup/source/supfilesrv.c: tmpnam(temp_file);
./usr.sbin/sup/source/supfilesrv.c: tmpnam(temp_file);
./sys/arch/i386/stand/installboot/getmount.c: if (mktemp(dir) == NULL) {

Of course, these are not all the /tmp races, far more exist. Many
programs will try to create their own /tmp files based on known
filenames, or... ("foo%d", pid), etc.

In general they are easy to fix. But who else is bothering?