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