From: Matthew Hancher Date: Thu, 9 Jul 1998 23:42:03 +0000 (+0000) Subject: Thu Jul 9 19:35:01 1998 Matthew D Hancher X-Git-Tag: krb5-1.1-beta1~666 X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=e6cdc7486d7448e14227db1fd928ec61ce545069;p=krb5.git Thu Jul 9 19:35:01 1998 Matthew D Hancher * 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 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 + + * 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 * 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) {