checkpath: use fchown and fchmod to handle ownership and mode changes

This is related to #195.

This is an attempt to shorten the window for the first two issues
discussed by using a file descriptor which does not follow symbolic
links and using the fchmod and fchown calls instead of chown and chmod.
with.
This commit is contained in:
William Hubbs 2018-01-23 16:56:06 -06:00
parent 87c98ebb01
commit 1771bc2a83

View File

@ -73,25 +73,32 @@ static int do_check(char *path, uid_t uid, gid_t gid, mode_t mode,
inode_t type, bool trunc, bool chowner, bool selinux_on) inode_t type, bool trunc, bool chowner, bool selinux_on)
{ {
struct stat st; struct stat st;
int fd, flags; int fd;
int flags;
int r; int r;
int readfd;
int readflags;
int u; int u;
memset(&st, 0, sizeof(st)); memset(&st, 0, sizeof(st));
if (lstat(path, &st) || trunc) { flags = O_CREAT|O_NDELAY|O_WRONLY|O_NOCTTY;
readflags = O_NDELAY|O_NOCTTY|O_RDONLY;
#ifdef O_CLOEXEC
flags |= O_CLOEXEC;
readflags |= O_CLOEXEC;
#endif
#ifdef O_NOFOLLOW
flags |= O_NOFOLLOW;
readflags |= O_NOFOLLOW;
#endif
if (trunc)
flags |= O_TRUNC;
readfd = open(path, readflags);
if (readfd == -1 || (type == inode_file && trunc)) {
if (type == inode_file) { if (type == inode_file) {
einfo("%s: creating file", path); einfo("%s: creating file", path);
if (!mode) /* 664 */ if (!mode) /* 664 */
mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH; mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH;
flags = O_CREAT|O_NDELAY|O_WRONLY|O_NOCTTY;
#ifdef O_CLOEXEC
flags |= O_CLOEXEC;
#endif
#ifdef O_NOFOLLOW
flags |= O_NOFOLLOW;
#endif
if (trunc)
flags |= O_TRUNC;
u = umask(0); u = umask(0);
fd = open(path, flags, mode); fd = open(path, flags, mode);
umask(u); umask(u);
@ -99,7 +106,9 @@ static int do_check(char *path, uid_t uid, gid_t gid, mode_t mode,
eerror("%s: open: %s", applet, strerror(errno)); eerror("%s: open: %s", applet, strerror(errno));
return -1; return -1;
} }
close (fd); if (readfd != -1 && trunc)
close(readfd);
readfd = fd;
} else if (type == inode_dir) { } else if (type == inode_dir) {
einfo("%s: creating directory", path); einfo("%s: creating directory", path);
if (!mode) /* 775 */ if (!mode) /* 775 */
@ -113,7 +122,12 @@ static int do_check(char *path, uid_t uid, gid_t gid, mode_t mode,
strerror (errno)); strerror (errno));
return -1; return -1;
} }
mode = 0; readfd = open(path, readflags);
if (readfd == -1) {
eerror("%s: unable to open directory: %s", applet,
strerror(errno));
return -1;
}
} else if (type == inode_fifo) { } else if (type == inode_fifo) {
einfo("%s: creating fifo", path); einfo("%s: creating fifo", path);
if (!mode) /* 600 */ if (!mode) /* 600 */
@ -126,34 +140,46 @@ static int do_check(char *path, uid_t uid, gid_t gid, mode_t mode,
strerror (errno)); strerror (errno));
return -1; return -1;
} }
readfd = open(path, readflags);
if (readfd == -1) {
eerror("%s: unable to open fifo: %s", applet,
strerror(errno));
return -1;
} }
} else { }
}
if (fstat(readfd, &st) != -1) {
if (type != inode_dir && S_ISDIR(st.st_mode)) { if (type != inode_dir && S_ISDIR(st.st_mode)) {
eerror("%s: is a directory", path); eerror("%s: is a directory", path);
close(readfd);
return 1; return 1;
} }
if (type != inode_file && S_ISREG(st.st_mode)) { if (type != inode_file && S_ISREG(st.st_mode)) {
eerror("%s: is a file", path); eerror("%s: is a file", path);
close(readfd);
return 1; return 1;
} }
if (type != inode_fifo && S_ISFIFO(st.st_mode)) { if (type != inode_fifo && S_ISFIFO(st.st_mode)) {
eerror("%s: is a fifo", path); eerror("%s: is a fifo", path);
close(readfd);
return -1; return -1;
} }
}
if (mode && (st.st_mode & 0777) != mode) { if (mode && (st.st_mode & 0777) != mode) {
if ((type != inode_dir) && (st.st_nlink > 1)) { if ((type != inode_dir) && (st.st_nlink > 1)) {
eerror("%s: chmod: %s %s", applet, "Too many hard links to", path); eerror("%s: chmod: %s %s", applet, "Too many hard links to", path);
close(readfd);
return -1; return -1;
} }
if (S_ISLNK(st.st_mode)) { if (S_ISLNK(st.st_mode)) {
eerror("%s: chmod: %s %s", applet, path, " is a symbolic link"); eerror("%s: chmod: %s %s", applet, path, " is a symbolic link");
close(readfd);
return -1; return -1;
} }
einfo("%s: correcting mode", path); einfo("%s: correcting mode", path);
if (chmod(path, mode)) { if (fchmod(readfd, mode)) {
eerror("%s: chmod: %s", applet, strerror(errno)); eerror("%s: chmod: %s", applet, strerror(errno));
close(readfd);
return -1; return -1;
} }
} }
@ -161,21 +187,29 @@ static int do_check(char *path, uid_t uid, gid_t gid, mode_t mode,
if (chowner && (st.st_uid != uid || st.st_gid != gid)) { if (chowner && (st.st_uid != uid || st.st_gid != gid)) {
if ((type != inode_dir) && (st.st_nlink > 1)) { if ((type != inode_dir) && (st.st_nlink > 1)) {
eerror("%s: chown: %s %s", applet, "Too many hard links to", path); eerror("%s: chown: %s %s", applet, "Too many hard links to", path);
close(readfd);
return -1; return -1;
} }
if (S_ISLNK(st.st_mode)) { if (S_ISLNK(st.st_mode)) {
eerror("%s: chown: %s %s", applet, path, " is a symbolic link"); eerror("%s: chown: %s %s", applet, path, " is a symbolic link");
close(readfd);
return -1; return -1;
} }
einfo("%s: correcting owner", path); einfo("%s: correcting owner", path);
if (lchown(path, uid, gid)) { if (fchown(readfd, uid, gid)) {
eerror("%s: lchown: %s", applet, strerror(errno)); eerror("%s: chown: %s", applet, strerror(errno));
close(readfd);
return -1; return -1;
} }
} }
if (selinux_on) if (selinux_on)
selinux_util_label(path); selinux_util_label(path);
} else {
eerror(fstat: %s: %s", path, strerror(errno));
close(readfd);
return -1;
}
close(readfd);
return 0; return 0;
} }