From owner-freebsd-bugs Wed Aug 16 8:30:12 2000 Delivered-To: freebsd-bugs@freebsd.org Received: from freefall.freebsd.org (freefall.FreeBSD.ORG [204.216.27.21]) by hub.freebsd.org (Postfix) with ESMTP id F0BE237BF06 for ; Wed, 16 Aug 2000 08:30:01 -0700 (PDT) (envelope-from gnats@FreeBSD.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.9.3/8.9.2) id IAA63368; Wed, 16 Aug 2000 08:30:00 -0700 (PDT) (envelope-from gnats@FreeBSD.org) Received: from sentinel.office1.bg (sentinel.office1.bg [195.24.48.182]) by hub.freebsd.org (Postfix) with SMTP id 6848A37BFE6 for ; Wed, 16 Aug 2000 08:29:06 -0700 (PDT) (envelope-from roam@orbitel.bg) Received: (qmail 2654 invoked by uid 1001); 16 Aug 2000 15:21:08 -0000 Message-Id: <20000816152108.2653.qmail@ringwraith.office1> Date: 16 Aug 2000 15:21:08 -0000 From: Peter Pentchev Reply-To: Peter Pentchev To: FreeBSD-gnats-submit@freebsd.org X-Send-Pr-Version: 3.2 Subject: bin/20646: [PATCH] /bin/cp -p whines on set[ug]id immutable files Sender: owner-freebsd-bugs@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.org >Number: 20646 >Category: bin >Synopsis: [PATCH] /bin/cp -p whines on set[ug]id immutable files >Confidential: no >Severity: serious >Priority: medium >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Wed Aug 16 08:30:00 PDT 2000 >Closed-Date: >Last-Modified: >Originator: Peter Pentchev >Release: FreeBSD 4.1-STABLE i386 >Organization: Orbitel JSCo >Environment: RELENG_4 as of today >Description: cp -p preserves, among others, the file flags. Unfortunately, this clashes with its attempt just a little bit later to retain a file's setuid/setgid/sticky bits if the user doing the copying is the file owner. (There was a similar PR about /sbin/restore's handling of utimes a while back.) >How-To-Repeat: [root@ringwraith /usr/home/roam]# ls -lo /usr/bin/passwd -r-sr-xr-x 2 root wheel schg 26260 Aug 16 16:13 /usr/bin/passwd [root@ringwraith /usr/home/roam]# cp -p /usr/bin/passwd . cp: ./passwd: Operation not permitted [root@ringwraith /usr/home/roam]# ls -lo passwd -r-sr-xr-x 1 root wheel schg 26260 Aug 16 16:13 passwd [root@ringwraith /usr/home/roam]# >Fix: Alright, so this might not be the best solution; arguably the best way is to move the whole setuid/setgid/sticky bits fixup into setfile() itself, where it belongs. But this works for me ;) diff -u -urN src/bin/cp/cp.c mysrc/bin/cp/cp.c --- src/bin/cp/cp.c Sun Nov 28 11:34:21 1999 +++ mysrc/bin/cp/cp.c Wed Aug 16 18:03:29 2000 @@ -388,7 +388,7 @@ * umask; arguably wrong, but it's been that way * forever. */ - if (pflag && setfile(curr->fts_statp, 0)) + if (pflag && setfile(curr->fts_statp, 0, 1)) badcp = rval = 1; else if (dne) (void)chmod(to.p_path, diff -u -urN src/bin/cp/extern.h mysrc/bin/cp/extern.h --- src/bin/cp/extern.h Wed Aug 16 18:15:40 2000 +++ mysrc/bin/cp/extern.h Wed Aug 16 17:46:20 2000 @@ -51,6 +51,6 @@ int copy_file __P((FTSENT *, int)); int copy_link __P((FTSENT *, int)); int copy_special __P((struct stat *, int)); -int setfile __P((struct stat *, int)); +int setfile __P((struct stat *, int, int)); void usage __P((void)); __END_DECLS diff -u -urN src/bin/cp/utils.c mysrc/bin/cp/utils.c --- src/bin/cp/utils.c Wed Aug 16 18:15:40 2000 +++ mysrc/bin/cp/utils.c Wed Aug 16 18:16:37 2000 @@ -176,7 +176,8 @@ * to remove it if we created it and its length is 0. */ - if (pflag && setfile(fs, to_fd)) + /* do not copy flags at this time, or the next (f)chmod() fails */ + if (pflag && setfile(fs, to_fd, 0)) rval = 1; /* * If the source was setuid or setgid, lose the bits unless the @@ -194,6 +195,11 @@ rval = 1; } } + + /* setfile() again, just for the file flags */ + if (pflag && setfile(fs, to_fd, 2)) + rval = 1; + (void)close(from_fd); if (close(to_fd)) { warn("%s", to.p_path); @@ -239,7 +245,7 @@ warn("mkfifo: %s", to.p_path); return (1); } - return (pflag ? setfile(from_stat, 0) : 0); + return (pflag ? setfile(from_stat, 0, 1) : 0); } int @@ -255,14 +261,23 @@ warn("mknod: %s", to.p_path); return (1); } - return (pflag ? setfile(from_stat, 0) : 0); + return (pflag ? setfile(from_stat, 0, 1) : 0); } int -setfile(fs, fd) +setfile(fs, fd, setflags) register struct stat *fs; - int fd; + int fd, setflags; + /* values for setflags: + 0 - do not touch fd's flags; + 1 - set fd's flags to match fs's flags; + 2 - ONLY set fd's flags to fs's flags, do nothing more + + the only reason for setflags to be 2 is in copy_file() + after the set[ug]id fixup; this would be better if + the fixup itself were moved here. + */ { static struct timeval tv[2]; struct stat ts; @@ -292,28 +307,34 @@ * the mode; current BSD behavior is to remove all setuid bits on * chown. If chown fails, lose setuid/setgid bits. */ - if (!gotstat || fs->st_uid != ts.st_uid || fs->st_gid != ts.st_gid) - if (fd ? fchown(fd, fs->st_uid, fs->st_gid) : - chown(to.p_path, fs->st_uid, fs->st_gid)) { - if (errno != EPERM) { + + /* Oops. Are we called *after* the copy_file() set[ug]id fixup? */ + if (setflags != 2) { + if (!gotstat || fs->st_uid != ts.st_uid || fs->st_gid != ts.st_gid) + if (fd ? fchown(fd, fs->st_uid, fs->st_gid) : + chown(to.p_path, fs->st_uid, fs->st_gid)) { + if (errno != EPERM) { + warn("chown: %s", to.p_path); + rval = 1; + } + fs->st_mode &= ~(S_ISUID | S_ISGID); + } + + if (!gotstat || fs->st_mode != ts.st_mode) + if (fd ? fchmod(fd, fs->st_mode) : chmod(to.p_path, fs->st_mode)) { warn("chown: %s", to.p_path); rval = 1; } - fs->st_mode &= ~(S_ISUID | S_ISGID); - } - - if (!gotstat || fs->st_mode != ts.st_mode) - if (fd ? fchmod(fd, fs->st_mode) : chmod(to.p_path, fs->st_mode)) { - warn("chown: %s", to.p_path); - rval = 1; - } + } - if (!gotstat || fs->st_flags != ts.st_flags) - if (fd ? - fchflags(fd, fs->st_flags) : chflags(to.p_path, fs->st_flags)) { - warn("chflags: %s", to.p_path); - rval = 1; - } + if (setflags) { + if (!gotstat || fs->st_flags != ts.st_flags) + if (fd ? + fchflags(fd, fs->st_flags) : chflags(to.p_path, fs->st_flags)) { + warn("chflags: %s", to.p_path); + rval = 1; + } + } return (rval); } >Release-Note: >Audit-Trail: >Unformatted: To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-bugs" in the body of the message