Subject: Re: bin/35687: /etc/rc.d/perusertmp doesn't work
To: None <gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, juan@xtrarom.org>
From: Matt Fleming <mjf@NetBSD.org>
List: netbsd-bugs
Date: 11/25/2007 10:20:03
The following reply was made to PR bin/35687; it has been noted by GNATS.

From: Matt Fleming <mjf@NetBSD.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: bin/35687: /etc/rc.d/perusertmp doesn't work
Date: Sun, 25 Nov 2007 10:15:50 +0000

 --PEIAKu/WMn1b1Hv9
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: inline
 
 Attached is a patch that fixes per-user tmp for me. The problems that it
 fixes are,
 
 	- use kauth_get_getuid() when handling @uid with magic links.
 	  I think the original intention was that temp files were supposed to
 	  be created in tmp dir for the euid but this directory won't
 	  even exist unless that user has logged in, since all the code
 	  creating the tmp dir is in setusercontext(). I encountered
 	  this problem when trying to run chsh(1), the steps are as
 	  follows:
 		1) create temp file in user's private tmp
 		2) seteuid(0) to read password info to populate file
 		3) lookup the temp file to write to in the tmp
 		   directory, but because we're root that ends up being
 		   /private/tmp/0 (oops)
 		4) revoke privs and edit the file that was supposedly
 		   filled in step 2 but is in fact empty
 
 	- in setusercontext()
 
 		- the string used to calculate the per-user 
 	  	  tmp directory from the magic symlink was not nul-terminated
 
 		- the tmp directory that was created didn't
 	  	  set the sticky bit on the directory some programs (chsh) rely
 	      	  on this
 		
 		- if the private tmp directory already exists we must
 		  ensure that we are the owner and that it has the
 		  correct mode.
 
 --PEIAKu/WMn1b1Hv9
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: attachment; filename="per-tmp.patch"
 
 Index: lib/libutil/login_cap.c
 ===================================================================
 RCS file: /cvsroot/src/lib/libutil/login_cap.c,v
 retrieving revision 1.28
 diff -u -r1.28 login_cap.c
 --- lib/libutil/login_cap.c	6 Oct 2007 21:51:22 -0000	1.28
 +++ lib/libutil/login_cap.c	25 Nov 2007 10:14:55 -0000
 @@ -562,6 +562,7 @@
  	login_cap_t *flc;
  	quad_t p;
  	int i;
 +	ssize_t len;
  
  	flc = NULL;
  
 @@ -617,10 +618,13 @@
  	}
  
  	/* Create per-user temporary directories if needed. */
 -	if (readlink("/tmp", per_user_tmp, sizeof(per_user_tmp)) != -1) {
 +	if ((len = readlink("/tmp", per_user_tmp, sizeof(per_user_tmp))) != -1){
  		static const char atuid[] = "/@uid";
  		char *lp;
  
 +		/* readlink does not nul-terminate the string */
 +		per_user_tmp[len] = '\0';
 +
  		/* Check if it's magic symlink. */
  		lp = strstr(per_user_tmp, atuid);
  		if (lp != NULL && *(lp + (sizeof(atuid) - 1)) == '\0') {
 @@ -635,9 +639,47 @@
  			if (mkdir(per_user_tmp, S_IRWXU) != -1) {
  				(void)chown(per_user_tmp, pwd->pw_uid,
  				    pwd->pw_gid);
 +
 +				/* 
 +			 	 * Must set sticky bit for tmp directory, some
 +			 	 * programs rely on this.
 +			 	 */
 +				(void)chmod(per_user_tmp, S_IRWXU | S_ISVTX);
  			} else {
 -				syslog(LOG_ERR, "can't create `%s' directory",
 -				    per_user_tmp);
 +				/* We only get two tries at mkdir */
 +				if (errno != EEXIST) {
 +					syslog(LOG_ERR, "%m");
 +					login_close(flc);
 +					return (-1);
 +				} else {
 +					/* 
 +					 * We must ensure that we own the
 +					 * directory and that is has the correct
 +					 * permissions, otherwise a DOS attack
 +					 * is possible.
 +					 */
 +					struct stat sb;
 +					if (stat(per_user_tmp, &sb) == -1)
 +						syslog(LOG_ERR, "%m");
 +
 +					if (sb.st_uid != pwd->pw_uid) {
 +						if (chown(per_user_tmp, 
 +						    pwd->pw_uid, pwd->pw_gid)) {
 +							syslog(LOG_ERR, "%m");
 +							login_close(flc);
 +							return (-1);
 +						}
 +					}
 +
 +					if (sb.st_mode != (S_IRWXU | S_ISVTX)) {
 +						if (chmod(per_user_tmp, 
 +						    S_IRWXU | S_ISVTX)) {
 +							syslog(LOG_ERR, "%m");
 +							login_close(flc);
 +							return (-1);
 +						}
 +					}
 +				}
  			}
  		}
  	}
 Index: sys/kern/vfs_lookup.c
 ===================================================================
 RCS file: /cvsroot/src/sys/kern/vfs_lookup.c,v
 retrieving revision 1.99
 diff -u -r1.99 vfs_lookup.c
 --- sys/kern/vfs_lookup.c	7 Nov 2007 00:23:25 -0000	1.99
 +++ sys/kern/vfs_lookup.c	25 Nov 2007 10:14:55 -0000
 @@ -168,7 +168,7 @@
  			char uidtmp[11]; /* XXX elad */
  
  			(void)snprintf(uidtmp, sizeof(uidtmp), "%u",
 -			    kauth_cred_geteuid(kauth_cred_get()));
 +			    kauth_cred_getuid(kauth_cred_get()));
  			SUBSTITUTE("uid", uidtmp, strlen(uidtmp));
  		} else {
  			tmp[newlen++] = '@';
 
 --PEIAKu/WMn1b1Hv9--