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 15:05:04
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: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, juan@xtrarom.org
Subject: Re: bin/35687: /etc/rc.d/perusertmp doesn't work
Date: Sun, 25 Nov 2007 15:03:24 +0000

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