Permalink
Browse files

patch 8.0.1300: file permissions may end up wrong when writing

Problem:    File permissions may end up wrong when writing.
Solution:   Use fchmod() instead of chmod() when possible.  Don't truncate
            until we know we can change the file.
  • Loading branch information...
brammool committed Nov 16, 2017
1 parent a42ad57 commit cd142e3369db8888163a511dbe9907bcd138829c
Showing with 88 additions and 24 deletions.
  1. +2 −2 src/auto/configure
  2. +3 −1 src/config.h.in
  3. +2 −2 src/configure.ac
  4. +64 −16 src/fileio.c
  5. +14 −3 src/os_unix.c
  6. +1 −0 src/proto/os_unix.pro
  7. +2 −0 src/version.c
View
@@ -12090,13 +12090,13 @@ if test "x$vim_cv_getcwd_broken" = "xyes" ; then
fi
for ac_func in fchdir fchown fsync getcwd getpseudotty \
for ac_func in fchdir fchown fchmod fsync getcwd getpseudotty \
getpwent getpwnam getpwuid getrlimit gettimeofday getwd lstat \
memset mkdtemp nanosleep opendir putenv qsort readlink select setenv \
getpgid setpgid setsid sigaltstack sigstack sigset sigsetjmp sigaction \
sigprocmask sigvec strcasecmp strerror strftime stricmp strncasecmp \
strnicmp strpbrk strtol tgetent towlower towupper iswupper \
usleep utime utimes mblen
usleep utime utimes mblen ftruncate
do :
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
View
@@ -156,9 +156,11 @@
/* Define if you the function: */
#undef HAVE_FCHDIR
#undef HAVE_FCHOWN
#undef HAVE_FCHMOD
#undef HAVE_FLOAT_FUNCS
#undef HAVE_FSEEKO
#undef HAVE_FSYNC
#undef HAVE_FLOAT_FUNCS
#undef HAVE_FTRUNCATE
#undef HAVE_GETCWD
#undef HAVE_GETPGID
#undef HAVE_GETPSEUDOTTY
View
@@ -3655,13 +3655,13 @@ fi
dnl Check for functions in one big call, to reduce the size of configure.
dnl Can only be used for functions that do not require any include.
AC_CHECK_FUNCS(fchdir fchown fsync getcwd getpseudotty \
AC_CHECK_FUNCS(fchdir fchown fchmod fsync getcwd getpseudotty \
getpwent getpwnam getpwuid getrlimit gettimeofday getwd lstat \
memset mkdtemp nanosleep opendir putenv qsort readlink select setenv \
getpgid setpgid setsid sigaltstack sigstack sigset sigsetjmp sigaction \
sigprocmask sigvec strcasecmp strerror strftime stricmp strncasecmp \
strnicmp strpbrk strtol tgetent towlower towupper iswupper \
usleep utime utimes mblen)
usleep utime utimes mblen ftruncate)
AC_FUNC_FSEEKO
dnl define _LARGE_FILES, _FILE_OFFSET_BITS and _LARGEFILE_SOURCE when
View
@@ -3863,6 +3863,7 @@ buf_write(
char_u *rootname;
#if defined(UNIX)
int did_set_shortname;
mode_t umask_save;
#endif
copybuf = alloc(BUFSIZE + 1);
@@ -3994,22 +3995,30 @@ buf_write(
/* remove old backup, if present */
mch_remove(backup);
/* Open with O_EXCL to avoid the file being created while
* we were sleeping (symlink hacker attack?) */
* we were sleeping (symlink hacker attack?). Reset umask
* if possible to avoid mch_setperm() below. */
#ifdef UNIX
umask_save = umask(0);
#endif
bfd = mch_open((char *)backup,
O_WRONLY|O_CREAT|O_EXTRA|O_EXCL|O_NOFOLLOW,
perm & 0777);
#ifdef UNIX
(void)umask(umask_save);
#endif
if (bfd < 0)
{
vim_free(backup);
backup = NULL;
}
else
{
/* set file protection same as original file, but
* strip s-bit */
/* Set file protection same as original file, but
* strip s-bit. Only needed if umask() wasn't used
* above. */
#ifndef UNIX
(void)mch_setperm(backup, perm & 0777);
#ifdef UNIX
#else
/*
* Try to set the group of the backup same as the
* original file. If this fails, set the protection
@@ -4377,6 +4386,11 @@ buf_write(
}
else
{
#ifdef HAVE_FTRUNCATE
# define TRUNC_ON_OPEN 0
#else
# define TRUNC_ON_OPEN O_TRUNC
#endif
/*
* Open the file "wfname" for writing.
* We may try to open the file twice: If we can't write to the file
@@ -4389,7 +4403,7 @@ buf_write(
*/
while ((fd = mch_open((char *)wfname, O_WRONLY | O_EXTRA | (append
? (forceit ? (O_APPEND | O_CREAT) : O_APPEND)
: (O_CREAT | O_TRUNC))
: (O_CREAT | TRUNC_ON_OPEN))
, perm < 0 ? 0666 : (perm & 0777))) < 0)
{
/*
@@ -4482,6 +4496,30 @@ buf_write(
}
write_info.bw_fd = fd;
#if defined(UNIX)
{
stat_T st;
/* Double check we are writing the intended file before making
* any changes. */
if (overwriting
&& (!dobackup || backup_copy)
&& fname == wfname
&& perm >= 0
&& mch_fstat(fd, &st) == 0
&& st.st_ino != st_old.st_ino)
{
close(fd);
errmsg = (char_u *)_("E949: File changed while writing");
goto fail;
}
}
#endif
#ifdef HAVE_FTRUNCATE
if (!append)
ftruncate(fd, (off_t)0);
#endif
#if defined(WIN3264)
if (backup != NULL && overwriting && !append)
{
@@ -4752,15 +4790,17 @@ buf_write(
# ifdef HAVE_FCHOWN
stat_T st;
/* don't change the owner when it's already OK, some systems remove
* permission or ACL stuff */
/* Don't change the owner when it's already OK, some systems remove
* permission or ACL stuff. */
if (mch_stat((char *)wfname, &st) < 0
|| st.st_uid != st_old.st_uid
|| st.st_gid != st_old.st_gid)
{
ignored = fchown(fd, st_old.st_uid, st_old.st_gid);
if (perm >= 0) /* set permission again, may have changed */
(void)mch_setperm(wfname, perm);
/* changing owner might not be possible */
ignored = fchown(fd, st_old.st_uid, -1);
/* if changing group fails clear the group permissions */
if (fchown(fd, -1, st_old.st_gid) == -1 && perm > 0)
perm &= ~070;
}
# endif
buf_setino(buf);
@@ -4770,18 +4810,26 @@ buf_write(
buf_setino(buf);
#endif
#ifdef UNIX
if (made_writable)
perm &= ~0200; /* reset 'w' bit for security reasons */
#endif
#ifdef HAVE_FCHMOD
/* set permission of new file same as old file */
if (perm >= 0)
(void)mch_fsetperm(fd, perm);
#endif
if (close(fd) != 0)
{
errmsg = (char_u *)_("E512: Close failed");
end = 0;
}
#ifdef UNIX
if (made_writable)
perm &= ~0200; /* reset 'w' bit for security reasons */
#endif
if (perm >= 0) /* set perm. of new file same as old file */
#ifndef HAVE_FCHMOD
/* set permission of new file same as old file */
if (perm >= 0)
(void)mch_setperm(wfname, perm);
#endif
#ifdef HAVE_ACL
/*
* Probably need to set the ACL before changing the user (can't set the
View
@@ -2725,9 +2725,8 @@ mch_getperm(char_u *name)
}
/*
* set file permission for 'name' to 'perm'
*
* return FAIL for failure, OK otherwise
* Set file permission for "name" to "perm".
* Return FAIL for failure, OK otherwise.
*/
int
mch_setperm(char_u *name, long perm)
@@ -2741,6 +2740,18 @@ mch_setperm(char_u *name, long perm)
(mode_t)perm) == 0 ? OK : FAIL);
}
#if defined(HAVE_FCHMOD) || defined(PROTO)
/*
* Set file permission for open file "fd" to "perm".
* Return FAIL for failure, OK otherwise.
*/
int
mch_fsetperm(int fd, long perm)
{
return (fchmod(fd, (mode_t)perm) == 0 ? OK : FAIL);
}
#endif
#if defined(HAVE_ACL) || defined(PROTO)
# ifdef HAVE_SYS_ACL_H
# include <sys/acl.h>
View
@@ -36,6 +36,7 @@ int mch_isFullName(char_u *fname);
void fname_case(char_u *name, int len);
long mch_getperm(char_u *name);
int mch_setperm(char_u *name, long perm);
int mch_fsetperm(int fd, long perm);
void mch_copy_sec(char_u *from_file, char_u *to_file);
vim_acl_T mch_get_acl(char_u *fname);
void mch_set_acl(char_u *fname, vim_acl_T aclent);
View
@@ -766,6 +766,8 @@ static char *(features[]) =
static int included_patches[] =
{ /* Add new patch number below this line */
/**/
1300,
/**/
1299,
/**/

0 comments on commit cd142e3

Please sign in to comment.