Date: 16 Aug 2000 15:21:08 -0000 From: Peter Pentchev <roam@orbitel.bg> To: FreeBSD-gnats-submit@freebsd.org Subject: bin/20646: [PATCH] /bin/cp -p whines on set[ug]id immutable files Message-ID: <20000816152108.2653.qmail@ringwraith.office1>
next in thread | raw e-mail | index | archive | help
>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 <roam@orbitel.bg>
>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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20000816152108.2653.qmail>