From schwim@whatmore.Stanford.EDU Wed Mar 18 22:27:50 1998 Received: from MIT.EDU (PACIFIC-CARRIER-ANNEX.MIT.EDU [18.69.0.28]) by rt-11.MIT.EDU (8.7.5/8.7.3) with SMTP id WAA25422 for ; Wed, 18 Mar 1998 22:27:49 -0500 Received: from whatmore.Stanford.EDU by MIT.EDU with SMTP id AA14576; Wed, 18 Mar 98 22:28:21 EST Received: (from schwim@localhost) by whatmore.Stanford.EDU (8.8.8/8.8.8) id TAA04743; Wed, 18 Mar 1998 19:27:47 -0800 (PST) Message-Id: <199803190327.TAA04743@whatmore.Stanford.EDU> Date: Wed, 18 Mar 1998 19:27:46 -0800 (PST) From: Larry Schwimmer Cc: krb5-bugs@MIT.EDU, schwim@leland.Stanford.EDU In-Reply-To: <199803190138.RAA04137@whatmore.Stanford.EDU> from "Larry Schwimmer" at Mar 18, 98 05:38:33 pm Subject: Re: BUG: possible lib/krb4/tf_util.c race condition >Number: 566 >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: Wed Mar 18 22:28:00 EST 1998 >Last-Modified: Thu Jul 09 18:29:52 EDT 1998 >Originator: >Organization: >Release: >Environment: >Description: >How-To-Repeat: >Fix: >Audit-Trail: State-Changed-From-To: open-closed State-Changed-By: mdh State-Changed-When: Thu Jul 9 18:21:28 1998 State-Changed-Why: Superseded by PR 567. Responsible-Changed-From-To: gnats-admin->mdh Responsible-Changed-By: mdh Responsible-Changed-When: Thu Jul 9 18:29:27 1998 Responsible-Changed-Why: I'll take this. >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 Sorry, let try that one more time. Since the file exists, O_EXCL is not an option for creation. So, open then fstat. I did an lstat after that to check that the file opened is the one that the code thinks it opened. I'm not positive that is needed, but it shouldn't hurt. I haven't dealt with the shared memory code in this patch; the fopen call probably also needs a check on the file. yours, Larry Schwimmer schwim@leland.stanford.edu Leland Systems Group --- lib/krb4/tf_util.c.orig Fri Feb 6 19:44:22 1998 +++ lib/krb4/tf_util.c Wed Mar 18 19:20:11 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_buf) < 0) { + (void) close(fd); + fd = -1; + switch (errno) { + case ENOENT: + return NO_TKT_FIL; + default: + return TKT_FIL_ACC; + } + } + if ((stat_buf.st_uid != me && me != 0) || + ((stat_buf.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) {