bin/112336: install -S (safe copy) with -C or -p is not so safe

[John E Hein]

The following reply was made to PR bin/112336; it has been noted by GNATS.

From: John E Hein <jhein at timing.com>
To: FreeBSD-gnats-submit at FreeBSD.org, freebsd-bugs at FreeBSD.org
Cc:  
Subject: Re: bin/112336: install -S (safe copy) with -C or -p is not so safe
Date: Sat, 15 Dec 2007 13:34:12 -0700

 Here is an update to the patch to refresh it after a recent commit to
 xinstall.c and to additionally check euid which is important in some
 non-superuser cases.
 
 Index: xinstall.c
 ===================================================================
 RCS file: /base/FreeBSD-CVS/src/usr.bin/xinstall/xinstall.c,v
 retrieving revision 1.68
 diff -u -p -r1.68 xinstall.c
 --- xinstall.c	14 Dec 2007 08:46:57 -0000	1.68
 +++ xinstall.c	15 Dec 2007 20:21:35 -0000
 @@ -278,6 +278,7 @@ install(const char *from_name, const cha
  	int devnull, files_match, from_fd, serrno, target;
  	int tempcopy, temp_fd, to_fd;
  	char backup[MAXPATHLEN], *p, pathbuf[MAXPATHLEN], tempfile[MAXPATHLEN];
 +	uid_t euid;
  
  	files_match = 0;
  	from_fd = -1;
 @@ -322,7 +323,20 @@ install(const char *from_name, const cha
  	if (docompare && !dostrip && target) {
  		if ((to_fd = open(to_name, O_RDONLY, 0)) < 0)
  			err(EX_OSERR, "%s", to_name);
 -		if (devnull)
 +		/*
 +		 * Even if the contents are the same, we want to rename
 +		 * temp files when doing a "safe" copy if the
 +		 * permissions and ownership need to change.  We may
 +		 * not have permission to chown/chmod the "to" file
 +		 * directly.
 +		 */
 +		if (tempcopy && (euid = geteuid()) != 0 &&
 +		    euid != to_sb.st_uid &&
 +		    ((gid != (gid_t)-1 && gid != to_sb.st_gid) ||
 +		    (uid != (uid_t)-1 && uid != to_sb.st_uid) ||
 +		    (mode != (to_sb.st_mode & ALLPERMS))))
 +		    files_match = 0;
 +		else if (devnull)
  			files_match = to_sb.st_size == 0;
  		else
  			files_match = !(compare(from_fd, from_name,