From owner-freebsd-bugs  Mon Aug  7  9:37:30 2000
Delivered-To: freebsd-bugs@freebsd.org
Received: from ribbit.CS.Berkeley.EDU (ribbit.CS.Berkeley.EDU [128.32.131.152])
	by hub.freebsd.org (Postfix) with ESMTP
	id 2DB8537B6F3; Mon,  7 Aug 2000 09:37:23 -0700 (PDT)
	(envelope-from jkuroda@ribbit.CS.Berkeley.EDU)
Received: from ribbit.CS.Berkeley.EDU (jkuroda@localhost)
	by ribbit.CS.Berkeley.EDU (8.9.3/8.9.3) with ESMTP id JAA08047;
	Mon, 7 Aug 2000 09:37:22 -0700 (PDT)
From: Jon Masami Kuroda <jkuroda@EECS.Berkeley.EDU>
Message-Id: <200008071637.JAA08047@ribbit.CS.Berkeley.EDU>
To: sheldonh@freebsd.org
Cc: jkuroda@cs.berkeley.edu, freebsd-bugs@freebsd.org
Subject: Re: bin/20445: restore(8) uses non-varying filenames for /tmp files when restoring given filesystem repeatedly 
In-reply-to: Your message of "Mon, 07 Aug 2000 05:34:23 PDT."
             <200008071234.FAA65992@freefall.freebsd.org> 
Date: Mon, 07 Aug 2000 09:37:22 -0700
Sender: owner-freebsd-bugs@FreeBSD.ORG
Precedence: bulk
X-Loop: FreeBSD.org

The stuff at the bottom is from the cvs log on ${SRC}/sbin/restore/dirs.c.
Any stuff from OpenBSD was added in over three years ago.  I dug up the 
NetBSD and OpenBSD source files for comparison below and have added some
morning toast commentary.

--Jon

pulled from ftp.openbsd.org:/pub/OpenBSD/src/sbin/restore/dirs.c
/*      $OpenBSD: dirs.c,v 1.17 1999/08/17 09:13:15 millert Exp $       */
...
extractdirs(genmode)
        int genmode;
{
        register int i;
        register struct dinode *ip;
        struct inotab *itp;
        struct direct nulldir;
        int fd;

        Vprintf(stdout, "Extract directories from tape\n");
        (void)snprintf(dirfile, sizeof(dirfile), "%s/rstdir%d", tmpdir,
            dumpdate);
        if (command != 'r' && command != 'R') {
                strncat(dirfile, "-XXXXXXXXXX",
                    sizeof(dirfile) - strlen(dirfile));
                fd = mkstemp(dirfile);
        } else
                fd = open(dirfile, O_RDWR|O_CREAT|O_EXCL, 0666);
...

OpenBSD does the same thing in this respect at FreeBSD and NetBSD does
something similar with respect to mkstemp(3) and open(2):

pulled from ftp.netbsd.org:/pub/NetBSD/NetBSD-current/src/sbin/restore/dirs.c

/*      $NetBSD: dirs.c,v 1.33 1998/05/12 00:42:48 enami Exp $  */
...
        vprintf(stdout, "Extract directories from tape\n");
        (void) snprintf(dirfile, sizeof(dirfile), "%s/rstdir%d",
            tmpdir, (int)dumpdate);
        if (command != 'r' && command != 'R') {
                (void) snprintf(dirfile, sizeof(dirfile), "%s/rstdir%d-XXXXXX",
                    tmpdir, (int)dumpdate);
                if ((dfd = mkstemp(dirfile)) == -1)
                        err(1, "cannot mkstemp temporary file %s", dirfile);
                df = fdopen(dfd, "w");
        }
        else
                df = fopen(dirfile, "w");
...

It seems mkstemp(3) was applied only where "nasty race conditions"
would arise.  The only major difference I can see is Net/OpenBSD's use
of snprintf(3) and strncat(3) with sizeof(dirfile) in these functions
(FreeBSD uses these and similar library calls elsewhere in restore but
not here).  The rest could arguably be called coding style differences.

This "bug" should not have any security impact if I understand open(2)
and the "O_EXCL" flag correctly.  It is, so far, just a productivity
annoyance.

revision 1.7
date: 1997/01/01 00:03:45;  author: imp;  state: Exp;  lines: +35 -15
Various security related deltas from OpenBSD

dirs.c:
        From OpenBSD 1.2, 1.3, 1.5, 1.8, 1.10, 1.11, 1.12
        1.2:
                use unique temporary files; netbsd pr#2544;
                lukem@supp.cpr.itg.telecom.com.au
        1.3:
                updated patch from lukem@supp.cpr.itg.telecom.com.au
                to also make -r and -R work again
        1.5:
                mktemp open & fdopen
        1.8:
                /tmp// -> /tmp/
        1.10:
                Fix strncpy usage and correct strncat length field,
                from Theo.  Also change some occurrence of MAXPATHLEN
                with sizeof(foo).
        1.11:
                does noone know how to use strncat correctly?
        1.12:
                use mkstemp()
        From NetBSD:
                Use open rather than create so we can specify
                exclusive open mode.

main.c:
        From OpenBSD 1.2, 1.5
        1.2:
                From NetBSD: support $TAPE.
        1.5
                Set umask to be read only by owner until we set real
                file permissions.
tape.c:
        From NetBSD:
                Use open rather than create so we can specify
                exclusive open mode.


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-bugs" in the body of the message