patch 8.0.1300: file permissions may end up wrong when writingv8.0.1300

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.

1 files changed, 64 insertions, 16 deletions

diff --git a/src/fileio.c b/src/fileio.c
index d991822c34..fbc164a914 100644
--- 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