# HG changeset patch # User Christian Brabandt # Date 1510848906 -3600 # Node ID 14f2875522181e4e71558cc93cfe3ca62a65cdd1 # Parent 09b3b28f4400e0220f2a4d79ee4d471824ef7957 patch 8.0.1300: file permissions may end up wrong when writing commit https://github.com/vim/vim/commit/cd142e3369db8888163a511dbe9907bcd138829c Author: Bram Moolenaar Date: Thu Nov 16 17:03:45 2017 +0100 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. diff --git a/src/auto/configure b/src/auto/configure --- a/src/auto/configure +++ b/src/auto/configure @@ -12090,13 +12090,13 @@ if test "x$vim_cv_getcwd_broken" = "xyes 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" diff --git a/src/config.h.in b/src/config.h.in --- a/src/config.h.in +++ b/src/config.h.in @@ -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 diff --git a/src/configure.ac b/src/configure.ac --- a/src/configure.ac +++ b/src/configure.ac @@ -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 diff --git a/src/fileio.c b/src/fileio.c --- a/src/fileio.c +++ b/src/fileio.c @@ -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,10 +3995,17 @@ 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); @@ -4005,11 +4013,12 @@ buf_write( } 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 @@ restore_backup: } 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 @@ restore_backup: # 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 @@ restore_backup: 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 diff --git a/src/os_unix.c b/src/os_unix.c --- a/src/os_unix.c +++ b/src/os_unix.c @@ -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 diff --git a/src/proto/os_unix.pro b/src/proto/os_unix.pro --- a/src/proto/os_unix.pro +++ b/src/proto/os_unix.pro @@ -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); diff --git a/src/version.c b/src/version.c --- a/src/version.c +++ b/src/version.c @@ -767,6 +767,8 @@ static char *(features[]) = static int included_patches[] = { /* Add new patch number below this line */ /**/ + 1300, +/**/ 1299, /**/ 1298,