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>