While this bug is still mostly about unsafe directories, I worked on a
patch for this problem that verifies the file type, owner, etc, before
chmod'ing (and it uses fchmod). (The fix from Jim Meyering is a good
start but doesn't handle checking for hardlinks, or for opening fifos
which need O_NONBLOCK.)
Given the knowledge that the file was just created, it must pass all the
tests in this patch (owned by euid, is the same type of file as what we
just created). Once that passes, we can call fchmod safely on the file
descriptor.
--
Kees Cook @outflux.net
diff -uNrp coreutils-5.97/src/mkdir.c coreutils-5.97-kees/src/mkdir.c
--- coreutils-5.97/src/mkdir.c 2006-08-11 09:33:49.998144368 -0700
+++ coreutils-5.97-kees/src/mkdir.c 2006-08-11 09:34:23.674765313 -0700
@@ -207,12 +207,23 @@ main (int argc, char **argv)
/* Set the permissions only if this directory has just
been created. */
- if (ok && specified_mode
- && chmod (dir, newmode) != 0)
+ if (ok && specified_mode)
{
- error (0, errno, _("cannot set permissions of directory %s"),
- quote (dir));
- ok = false;
+ int fd = -1;
+ struct stat statbuf;
+
+ if ( (fd = open (dir, O_RDONLY|O_NOFOLLOW|O_NONBLOCK)) < 0 ||
+ fstat (fd, &statbuf) < 0 ||
+ statbuf.st_uid != geteuid() ||
+ !S_ISDIR(statbuf.st_mode) ||
+ fchmod (fd, newmode) != 0)
+ {
+ error (0, errno, _("cannot set permissions of directory %s"),
+ quote (dir));
+ ok = false;
+ }
+
+ if ( fd != -1 ) close (fd);
}
}
diff -uNrp coreutils-5.97/src/mkfifo.c coreutils-5.97-kees/src/mkfifo.c
--- coreutils-5.97/src/mkfifo.c 2006-08-11 09:33:50.005143250 -0700
+++ coreutils-5.97-kees/src/mkfifo.c 2006-08-11 09:34:23.674765313 -0700
@@ -162,10 +162,21 @@ main (int argc, char **argv)
if (fail == 0 && specified_mode)
{
- fail = chmod (argv[optind], newmode);
- if (fail)
- error (0, errno, _("cannot set permissions of fifo %s"),
- quote (argv[optind]));
+ int fd = -1;
+ struct stat statbuf;
+
+ if ( (fd = open (argv[optind], O_RDONLY|O_NOFOLLOW|O_NONBLOCK)) < 0
||
+ fstat (fd, &statbuf) < 0 ||
+ statbuf.st_uid != geteuid() ||
+ !S_ISFIFO(statbuf.st_mode) ||
+ fchmod (fd, newmode) != 0)
+ {
+ fail = -1;
+ error (0, errno, _("cannot set permissions of fifo %s"),
+ quote (argv[optind]));
+ }
+
+ if ( fd != -1 ) close(fd);
}
if (fail)
diff -uNrp coreutils-5.97/src/mknod.c coreutils-5.97-kees/src/mknod.c
--- coreutils-5.97/src/mknod.c 2006-08-11 09:33:50.004143409 -0700
+++ coreutils-5.97-kees/src/mknod.c 2006-08-11 09:34:23.674765313 -0700
@@ -240,6 +240,7 @@ main (int argc, char **argv)
#ifndef S_ISFIFO
error (EXIT_FAILURE, 0, _("fifo files not supported"));
#else
+ node_type = S_IFIFO;
if (mkfifo (argv[optind], newmode))
error (EXIT_FAILURE, errno, "%s", quote (argv[optind]));
#endif
@@ -256,9 +257,20 @@ main (int argc, char **argv)
if (specified_mode)
{
- if (chmod (argv[optind], newmode))
- error (0, errno, _("cannot set permissions of %s"),
- quote (argv[optind]));
+ int fd = -1;
+ struct stat statbuf;
+
+ if ( (fd = open (argv[optind], O_RDONLY|O_NOFOLLOW|O_NONBLOCK)) < 0 ||
+ fstat (fd, &statbuf) < 0 ||
+ statbuf.st_uid != geteuid() ||
+ (statbuf.st_mode & node_type) != node_type ||
+ fchmod (fd, newmode) != 0)
+ {
+ error (0, errno, _("cannot set permissions of %s %d!=%d"),
+ quote (argv[optind]), (statbuf.st_mode | node_type),
node_type);
+ }
+
+ if ( fd != -1 ) close(fd);
}
exit (EXIT_SUCCESS);
