From schwim@whatmore.Stanford.EDU Thu Mar 19 08:10:10 1998 Received: from MIT.EDU (SOUTH-STATION-ANNEX.MIT.EDU [18.72.1.2]) by rt-11.MIT.EDU (8.7.5/8.7.3) with SMTP id IAA27987 for ; Thu, 19 Mar 1998 08:10:09 -0500 Received: from whatmore.Stanford.EDU by MIT.EDU with SMTP id AA27729; Thu, 19 Mar 98 08:09:59 EST Received: (from schwim@localhost) by whatmore.Stanford.EDU (8.8.8/8.8.8) id FAA06007; Thu, 19 Mar 1998 05:09:52 -0800 (PST) Message-Id: <199803191309.FAA06007@whatmore.Stanford.EDU> Date: Thu, 19 Mar 1998 05:09:51 -0800 (PST) From: Larry Schwimmer Cc: krb5-bugs@MIT.EDU, schwim@leland.Stanford.EDU In-Reply-To: <199803190327.TAA04743@whatmore.Stanford.EDU> from "Larry Schwimmer" at Mar 18, 98 07:27:46 pm Subject: Re: BUG: possible lib/krb4/tf_util.c race condition >Number: 567 >Category: krb5-libs >Synopsis: Re: BUG: possible lib/krb4/tf_util.c race condition >Confidential: no >Severity: serious >Priority: medium >Responsible: mdh >State: closed >Class: sw-bug >Submitter-Id: unknown >Arrival-Date: Thu Mar 19 08:11:00 EST 1998 >Last-Modified: Thu Jul 09 19:56:24 EDT 1998 >Originator: >Organization: >Release: >Environment: >Description: >How-To-Repeat: >Fix: >Audit-Trail: Responsible-Changed-From-To: gnats-admin->mdh Responsible-Changed-By: mdh Responsible-Changed-When: Thu Jul 9 19:55:37 1998 Responsible-Changed-Why: Taken and solved. State-Changed-From-To: open-closed State-Changed-By: mdh State-Changed-When: Thu Jul 9 19:55:51 1998 State-Changed-Why: Fixed. See pr 560. >Unformatted: You (Larry Schwimmer) write: > You (Larry Schwimmer) write: > > Submitter-Id: net > > Originator: Larry Schwimmer > > Confidential: no > > Synopsis: tf_init has a /tmp race condition > > Severity: serious > > Priority: medium > > Category: krb5-libs > > Class: sw-bug > > Release: 1.0.5 > > Environment: All *sigh* Third time's the charm. (I used the wrong variable for the read code in the previous patch.) I've tested this against reading and writing of tickets. yours, Larry --- lib/krb4/tf_util.c.orig Fri Feb 6 19:44:22 1998 +++ lib/krb4/tf_util.c Thu Mar 19 03:37:22 1998 @@ -184,7 +184,7 @@ { int wflag; uid_t me, getuid(); - struct stat stat_buf; + struct stat stat_buf, stat_buffd; #ifdef TKT_SHMEM char shmidname[MAXPATHLEN]; FILE *sfp; @@ -207,17 +207,7 @@ 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"); @@ -280,6 +270,40 @@ if (wflag) { fd = open(tf_name, O_RDWR, 0600); if (fd < 0) { + return TKT_FIL_ACC; + } + /* Check the ownership on the file opened */ + if (fstat(fd, &stat_buffd) < 0) { + (void) close(fd); + fd = -1; + switch (errno) { + case ENOENT: + return NO_TKT_FIL; + default: + return TKT_FIL_ACC; + } + } + 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; + } + /* Check to see that the file opened is the correct one */ + if (lstat(tf_name, &stat_buf) < 0) { + (void) close(fd); + fd = -1; + switch (errno) { + case ENOENT: + return NO_TKT_FIL; + default: + return TKT_FIL_ACC; + } + } + 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; } if (flock(fd, LOCK_EX | LOCK_NB) < 0) { @@ -297,10 +321,45 @@ * for read-only operations and locked for shared access. */ - fd = open(tf_name, O_RDONLY, 0600); + fd = open(tf_name, O_RDONLY|O_NONBLOCK, 0600); if (fd < 0) { return TKT_FIL_ACC; } + /* Check the ownership on the file opened */ + if (fstat(fd, &stat_buffd) < 0) { + (void) close(fd); + fd = -1; + switch (errno) { + case ENOENT: + return NO_TKT_FIL; + default: + return TKT_FIL_ACC; + } + } + 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; + } + /* Check to see that the file opened is the correct one */ + if (lstat(tf_name, &stat_buf) < 0) { + (void) close(fd); + fd = -1; + switch (errno) { + case ENOENT: + return NO_TKT_FIL; + default: + return TKT_FIL_ACC; + } + } + 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; + } + if (flock(fd, LOCK_SH | LOCK_NB) < 0) { sleep(TF_LCK_RETRY); if (flock(fd, LOCK_SH | LOCK_NB) < 0) {