From owner-freebsd-current Thu Dec 9 6:34:53 1999 Delivered-To: freebsd-current@freebsd.org Received: from thoth.mch.sni.de (thoth.mch.sni.de [192.35.17.2]) by hub.freebsd.org (Postfix) with ESMTP id 1ADAD15617 for ; Thu, 9 Dec 1999 06:33:48 -0800 (PST) (envelope-from andre.albsmeier@mchp.siemens.de) X-Envelope-Sender-Is: andre.albsmeier@mchp.siemens.de (at relayer thoth.mch.sni.de) Received: from mail2.siemens.de (mail2.siemens.de [139.25.208.11]) by thoth.mch.sni.de (8.9.3/8.9.3) with ESMTP id PAA09793 for ; Thu, 9 Dec 1999 15:33:21 +0100 (MET) Received: from curry.mchp.siemens.de (curry.mchp.siemens.de [139.25.42.7]) by mail2.siemens.de (8.9.3/8.9.3) with ESMTP id PAA02715 for ; Thu, 9 Dec 1999 15:33:21 +0100 (MET) Received: (from daemon@localhost) by curry.mchp.siemens.de (8.9.3/8.9.3) id PAA98457 for ; Thu, 9 Dec 1999 15:33:21 +0100 (CET) Date: Thu, 9 Dec 1999 15:33:20 +0100 From: Andre Albsmeier To: Alfred Perlstein Cc: Warner Losh , Garance A Drosihn , current@FreeBSD.ORG, stable@FreeBSD.ORG Subject: Re: NO! Re: [PATCHES] Two fixes for lpd/lpc for review and test Message-ID: <19991209153320.A62121@internal> References: <199912072106.OAA44391@harmony.village.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Mailer: Mutt 1.0i In-Reply-To: ; from bright@wintelcom.net on Tue, Dec 07, 1999 at 02:55:37PM -0800 Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG On Tue, 07-Dec-1999 at 14:55:37 -0800, Alfred Perlstein wrote: I think the (really big) security hole can be closed by not doing the chown/chmod commands. I inserted them because I wanted the file in the spool directory to appear exactly as if lpr would have copied it. I am currently running the patch with the chown/chmod removed and lpd doesn't seem to have any problems with it. The side effect now is that the file in the spool directory keeps it's permissions. I don't think that this is a problem because if the file was set to 666 by the creator before, he doesn't care a lot about it anyway :-) What do people think about this? Alfred, Warner ? For better reference, here is the current patch: *** lpr.c.ORI Thu Dec 9 15:30:18 1999 --- lpr.c Thu Dec 9 15:30:35 1999 *************** *** 370,375 **** --- 370,405 ---- } if (sflag) printf("%s: %s: not linked, copying instead\n", name, arg); + /* + * If lpr was invoked with -r we try to move the file to + * be printed instead of copying and deleting it later. + * This works if the file and lpd's spool directory are + * on the same filesystem as it is often the case for files + * printed by samba or pcnfsd. In this case, a lot of I/O + * and temporary disk space can be avoided. Otherwise, we + * will continue normally. + */ + if (f) { /* file should be deleted */ + seteuid(euid); /* needed for rename() */ + if (!rename(arg, dfname)) { + int i; + #if 0 + chown(dfname, userid, getegid()); + chmod(dfname, S_IRUSR | S_IWUSR | + S_IRGRP | S_IWGRP); + #endif + seteuid(uid); /* restore old uid */ + if (format == 'p') + card('T', title ? title : arg); + for (i = 0; i < ncopies; i++) + card(format, &dfname[inchar-2]); + card('U', &dfname[inchar-2]); + card('N', arg); + nact++; + continue; + } + seteuid(uid); /* restore old uid */ + } if ((i = open(arg, O_RDONLY)) < 0) { printf("%s: cannot open %s\n", name, arg); } else { > On Tue, 7 Dec 1999, Warner Losh wrote: > > > I've been reviewing this patch with someone and I think the last > > version is ready to commit. I'll take a look at my tree to make > > sure. > > please do not, the patch in PR 11997 introduces a major security flaw. > > someone can hardlink to any file and clobber it with a file owned by > them: > > try this: > > as root: > > # cd /var/tmp ; touch rootfile ; chown root:wheel rootfile ; chmod 600 rootfile > > as a user: > > % cd /var/tmp ; echo foo > foo > % lpr -r foo > sleeping > > in another session as user: > > % rm foo ; ln rootfile foo > > wait a second... > > # ls -l rootfile > -rw-rw---- 3 user daemon 5 Dec 7 13:38 rootfile > # cat rootfile > foo > # > > ouch! > > -Alfred > > use this patch to make the race condition apparrent: > > > Index: usr.sbin/lpr/lpr/lpr.c > =================================================================== > RCS file: /home/ncvs/src/usr.sbin/lpr/lpr/lpr.c,v > retrieving revision 1.27.2.2 > diff -u -u -r1.27.2.2 lpr.c > --- lpr.c 1999/08/29 15:43:29 1.27.2.2 > +++ lpr.c 1999/12/08 01:47:47 > @@ -370,6 +370,27 @@ > } > if (sflag) > printf("%s: %s: not linked, copying instead\n", name, arg); > + if( f ) { /* means that the file should be deleted */ > + printf("sleeping\n"); > + sleep(5); > + printf("done.\n"); > + seteuid(euid); /* needed for rename() to succeed */ > + if( ! rename( arg, dfname ) ) { > + register int i; > + chmod( dfname, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP ); > + chown( dfname, userid, getgrnam("daemon")->gr_gid ); > + seteuid(uid); > + if (format == 'p') > + card('T', title ? title : arg); > + for (i = 0; i < ncopies; i++) > + card(format, &dfname[inchar-2]); > + card('U', &dfname[inchar-2]); > + card('N', arg); > + nact++; > + continue; > + } > + seteuid(uid); > + } > if ((i = open(arg, O_RDONLY)) < 0) { > printf("%s: cannot open %s\n", name, arg); > } else { > > > > > > To Unsubscribe: send mail to majordomo@FreeBSD.org > with "unsubscribe freebsd-stable" in the body of the message -- "In a world without walls and fences, who needs windows and gates?" To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message