bin/111226: Incorrect usage of chflags(2) in FreeBSD utility programs

Martin Kammerhofer dada at pluto.tugraz.at
Wed Apr 4 13:20:13 UTC 2007


>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 at 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 at Martin:~$ su -
Password: **********
root at Martin:~# touch myfile
root at Martin:~# ln -s myfile myslink
root at Martin:~# chflags -h uchg myfile myslink
root at 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 at Martin:~# rm -f myslink # fails and modifies myfile instead (1)+(2)
rm: myslink: Operation not permitted
root at 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 at Martin:~# cp -Rp myslink myslink2 # fails to copy uchg flag (5)
cp: chflags: myslink2: Function not implemented
root at 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:


More information about the freebsd-bugs mailing list