Subject: Re: bin/35687: /etc/rc.d/perusertmp doesn't work
To: None <gnats-bugs@NetBSD.org>
From: Matt Fleming <mjf@NetBSD.org>
List: netbsd-bugs
Date: 11/25/2007 15:03:24
Attached is a new patch based on input from Elad Efrat. We consistently
check the return values of stat/chown/chmod and print a more descriptive
message via syslog() if an error has occurred.
--mjf
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 13:04:30 -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') {
@@ -633,11 +637,67 @@
}
(void)sprintf(lp, "/%u", pwd->pw_uid); /* safe */
if (mkdir(per_user_tmp, S_IRWXU) != -1) {
- (void)chown(per_user_tmp, pwd->pw_uid,
- pwd->pw_gid);
+ if (chown(per_user_tmp, pwd->pw_uid,
+ pwd->pw_gid)) {
+ syslog(LOG_ERR, "chown %s: %m",
+ per_user_tmp);
+ login_close(flc);
+ return (-1);
+ }
+
+ /*
+ * Must set sticky bit for tmp directory, some
+ * programs rely on this.
+ */
+ if(chmod(per_user_tmp, S_IRWXU | S_ISVTX)) {
+ syslog(LOG_ERR, "chmod %s: %m",
+ per_user_tmp);
+ login_close(flc);
+ return (-1);
+ }
} else {
- syslog(LOG_ERR, "can't create `%s' directory",
- per_user_tmp);
+ if (errno != EEXIST) {
+ syslog(LOG_ERR, "mkdir %s: %m",
+ per_user_tmp);
+ 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, "stat %s: %m",
+ per_user_tmp);
+ login_close(flc);
+ return (-1);
+ }
+
+ if (sb.st_uid != pwd->pw_uid) {
+ if (chown(per_user_tmp,
+ pwd->pw_uid, pwd->pw_gid)) {
+ syslog(LOG_ERR,
+ "chown %s: %m",
+ per_user_tmp);
+ 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,
+ "chmod %s: %m",
+ per_user_tmp);
+ 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 13:04:30 -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++] = '@';