[2899] in Kerberos-V5-bugs
pending/567: Re: BUG: possible lib/krb4/tf_util.c race condition
daemon@ATHENA.MIT.EDU (Larry Schwimmer)
Thu Mar 19 08:12:35 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: Thu, 19 Mar 1998 05:09:51 -0800 (PST)
From: Larry Schwimmer <schwim@whatmore.Stanford.EDU>
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
>Number: 567
>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: Thu Mar 19 08:11: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
*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) {
