Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 14 Dec 2000 19:23:19 +0200
From:      Peter Pentchev <roam@orbitel.bg>
To:        dwmalone@FreeBSD.org
Cc:        freebsd-bugs@FreeBSD.org
Subject:   Re: bin/20646: [PATCH] /bin/cp -p whines on set[ug]id immutable files
Message-ID:  <20001214192319.K369@ringworld.oblivion.bg>
In-Reply-To: <200008161826.LAA92649@freefall.freebsd.org>; from dwmalone@FreeBSD.org on Wed, Aug 16, 2000 at 11:26:58AM -0700
References:  <200008161826.LAA92649@freefall.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Aug 16, 2000 at 11:26:58AM -0700, dwmalone@FreeBSD.org wrote:
> Synopsis: [PATCH] /bin/cp -p whines on set[ug]id immutable files
> 
> Responsible-Changed-From-To: freebsd-bugs->dwmalone
> Responsible-Changed-By: dwmalone
> Responsible-Changed-When: Wed Aug 16 11:26:13 PDT 2000
> Responsible-Changed-Why: 
> I looked at a similar PR for restore - I'll try to have a look at this one.

OK, so the patch in the PR itself was not a good one at all; I submitted
a better one in a followup to Sheldon and -bugs at the time, but it seems
it did not enter the audit trail.  So here it is now :)

G'luck,
Peter

-- 
because I didn't think of a good beginning of it.

Index: src/bin/cp/utils.c
===================================================================
RCS file: /home/ncvs/src/bin/cp/utils.c,v
retrieving revision 1.28
diff -u -r1.28 utils.c
--- src/bin/cp/utils.c	2000/10/10 01:48:18	1.28
+++ src/bin/cp/utils.c	2000/12/14 16:33:09
@@ -179,22 +179,11 @@
 
 	if (pflag && setfile(fs, to_fd))
 		rval = 1;
-	/*
-	 * If the source was setuid or setgid, lose the bits unless the
-	 * copy is owned by the same user and group.
-	 */
-#define	RETAINBITS \
-	(S_ISUID | S_ISGID | S_ISVTX | S_IRWXU | S_IRWXG | S_IRWXO)
-	else if (fs->st_mode & (S_ISUID | S_ISGID) && fs->st_uid == myuid) {
-		if (fstat(to_fd, &to_stat)) {
-			warn("%s", to.p_path);
-			rval = 1;
-		} else if (fs->st_gid == to_stat.st_gid &&
-		    fchmod(to_fd, fs->st_mode & RETAINBITS & ~myumask)) {
-			warn("%s", to.p_path);
-			rval = 1;
-		}
-	}
+
+	/* We no longer need to reinstate the setuid/setgid bits - see
+	   the 'chown failed' part of setfile() - they are no longer reset,
+	   and we couldn't reinstate them on immutable files anyway. */
+
 	(void)close(from_fd);
 	if (close(to_fd)) {
 		warn("%s", to.p_path);
@@ -300,7 +289,20 @@
 				warn("chown: %s", to.p_path);
 				rval = 1;
 			}
-			fs->st_mode &= ~(S_ISUID | S_ISGID);
+
+			/* the chown failed, lose only the privileges
+			   we have no right to have */
+
+			if ((fs->st_mode & S_ISUID) && (fs->st_uid != myuid))
+				fs->st_mode &= ~S_ISUID;
+			if ((fs->st_mode & S_ISGID) && (fs->st_gid != ts.st_gid))
+				fs->st_mode &= ~S_ISGID;
+
+			/* I'm not quite sure what to do about the sticky bit -
+			   for a file it doesn't matter, for a dir - I think
+			   it's a good thing to have, just in case the world
+			   decides to storm into our little newborn dir ;)
+			   Think I'll just leave it alone. */
 		}
 
 	if (!gotstat || fs->st_mode != ts.st_mode)


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




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20001214192319.K369>