diff options
-rw-r--r-- | src/tests/check_and_open-tests.c | 4 | ||||
-rw-r--r-- | src/util/check_and_open.c | 76 | ||||
-rw-r--r-- | src/util/util.h | 29 |
3 files changed, 79 insertions, 30 deletions
diff --git a/src/tests/check_and_open-tests.c b/src/tests/check_and_open-tests.c index 7ec8f3bc..e3d98868 100644 --- a/src/tests/check_and_open-tests.c +++ b/src/tests/check_and_open-tests.c @@ -100,11 +100,11 @@ START_TEST(test_symlink) ret = symlink(filename, newpath); fail_unless(ret == 0, "symlink failed [%d][%s]", ret, strerror(ret)); - ret = check_and_open_readonly(newpath, &fd, uid, gid, mode, CHECK_REG); + ret = check_file(newpath, uid, gid, mode, CHECK_REG, NULL); unlink(newpath); + fail_unless(ret == EINVAL, "check_and_open_readonly succeeded on symlink"); - fail_unless(fd == -1, "check_and_open_readonly file descriptor not -1"); } END_TEST diff --git a/src/util/check_and_open.c b/src/util/check_and_open.c index d010670b..db926f10 100644 --- a/src/util/check_and_open.c +++ b/src/util/check_and_open.c @@ -29,6 +29,10 @@ #include "util/util.h" +static errno_t perform_checks(struct stat *stat_buf, + const int uid, const int gid, + const int mode, enum check_file_type type); + errno_t check_file(const char *filename, const int uid, const int gid, const int mode, enum check_file_type type, struct stat *caller_stat_buf) @@ -36,7 +40,6 @@ errno_t check_file(const char *filename, const int uid, const int gid, int ret; struct stat local_stat_buf; struct stat *stat_buf; - bool type_check; if (caller_stat_buf == NULL) { stat_buf = &local_stat_buf; @@ -51,6 +54,39 @@ errno_t check_file(const char *filename, const int uid, const int gid, return errno; } + return perform_checks(stat_buf, uid, gid, mode, type); +} + +errno_t check_fd(int fd, const int uid, const int gid, + const int mode, enum check_file_type type, + struct stat *caller_stat_buf) +{ + int ret; + struct stat local_stat_buf; + struct stat *stat_buf; + + if (caller_stat_buf == NULL) { + stat_buf = &local_stat_buf; + } else { + stat_buf = caller_stat_buf; + } + + ret = fstat(fd, stat_buf); + if (ret == -1) { + DEBUG(1, ("fstat for [%d] failed: [%d][%s].\n", fd, errno, + strerror(errno))); + return errno; + } + + return perform_checks(stat_buf, uid, gid, mode, type); +} + +static errno_t perform_checks(struct stat *stat_buf, + const int uid, const int gid, + const int mode, enum check_file_type type) +{ + bool type_check; + switch (type) { case CHECK_DONT_CHECK_FILE_TYPE: type_check = true; @@ -77,28 +113,28 @@ errno_t check_file(const char *filename, const int uid, const int gid, type_check = S_ISSOCK(stat_buf->st_mode); break; default: - DEBUG(1, ("Unsupprted file type.\n")); + DEBUG(1, ("Unsupported file type.\n")); return EINVAL; } if (!type_check) { - DEBUG(1, ("File [%s] is not the right type.\n", filename)); + DEBUG(1, ("File is not the right type.\n")); return EINVAL; } if (mode >= 0 && (stat_buf->st_mode & ~S_IFMT) != mode) { - DEBUG(1, ("File [%s] has the wrong mode [%.7o], expected [%.7o].\n", - filename, (stat_buf->st_mode & ~S_IFMT), mode)); + DEBUG(1, ("File has the wrong mode [%.7o], expected [%.7o].\n", + (stat_buf->st_mode & ~S_IFMT), mode)); return EINVAL; } if (uid >= 0 && stat_buf->st_uid != uid) { - DEBUG(1, ("File [%s] must be owned by uid [%d].\n", filename, uid)); + DEBUG(1, ("File must be owned by uid [%d].\n", uid)); return EINVAL; } if (gid >= 0 && stat_buf->st_gid != gid) { - DEBUG(1, ("File [%s] must be owned by gid [%d].\n", filename, gid)); + DEBUG(1, ("File must be owned by gid [%d].\n", gid)); return EINVAL; } @@ -111,36 +147,20 @@ errno_t check_and_open_readonly(const char *filename, int *fd, const uid_t uid, { int ret; struct stat stat_buf; - struct stat fd_stat_buf; - - *fd = -1; - - ret = check_file(filename, uid, gid, mode, type, &stat_buf); - if (ret != EOK) { - DEBUG(1, ("check_file failed.\n")); - return ret; - } *fd = open(filename, O_RDONLY); if (*fd == -1) { DEBUG(1, ("open [%s] failed: [%d][%s].\n", filename, errno, - strerror(errno))); + strerror(errno))); return errno; } - ret = fstat(*fd, &fd_stat_buf); - if (ret == -1) { - DEBUG(1, ("fstat for [%s] failed: [%d][%s].\n", filename, errno, - strerror(errno))); - return errno; - } - - if (stat_buf.st_dev != fd_stat_buf.st_dev || - stat_buf.st_ino != fd_stat_buf.st_ino) { - DEBUG(1, ("File [%s] was modified between lstat and open.\n", filename)); + ret = check_fd(*fd, uid, gid, mode, type, &stat_buf); + if (ret != EOK) { close(*fd); *fd = -1; - return EIO; + DEBUG(1, ("check_fd failed.\n")); + return ret; } return EOK; diff --git a/src/util/util.h b/src/util/util.h index db8e1ac3..fae8096a 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -268,9 +268,38 @@ enum check_file_type { CHECK_LNK, CHECK_SOCK }; + +/* check_file() + * Verify that a file has certain permissions and/or is of a certain + * file type. This function can be used to determine if a file is a + * symlink. + * Warning: use of this function implies a potential race condition + * Opening a file before or after checking it does NOT guarantee that + * it is still the same file. Additional checks should be performed + * on the caller_stat_buf to ensure that it has the same device and + * inode to minimize impact. Permission changes may have occurred, + * however. + */ errno_t check_file(const char *filename, const int uid, const int gid, const int mode, enum check_file_type type, struct stat *caller_stat_buf); + +/* check_fd() + * Verify that an open file descriptor has certain permissions and/or + * is of a certain file type. This function CANNOT detect symlinks, + * as the file is already open and symlinks have been traversed. This + * is the safer way to perform file checks and should be preferred + * over check_file for nearly all situations. + */ +errno_t check_fd(int fd, const int uid, const int gid, + const int mode, enum check_file_type type, + struct stat *caller_stat_buf); + +/* check_and_open_readonly() + * Utility function to open a file and verify that it has certain + * permissions and is of a certain file type. This function wraps + * check_fd(), and is considered race-condition safe. + */ errno_t check_and_open_readonly(const char *filename, int *fd, const uid_t uid, const gid_t gid, const mode_t mode, enum check_file_type type); |