From: Joel Holdsworth joel@airwebreathe.org.uk
When read-only files are opened with FILE_WRITE_ATTRIBUTES, server must temporarily chmod the file to grant sufficient permissions to allow the file to be opened.
The previous implementation used stat(2), chmod(2), open(2) and fchmod(2) (or chmod(2) in the failure case). Therefore the file path was being resolved 3 or possible 4 times in rapid succession. These successive path resolutions provide an opportunity for a Time-of-Check/Time-of-Use (TOCTOU) race condition with other processes.
This patch mitigates against this by first opening the fd of the parent directory, and then using fstatat(2), fchmodat(2) and openat(2) to open the file relative to its parent directory.
This method does not completely eliminate the TOCTOU issue, but it does significantly reduce its extent.
Signed-off-by: Joel Holdsworth joel@airwebreathe.org.uk --- server/fd.c | 70 ++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 50 insertions(+), 20 deletions(-)
diff --git a/server/fd.c b/server/fd.c index 8966c8e0648..8e667f7b82a 100644 --- a/server/fd.c +++ b/server/fd.c @@ -25,6 +25,7 @@ #include <dirent.h> #include <errno.h> #include <fcntl.h> +#include <libgen.h> #include <limits.h> #include <signal.h> #include <stdarg.h> @@ -1888,6 +1889,51 @@ void get_nt_name( struct fd *fd, struct unicode_str *name ) name->len = fd->nt_namelen; }
+static int try_chmod_open( const char *name, int flags, mode_t *mode, unsigned int access ) { + struct stat st; + int dirfd, fd = -1; + int tmp_errno; + char filename_buf[PATH_MAX]; + const char *file_base_name; + + if (!(access & FILE_WRITE_ATTRIBUTES) || + (access & (FILE_WRITE_DATA | FILE_APPEND_DATA | FILE_WRITE_EA | FILE_READ_DATA | FILE_READ_EA | DELETE))) + return -1; + + strcpy( filename_buf, name ); + dirfd = open( dirname( filename_buf ), +#ifdef O_PATH + O_PATH +#else + O_RDONLY +#endif + ); + + if (dirfd != -1) + { + strcpy( filename_buf, name ); + file_base_name = basename( filename_buf ); + + if (!fstatat( dirfd, file_base_name, &st, 0 ) && + st.st_uid == getuid() && + !fchmodat( dirfd, file_base_name, st.st_mode | S_IRUSR, 0 )) + { + fd = openat( dirfd, file_base_name, O_RDONLY | (flags & ~(O_TRUNC | O_CREAT | O_EXCL)), *mode ); + tmp_errno = errno; + if (fd == -1) + fchmod( fd, st.st_mode ); + else + fchmodat( dirfd, file_base_name, st.st_mode, 0 ); + errno = tmp_errno; + *mode = st.st_mode; + } + + close(dirfd); + } + + return fd; +} + /* open() wrapper that returns a struct fd with no fd user set */ struct fd *open_fd( struct fd *root, const char *name, struct unicode_str nt_name, int flags, mode_t *mode, unsigned int access, @@ -1899,7 +1945,7 @@ struct fd *open_fd( struct fd *root, const char *name, struct unicode_str nt_nam int root_fd = -1; int rw_mode; char *path; - int restore_chmod = 0; + int mode_assigned = 0;
if (((options & FILE_DELETE_ON_CLOSE) && !(access & DELETE)) || ((options & FILE_DIRECTORY_FILE) && (flags & O_TRUNC))) @@ -1962,20 +2008,8 @@ struct fd *open_fd( struct fd *root, const char *name, struct unicode_str nt_nam else if (errno == EACCES) { /* try to change permissions temporarily to open a file descriptor */ - if ((access & FILE_WRITE_ATTRIBUTES) && - !(access & (FILE_WRITE_DATA | FILE_APPEND_DATA | FILE_WRITE_EA | - FILE_READ_DATA | FILE_READ_EA | DELETE)) && - !stat( name, &st ) && st.st_uid == getuid() && - !chmod( name, st.st_mode | S_IRUSR )) - { - fd->unix_fd = open( name, O_RDONLY | (flags & ~(O_TRUNC | O_CREAT | O_EXCL)), *mode ); - if (fd->unix_fd == -1) - chmod( name, st.st_mode ); - else - fchmod( fd->unix_fd, st.st_mode ); - *mode = st.st_mode; - restore_chmod = 1; - } + if ((fd->unix_fd = try_chmod_open( name, flags, mode, access )) != -1) + mode_assigned = 1; else { set_error( STATUS_ACCESS_DENIED ); @@ -1990,8 +2024,6 @@ struct fd *open_fd( struct fd *root, const char *name, struct unicode_str nt_nam set_error( STATUS_OBJECT_NAME_INVALID ); else file_set_error(); - - if (restore_chmod) chmod( name, *mode ); goto error; } } @@ -2008,9 +2040,7 @@ struct fd *open_fd( struct fd *root, const char *name, struct unicode_str nt_nam closed_fd->unlink = 0; closed_fd->unix_name = fd->unix_name;
- if (restore_chmod) - chmod( name, *mode ); - else + if (!mode_assigned) { fstat( fd->unix_fd, &st ); *mode = st.st_mode;