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
index f4cd8b492a22d77090302e4822fdf9a12e2a2cff..6e02374173c9fc614f573fc775a6f927f829159c 100755 (executable)
--- a/src/auto/configure
+++ b/src/auto/configure
@@ -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"

diff --git a/src/config.h.in b/src/config.h.in
index ec9fc592020933f75017904f05ffebb73edbb880..7d6122058f8d92fea9b9c3f7c7d2f6bd4d79577d 100644 (file)
--- 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
index 7277f8745f9fa3056f6ac232251d6b0b45f47cb3..938a77f914f3604bc04c5b4609a58a493e837af0 100644 (file)
--- 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
index d991822c3431a68df34e6c7b9127b0b2db1a1692..fbc164a914d621289fe4784dbdfd9dd9c066fc8e 100644 (file)
--- 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
index a39caffe4b161c4688e4cd26f49ac2aaaac4ea52..d2f080529e523241fcb01ba655ed15c477b7c819 100644 (file)
--- 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 <sys/acl.h>

diff --git a/src/proto/os_unix.pro b/src/proto/os_unix.pro
index bdbe17f770d5ba76e31fb45ad1782099ceb005d8..d3a00b5a24811074630f252d965517861c271b8b 100644 (file)
--- 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
index cafdd62e72fbb0bbd985e5db4e4836b249328eb5..2a50a1b5146468190bde71bac360f65d38f2f396 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -766,6 +766,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1300,
 /**/
     1299,
 /**/