Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 4 Apr 2007 15:16:57 +0200
From:      Martin Kammerhofer <dada@pluto.tugraz.at>
To:        freebsd-gnats-submit@FreeBSD.org
Subject:   bin/111226: Incorrect usage of chflags(2) in FreeBSD utility programs
Message-ID:  <200704041316.l34DGvWs008651@pluto.tugraz.at>
Resent-Message-ID: <200704041320.l34DKDaA086035@freefall.freebsd.org>

next in thread | raw e-mail | index | archive | help

>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:



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200704041316.l34DGvWs008651>