From dada@pluto.tugraz.at Wed Apr 4 13:17:09 2007 Return-Path: Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id CC55E16A409 for ; Wed, 4 Apr 2007 13:17:09 +0000 (UTC) (envelope-from dada@pluto.tugraz.at) Received: from mailrelay.tugraz.at (mailrelay.tu-graz.ac.at [129.27.2.202]) by mx1.freebsd.org (Postfix) with ESMTP id 54C6513C48C for ; Wed, 4 Apr 2007 13:17:08 +0000 (UTC) (envelope-from dada@pluto.tugraz.at) Received: from pluto.tugraz.at (pluto.tu-graz.ac.at [129.27.3.200]) by mailrelay2.tugraz.at (8.14.0/8.14.0) with ESMTP id l34DH3VU005232 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Wed, 4 Apr 2007 15:17:03 +0200 (CEST) Received: from pluto.tugraz.at (localhost.localdomain [127.0.0.1]) by pluto.tugraz.at (8.13.1/8.13.1) with ESMTP id l34DGvow008652 for ; Wed, 4 Apr 2007 15:16:58 +0200 Received: (from dada@localhost) by pluto.tugraz.at (8.13.1/8.13.1/Submit) id l34DGvWs008651 for freebsd-gnats-submit@freebsd.org; Wed, 4 Apr 2007 15:16:57 +0200 Message-Id: <200704041316.l34DGvWs008651@pluto.tugraz.at> Date: Wed, 4 Apr 2007 15:16:57 +0200 From: Martin Kammerhofer To: freebsd-gnats-submit@freebsd.org Subject: Incorrect usage of chflags(2) in FreeBSD utility programs >Number: 111226 >Category: bin >Synopsis: [patch] Incorrect usage of chflags() in various FreeBSD utility programs >Confidential: no >Severity: non-critical >Priority: medium >Responsible: freebsd-bugs >State: closed >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Wed Apr 04 13:20:12 GMT 2007 >Closed-Date: Sun Jan 17 21:03:08 UTC 2010 >Last-Modified: Sun Jan 17 21:03:08 UTC 2010 >Originator: Martin Kammerhofer >Release: FreeBSD 6.2-STABLE i386 >Organization: >Environment: System: FreeBSD Martin.liebt.Susi 6.2-STABLE FreeBSD 6.2-STABLE #0: Wed Feb 21 09:13:55 CET 2007 toor@Martin.liebt.Susi:/usr/obj/usr/src/sys/P2B-S i386 >Description: Quote from symlink(7) manpage: Because a symbolic link and its referenced object coexist in the file system name space, confusion can arise in distinguishing between the link itself and the referenced object. This PR is only about a few cases where chflags(2) is incorrectly used rather than the correct lchflags(2) system call. Those bugs lead to the following (POLA violating) behaviour: (1) /bin/rm operating as super user cannot remove a symbolic link which has UF_APPEND or UF_IMMUTABLE set. (2) /bin/rm when running as super user and failing to unlink a UF_APPEND|UF_IMMUTABLE protected symbolic link will reset the UF_APPEND and UF_IMMUTABLE flags on the symbolic link's target (if that target exists) - an object that /bin/rm should not touch! (Quote from SUSv3: ``The rm utility removes symbolic links themselves, not the files they refer to, as a consequence of the dependence on the unlink() functionality''). (3) /usr/bin/find ... -delete behaves like /bin/rm. (4) /usr/sbin/pkg_add has a similar issue when creating backup files. (5) Finally when /bin/cp -Rp copies a symbolic link it does not preserve the file flags but instead incorrectly reports ENOSYS. >How-To-Repeat: martin@Martin:~$ su - Password: ********** root@Martin:~# touch myfile root@Martin:~# ln -s myfile myslink root@Martin:~# chflags -h uchg myfile myslink root@Martin:~# ls -lo myfile myslink -rw-r--r-- 1 root wheel uchg 0 3 Apr 14:18 myfile lrwxr-xr-x 1 root wheel uchg 6 3 Apr 14:19 myslink -> myfile root@Martin:~# rm -f myslink # fails and modifies myfile instead (1)+(2) rm: myslink: Operation not permitted root@Martin:~# ls -lo myfile myslink -rw-r--r-- 1 root wheel - 0 3 Apr 14:18 myfile lrwxr-xr-x 1 root wheel uchg 6 3 Apr 14:19 myslink -> myfile root@Martin:~# cp -Rp myslink myslink2 # fails to copy uchg flag (5) cp: chflags: myslink2: Function not implemented root@Martin:~# ls -lo myslink* lrwxr-xr-x 1 root wheel uchg 6 3 Apr 14:19 myslink -> myfile lrwxr-xr-x 1 root wheel - 6 3 Apr 14:19 myslink2 -> myfile >Fix: Index: bin/cp/utils.c =================================================================== --- bin/cp/utils.c.orig 2006-10-07 14:14:50.000000000 +0200 +++ bin/cp/utils.c 2007-03-31 14:28:47.000000000 +0200 @@ -339,7 +339,7 @@ if (!gotstat || fs->st_flags != ts.st_flags) if (fdval ? fchflags(fd, fs->st_flags) : - (islink ? (errno = ENOSYS) : + (islink ? lchflags(to.p_path, fs->st_flags) : chflags(to.p_path, fs->st_flags))) { warn("chflags: %s", to.p_path); rval = 1; Index: bin/rm/rm.c =================================================================== --- bin/rm/rm.c.orig 2006-10-31 03:22:36.000000000 +0100 +++ bin/rm/rm.c 2007-03-31 14:28:47.000000000 +0200 @@ -231,7 +231,7 @@ else if (!uid && (p->fts_statp->st_flags & (UF_APPEND|UF_IMMUTABLE)) && !(p->fts_statp->st_flags & (SF_APPEND|SF_IMMUTABLE)) && - chflags(p->fts_accpath, + lchflags(p->fts_accpath, p->fts_statp->st_flags &= ~(UF_APPEND|UF_IMMUTABLE)) < 0) goto err; continue; @@ -250,7 +250,7 @@ if (!uid && (p->fts_statp->st_flags & (UF_APPEND|UF_IMMUTABLE)) && !(p->fts_statp->st_flags & (SF_APPEND|SF_IMMUTABLE))) - rval = chflags(p->fts_accpath, + rval = lchflags(p->fts_accpath, p->fts_statp->st_flags &= ~(UF_APPEND|UF_IMMUTABLE)); if (rval == 0) { /* @@ -350,7 +350,7 @@ if (!uid && !S_ISWHT(sb.st_mode) && (sb.st_flags & (UF_APPEND|UF_IMMUTABLE)) && !(sb.st_flags & (SF_APPEND|SF_IMMUTABLE))) - rval = chflags(f, sb.st_flags & ~(UF_APPEND|UF_IMMUTABLE)); + rval = lchflags(f, sb.st_flags & ~(UF_APPEND|UF_IMMUTABLE)); if (rval == 0) { if (S_ISWHT(sb.st_mode)) rval = undelete(f); Index: usr.bin/find/function.c =================================================================== --- usr.bin/find/function.c.orig 2006-05-27 20:27:41.000000000 +0200 +++ usr.bin/find/function.c 2007-03-31 14:28:47.000000000 +0200 @@ -441,7 +441,7 @@ if ((entry->fts_statp->st_flags & (UF_APPEND|UF_IMMUTABLE)) && !(entry->fts_statp->st_flags & (SF_APPEND|SF_IMMUTABLE)) && geteuid() == 0) - chflags(entry->fts_accpath, + lchflags(entry->fts_accpath, entry->fts_statp->st_flags &= ~(UF_APPEND|UF_IMMUTABLE)); /* rmdir directories, unlink everything else */ Index: usr.sbin/pkg_install/add/extract.c =================================================================== --- usr.sbin/pkg_install/add/extract.c.orig 2006-01-07 23:10:57.000000000 +0100 +++ usr.sbin/pkg_install/add/extract.c 2007-03-31 14:28:47.000000000 +0200 @@ -63,8 +63,7 @@ if (q->type == PLIST_FILE) { snprintf(try, FILENAME_MAX, "%s/%s", dir, q->name); if (make_preserve_name(bup, FILENAME_MAX, name, try) && fexists(bup)) { - (void)chflags(try, 0); - (void)unlink(try); + (void)lchflags(try, 0); if (rename(bup, try)) warnx("rollback: unable to rename %s back to %s", bup, try); } @@ -160,7 +159,7 @@ /* first try to rename it into place */ snprintf(try, FILENAME_MAX, "%s/%s", Directory, p->name); if (fexists(try)) { - (void)chflags(try, 0); /* XXX hack - if truly immutable, rename fails */ + (void)lchflags(try, 0); /* XXX hack - if truly immutable, rename fails */ if (preserve && PkgName) { char pf[FILENAME_MAX]; >Release-Note: >Audit-Trail: From: dfilter@FreeBSD.ORG (dfilter service) To: bug-followup@FreeBSD.org Cc: Subject: Re: bin/111226: commit references a PR Date: Sat, 30 May 2009 10:36:29 +0000 (UTC) Author: jilles Date: Sat May 30 10:36:14 2009 New Revision: 193086 URL: http://svn.freebsd.org/changeset/base/193086 Log: Preserve file flags on symlinks in cp -Rp. This reported ENOSYS before. PR: bin/111226 (part of) Submitted by: Martin Kammerhofer Approved by: ed (mentor) MFC after: 3 weeks Modified: head/bin/cp/utils.c Modified: head/bin/cp/utils.c ============================================================================== --- head/bin/cp/utils.c Sat May 30 08:53:13 2009 (r193085) +++ head/bin/cp/utils.c Sat May 30 10:36:14 2009 (r193086) @@ -365,7 +365,7 @@ setfile(struct stat *fs, int fd) if (!gotstat || fs->st_flags != ts.st_flags) if (fdval ? fchflags(fd, fs->st_flags) : - (islink ? (errno = ENOSYS) : + (islink ? lchflags(to.p_path, fs->st_flags) : chflags(to.p_path, fs->st_flags))) { warn("chflags: %s", to.p_path); rval = 1; _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org" From: dfilter@FreeBSD.ORG (dfilter service) To: bug-followup@FreeBSD.org Cc: Subject: Re: bin/111226: commit references a PR Date: Sat, 30 May 2009 10:42:31 +0000 (UTC) Author: jilles Date: Sat May 30 10:42:19 2009 New Revision: 193087 URL: http://svn.freebsd.org/changeset/base/193087 Log: rm, find -delete: fix removing symlinks with uchg/uappnd set. Formerly, this tried to clear the flags on the symlink's target instead of the symlink itself. As before, this only happens for root or for the unlink(1) variant of rm. PR: bin/111226 (part of) Submitted by: Martin Kammerhofer Approved by: ed (mentor) MFC after: 3 weeks Modified: head/bin/rm/rm.c head/usr.bin/find/function.c Modified: head/bin/rm/rm.c ============================================================================== --- head/bin/rm/rm.c Sat May 30 10:36:14 2009 (r193086) +++ head/bin/rm/rm.c Sat May 30 10:42:19 2009 (r193087) @@ -234,7 +234,7 @@ rm_tree(char **argv) else if (!uid && (p->fts_statp->st_flags & (UF_APPEND|UF_IMMUTABLE)) && !(p->fts_statp->st_flags & (SF_APPEND|SF_IMMUTABLE)) && - chflags(p->fts_accpath, + lchflags(p->fts_accpath, p->fts_statp->st_flags &= ~(UF_APPEND|UF_IMMUTABLE)) < 0) goto err; continue; @@ -253,7 +253,7 @@ rm_tree(char **argv) if (!uid && (p->fts_statp->st_flags & (UF_APPEND|UF_IMMUTABLE)) && !(p->fts_statp->st_flags & (SF_APPEND|SF_IMMUTABLE))) - rval = chflags(p->fts_accpath, + rval = lchflags(p->fts_accpath, p->fts_statp->st_flags &= ~(UF_APPEND|UF_IMMUTABLE)); if (rval == 0) { /* @@ -368,7 +368,7 @@ rm_file(char **argv) if (!uid && !S_ISWHT(sb.st_mode) && (sb.st_flags & (UF_APPEND|UF_IMMUTABLE)) && !(sb.st_flags & (SF_APPEND|SF_IMMUTABLE))) - rval = chflags(f, sb.st_flags & ~(UF_APPEND|UF_IMMUTABLE)); + rval = lchflags(f, sb.st_flags & ~(UF_APPEND|UF_IMMUTABLE)); if (rval == 0) { if (S_ISWHT(sb.st_mode)) rval = undelete(f); Modified: head/usr.bin/find/function.c ============================================================================== --- head/usr.bin/find/function.c Sat May 30 10:36:14 2009 (r193086) +++ head/usr.bin/find/function.c Sat May 30 10:42:19 2009 (r193087) @@ -443,7 +443,7 @@ f_delete(PLAN *plan __unused, FTSENT *en if ((entry->fts_statp->st_flags & (UF_APPEND|UF_IMMUTABLE)) && !(entry->fts_statp->st_flags & (SF_APPEND|SF_IMMUTABLE)) && geteuid() == 0) - chflags(entry->fts_accpath, + lchflags(entry->fts_accpath, entry->fts_statp->st_flags &= ~(UF_APPEND|UF_IMMUTABLE)); /* rmdir directories, unlink everything else */ _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org" From: dfilter@FreeBSD.ORG (dfilter service) To: bug-followup@FreeBSD.org Cc: Subject: Re: bin/111226: commit references a PR Date: Sun, 21 Jun 2009 15:36:21 +0000 (UTC) Author: jilles Date: Sun Jun 21 15:36:10 2009 New Revision: 194587 URL: http://svn.freebsd.org/changeset/base/194587 Log: MFC r193086: Preserve file flags on symlinks in cp -Rp. This reported ENOSYS before. PR: bin/111226 (part of) Submitted by: Martin Kammerhofer Approved by: ed (mentor) (implicit) Modified: stable/7/bin/cp/ (props changed) stable/7/bin/cp/utils.c Modified: stable/7/bin/cp/utils.c ============================================================================== --- stable/7/bin/cp/utils.c Sun Jun 21 13:41:32 2009 (r194586) +++ stable/7/bin/cp/utils.c Sun Jun 21 15:36:10 2009 (r194587) @@ -339,7 +339,7 @@ setfile(struct stat *fs, int fd) if (!gotstat || fs->st_flags != ts.st_flags) if (fdval ? fchflags(fd, fs->st_flags) : - (islink ? (errno = ENOSYS) : + (islink ? lchflags(to.p_path, fs->st_flags) : chflags(to.p_path, fs->st_flags))) { warn("chflags: %s", to.p_path); rval = 1; _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org" From: dfilter@FreeBSD.ORG (dfilter service) To: bug-followup@FreeBSD.org Cc: Subject: Re: bin/111226: commit references a PR Date: Sun, 21 Jun 2009 15:40:54 +0000 (UTC) Author: jilles Date: Sun Jun 21 15:40:39 2009 New Revision: 194588 URL: http://svn.freebsd.org/changeset/base/194588 Log: MFC r193087: rm, find -delete: fix removing symlinks with uchg/uappnd set Formerly, this tried to clear the flags on the symlink's target instead of the symlink itself. As before, this only happens for root or for the unlink(1) variant of rm. PR: bin/111226 (part of) Submitted by: Martin Kammerhofer Approved by: ed (mentor) (implicit) Modified: stable/7/bin/rm/ (props changed) stable/7/bin/rm/rm.c stable/7/usr.bin/find/ (props changed) stable/7/usr.bin/find/function.c Modified: stable/7/bin/rm/rm.c ============================================================================== --- stable/7/bin/rm/rm.c Sun Jun 21 15:36:10 2009 (r194587) +++ stable/7/bin/rm/rm.c Sun Jun 21 15:40:39 2009 (r194588) @@ -231,7 +231,7 @@ rm_tree(char **argv) else if (!uid && (p->fts_statp->st_flags & (UF_APPEND|UF_IMMUTABLE)) && !(p->fts_statp->st_flags & (SF_APPEND|SF_IMMUTABLE)) && - chflags(p->fts_accpath, + lchflags(p->fts_accpath, p->fts_statp->st_flags &= ~(UF_APPEND|UF_IMMUTABLE)) < 0) goto err; continue; @@ -250,7 +250,7 @@ rm_tree(char **argv) if (!uid && (p->fts_statp->st_flags & (UF_APPEND|UF_IMMUTABLE)) && !(p->fts_statp->st_flags & (SF_APPEND|SF_IMMUTABLE))) - rval = chflags(p->fts_accpath, + rval = lchflags(p->fts_accpath, p->fts_statp->st_flags &= ~(UF_APPEND|UF_IMMUTABLE)); if (rval == 0) { /* @@ -350,7 +350,7 @@ rm_file(char **argv) if (!uid && !S_ISWHT(sb.st_mode) && (sb.st_flags & (UF_APPEND|UF_IMMUTABLE)) && !(sb.st_flags & (SF_APPEND|SF_IMMUTABLE))) - rval = chflags(f, sb.st_flags & ~(UF_APPEND|UF_IMMUTABLE)); + rval = lchflags(f, sb.st_flags & ~(UF_APPEND|UF_IMMUTABLE)); if (rval == 0) { if (S_ISWHT(sb.st_mode)) rval = undelete(f); Modified: stable/7/usr.bin/find/function.c ============================================================================== --- stable/7/usr.bin/find/function.c Sun Jun 21 15:36:10 2009 (r194587) +++ stable/7/usr.bin/find/function.c Sun Jun 21 15:40:39 2009 (r194588) @@ -443,7 +443,7 @@ f_delete(PLAN *plan __unused, FTSENT *en if ((entry->fts_statp->st_flags & (UF_APPEND|UF_IMMUTABLE)) && !(entry->fts_statp->st_flags & (SF_APPEND|SF_IMMUTABLE)) && geteuid() == 0) - chflags(entry->fts_accpath, + lchflags(entry->fts_accpath, entry->fts_statp->st_flags &= ~(UF_APPEND|UF_IMMUTABLE)); /* rmdir directories, unlink everything else */ _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org" From: Martin Kammerhofer To: bug-followup@FreeBSD.org Cc: Subject: Re: bin/111226: [patch] Incorrect usage of chflags() in various FreeBSD utility programs Date: Fri, 03 Jul 2009 08:33:18 +0200 Originator update: Patches for 4 of the 5 problems mentioned have been comitted/mfc'd already. Only #4 (pkg_install) remains open - but this one is very unlikely to cause real problems anyway. Closing this PR is OK for me. Thanks, Martin State-Changed-From-To: open->closed State-Changed-By: gavin State-Changed-When: Sun Jan 17 10:29:17 UTC 2010 State-Changed-Why: Submitter feels this can be closed http://www.freebsd.org/cgi/query-pr.cgi?pr=111226 >Unformatted: