Date: Thu, 9 Dec 1999 15:02:41 -0800 (PST) From: Alfred Perlstein <bright@wintelcom.net> To: Andre Albsmeier <andre.albsmeier@mchp.siemens.de> Cc: Warner Losh <imp@village.org>, Garance A Drosihn <drosih@rpi.edu>, stable@FreeBSD.ORG Subject: Re: NO! Re: [PATCHES] Two fixes for lpd/lpc for review and test Message-ID: <Pine.BSF.4.21.9912091427240.4557-100000@fw.wintelcom.net> In-Reply-To: <19991209153320.A62121@internal>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 9 Dec 1999, Andre Albsmeier wrote:
> > 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.
> >
> On Tue, 07-Dec-1999 at 14:55:37 -0800, Alfred Perlstein wrote:
> > 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:
> >
>
> 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 {
>
>
I don't have too much time to think about this, argue me this:
why should I allow a user to print any file on the system?
the race condition is still there.
-Alfred
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-stable" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.4.21.9912091427240.4557-100000>