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++] = '@';