From owner-freebsd-bugs Thu Dec 14 9:24:26 2000 From owner-freebsd-bugs@FreeBSD.ORG Thu Dec 14 09:24:23 2000 Return-Path: Delivered-To: freebsd-bugs@freebsd.org Received: from ringworld.nanolink.com (ringworld.nanolink.com [195.24.48.189]) by hub.freebsd.org (Postfix) with SMTP id DF08B37B400 for ; Thu, 14 Dec 2000 09:24:20 -0800 (PST) Received: (qmail 4299 invoked by uid 1000); 14 Dec 2000 17:23:19 -0000 Date: Thu, 14 Dec 2000 19:23:19 +0200 From: Peter Pentchev 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> Mail-Followup-To: dwmalone@FreeBSD.org, freebsd-bugs@FreeBSD.org References: <200008161826.LAA92649@freefall.freebsd.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i In-Reply-To: <200008161826.LAA92649@freefall.freebsd.org>; from dwmalone@FreeBSD.org on Wed, Aug 16, 2000 at 11:26:58AM -0700 Sender: owner-freebsd-bugs@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.org 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