Thu Jul 9 19:35:01 1998 Matthew D Hancher <mdh@mit.edu>

	* tf_util.c (tf_init): Fixed a potential race condition in the opening
	of v4 ticket files. tf_init() was calling lstat() followed by fopen().
	Now it calls fopen() and then calls lstat() and fstat() to check file
	ownership and to check that it opened the file it thought it did. I
	patched the shared memory code similarly, but since nothing uses it I
	don't have a good way to test it properly.

git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@10629 dc483132-0cff-0310-8789-dd5450dbe970

diff --git a/src/lib/krb4/ChangeLog b/src/lib/krb4/ChangeLog
index 11953fb750445456ffb4cd6745c95acaf39b62aa..e260fd598470d622c1659cae9512efd598d7f4c6 100644 (file)
--- a/src/lib/krb4/ChangeLog
+++ b/src/lib/krb4/ChangeLog
@@ -1,3 +1,12 @@
+Thu Jul  9 19:35:01 1998  Matthew D Hancher  <mdh@mit.edu>
+
+       * tf_util.c (tf_init): Fixed a potential race condition in the opening 
+       of v4 ticket files. tf_init() was calling lstat() followed by fopen().
+       Now it calls fopen() and then calls lstat() and fstat() to check file 
+       ownership and to check that it opened the file it thought it did. I 
+       patched the shared memory code similarly, but since nothing uses it I
+       don't have a good way to test it properly.
+       
 Wed Jun 24 03:09:28 1998  Tom Yu  <tlyu@mit.edu>
 
        * mk_priv.c (krb_mk_priv): Fix up call to pcbc_encrypt().  By

diff --git a/src/lib/krb4/tf_util.c b/src/lib/krb4/tf_util.c
index 3470dc20901f421507d3420dfb1440aea9b73300..79884f3a06fede64a973c8c4f33fff98b4531e40 100644 (file)
--- a/src/lib/krb4/tf_util.c
+++ b/src/lib/krb4/tf_util.c
@@ -183,7 +183,7 @@ int tf_init(tf_name, rw)
 {
     int     wflag;
     uid_t   me, getuid();
-    struct stat stat_buf;
+    struct stat stat_buf, stat_buffd;
 #ifdef TKT_SHMEM
     char shmidname[MAXPATHLEN]; 
     FILE *sfp;
@@ -206,25 +206,9 @@ int tf_init(tf_name, rw)
     if (tf_name == 0)
        tf_name = tkt_string();
 
-    if (lstat(tf_name, &stat_buf) < 0)
-       switch (errno) {
-       case ENOENT:
-           return NO_TKT_FIL;
-       default:
-           return TKT_FIL_ACC;
-       }
-    me = getuid();
-    if ((stat_buf.st_uid != me && me != 0) ||
-       ((stat_buf.st_mode & S_IFMT) != S_IFREG))
-       return TKT_FIL_ACC;
 #ifdef TKT_SHMEM
     (void) strcpy(shmidname, tf_name);
     (void) strcat(shmidname, ".shm");
-    if (stat(shmidname,&stat_buf) < 0)
-       return(TKT_FIL_ACC);
-    if ((stat_buf.st_uid != me && me != 0) ||
-       ((stat_buf.st_mode & S_IFMT) != S_IFREG))
-       return TKT_FIL_ACC;
 #endif /* TKT_SHMEM */
 
     /*
@@ -239,15 +223,51 @@ int tf_init(tf_name, rw)
 #ifdef TKT_SHMEM
     sfp = fopen(shmidname, "r");       /* only need read/write on the
                                           actual tickets */
-    if (sfp == 0)
-       return TKT_FIL_ACC;
+    if (sfp == 0) {
+        switch(errno) {
+        case ENOENT:
+           return NO_TKT_FIL;
+       default:
+           return TKT_FIL_ACC;
+       }
+    }
+
+    /* lstat() and fstat() the file to check that the file we opened is the *
+     * one we think it is, and to check ownership.                          */
+    if ((fstat(sfp->_file, &stat_buffd) < 0) || 
+       (lstat(shmidname, &stat_buf) < 0)) {
+        (void) close(fd);
+       fd = -1;
+       switch(errno) {
+       case ENOENT:
+           return NO_TKT_FIL;
+       default:
+           return TKT_FIL_ACC;
+       }
+    }
+    /* Check that it's the right file */
+    if ((stat_buf.st_ino != stat_buffd.st_ino) ||
+       (stat_buf.st_dev != stat_buffd.st_dev)) {
+        (void) close(fd);
+       fd = -1;
+       return TKT_FIL_ACC;
+    }
+    /* Check ownership */
+    if ((stat_buffd.st_uid != me && me != 0) ||
+       ((stat_buffd.st_mode & S_IFMT) != S_IFREG)) {
+        (void) close(fd);
+       fd = -1;
+       return TKT_FIL_ACC;
+    }
+
+
+
     shmid = -1;
     {
        char buf[BUFSIZ];
        int val;                        /* useful for debugging fscanf */
        /* We provide our own buffer here since some STDIO libraries
           barf on unbuffered input with fscanf() */
-
        setbuf(sfp, buf);
        if ((val = fscanf(sfp,"%d",&shmid)) != 1) {
            (void) fclose(sfp);
@@ -279,6 +299,38 @@ int tf_init(tf_name, rw)
     if (wflag) {
        fd = open(tf_name, O_RDWR, 0600);
        if (fd < 0) {
+           switch(errno) {
+           case ENOENT:
+               return NO_TKT_FIL;
+           default:
+               return TKT_FIL_ACC;
+         }
+       }
+       /* lstat() and fstat() the file to check that the file we opened is the *
+        * one we think it is, and to check ownership.                          */
+       if ((fstat(fd, &stat_buffd) < 0) || 
+           (lstat(tf_name, &stat_buf) < 0)) {
+           (void) close(fd);
+           fd = -1;
+           switch(errno) {
+           case ENOENT:
+               return NO_TKT_FIL;
+           default:
+               return TKT_FIL_ACC;
+           }
+       }
+       /* Check that it's the right file */
+       if ((stat_buf.st_ino != stat_buffd.st_ino) ||
+           (stat_buf.st_dev != stat_buffd.st_dev)) {
+           (void) close(fd);
+           fd = -1;
+           return TKT_FIL_ACC;
+       }
+       /* Check ownership */
+       if ((stat_buffd.st_uid != me && me != 0) ||
+           ((stat_buffd.st_mode & S_IFMT) != S_IFREG)) {
+           (void) close(fd);
+           fd = -1;
            return TKT_FIL_ACC;
        }
        if (flock(fd, LOCK_EX | LOCK_NB) < 0) {
@@ -298,6 +350,38 @@ int tf_init(tf_name, rw)
 
     fd = open(tf_name, O_RDONLY, 0600);
     if (fd < 0) {
+        switch(errno) {
+       case ENOENT:
+           return NO_TKT_FIL;
+       default:
+           return TKT_FIL_ACC;
+       }
+    }
+    /* lstat() and fstat() the file to check that the file we opened is the *
+     * one we think it is, and to check ownership.                          */
+    if ((fstat(fd, &stat_buffd) < 0) || 
+       (lstat(tf_name, &stat_buf) < 0)) {
+        (void) close(fd);
+       fd = -1;
+       switch(errno) {
+       case ENOENT:
+           return NO_TKT_FIL;
+       default:
+           return TKT_FIL_ACC;
+       }
+    }
+    /* Check that it's the right file */
+    if ((stat_buf.st_ino != stat_buffd.st_ino) ||
+       (stat_buf.st_dev != stat_buffd.st_dev)) {
+        (void) close(fd);
+       fd = -1;
+       return TKT_FIL_ACC;
+    }
+    /* Check ownership */
+    if ((stat_buffd.st_uid != me && me != 0) ||
+       ((stat_buffd.st_mode & S_IFMT) != S_IFREG)) {
+        (void) close(fd);
+       fd = -1;
        return TKT_FIL_ACC;
     }
     if (flock(fd, LOCK_SH | LOCK_NB) < 0) {