[2898] in Kerberos-V5-bugs
pending/566: Re: BUG: possible lib/krb4/tf_util.c race condition
daemon@ATHENA.MIT.EDU (Larry Schwimmer)
Wed Mar 18 22:28:07 1998
Resent-From: gnats@rt-11.MIT.EDU (GNATS Management)
Resent-To: gnats-admin@rt-11.MIT.EDU
Resent-Reply-To: krb5-bugs@MIT.EDU,
Larry Schwimmer <schwim@whatmore.Stanford.EDU>
Date: Wed, 18 Mar 1998 19:27:46 -0800 (PST)
From: Larry Schwimmer <schwim@whatmore.Stanford.EDU>
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
>Number: 566
>Category: pending
>Synopsis: Re: BUG: possible lib/krb4/tf_util.c race condition
>Confidential: yes
>Severity: serious
>Priority: medium
>Responsible: gnats-admin
>State: open
>Class: sw-bug
>Submitter-Id: unknown
>Arrival-Date: Wed Mar 18 22:28:00 EST 1998
>Last-Modified:
>Originator:
>Organization:
>Release:
>Environment:
>Description:
>How-To-Repeat:
>Fix:
>Audit-Trail:
>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) {
