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

2 files changed, 113 insertions, 20 deletions

diff --git a/src/lib/krb4/ChangeLog b/src/lib/krb4/ChangeLog
index 11953fb75..e260fd598 100644
--- 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 3470dc209..79884f3a0 100644
--- 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) {