Skip Menu |
 

To: krb5-bugs@mit.edu
Subject: race in replay cache file ownership
Date: Tue, 07 Mar 2006 11:37:59 -0500
From: Roland Dowdeswell <elric@imrryr.org>
In the replay cache code, in krb5_rc_io_open() there is the following
logic:

if ((d->fd = stat(d->fn, &statb)) != -1) {
uid_t me;

me = geteuid();
/* must be owned by this user, to prevent some security problems with
* other users modifying replay cache stufff */
if ((statb.st_uid != me) || ((statb.st_mode & S_IFMT) != S_IFREG)) {
FREE(d->fn);
return KRB5_RC_IO_PERM;
}
d->fd = THREEPARAMOPEN(d->fn, O_RDWR | O_BINARY, 0600);
}

This is wrong and has a race between the stat(2) and the open. The
correct way to do this is:

d->fd = THREEPARAMOPEN(d->fn, O_RDWR | O_BINARY, 0600);
ret = fstat(d->fd, &statb);
.
.
.

I.e. fstat(2) the actual open fd in order to avoid the race.

Thanks,

--
Roland Dowdeswell http://www.Imrryr.ORG/~elric/
We should do the check after opening.

However, there are device files on some UNIX platforms where opening the file at all can have
potentially undesirable effects. So I think it's probably a good idea to keep the check before
opening, as well. (Though perhaps we want to use lstat, and make sure the replay cache "file"
isn't actually a symlink.)
(And yes, I know the "before" test still has a race condition, but it's probably better than not
doing it at all.)
Fixed in r21770 as submitted in ticket #6289.