From owner-freebsd-bugs@FreeBSD.ORG Wed Apr 4 13:20:13 2007 Return-Path: X-Original-To: freebsd-bugs@hub.freebsd.org Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 7D0E916A4DA for ; Wed, 4 Apr 2007 13:20:13 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [69.147.83.40]) by mx1.freebsd.org (Postfix) with ESMTP id 5B99E13C484 for ; Wed, 4 Apr 2007 13:20:13 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.13.4/8.13.4) with ESMTP id l34DKDoZ086036 for ; Wed, 4 Apr 2007 13:20:13 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.13.4/8.13.4/Submit) id l34DKDaA086035; Wed, 4 Apr 2007 13:20:13 GMT (envelope-from gnats) Resent-Date: Wed, 4 Apr 2007 13:20:13 GMT Resent-Message-Id: <200704041320.l34DKDaA086035@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Martin Kammerhofer 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 Cc: Subject: bin/111226: Incorrect usage of chflags(2) in FreeBSD utility programs X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 04 Apr 2007 13:20:13 -0000 >Number: 111226 >Category: bin >Synopsis: Incorrect usage of chflags(2) in FreeBSD utility programs >Confidential: no >Severity: non-critical >Priority: medium >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Wed Apr 04 13:20:12 GMT 2007 >Closed-Date: >Last-Modified: >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: >Unformatted: