Mercurial > vim
diff src/fileio.c @ 12847:14f287552218 v8.0.1300
patch 8.0.1300: file permissions may end up wrong when writing
commit https://github.com/vim/vim/commit/cd142e3369db8888163a511dbe9907bcd138829c
Author: Bram Moolenaar <Bram@vim.org>
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.
author | Christian Brabandt <cb@256bit.org> |
---|---|
date | Thu, 16 Nov 2017 17:15:06 +0100 |
parents | 3f2f468b8b9d |
children | ffdf2e4b5d9a |
line wrap: on
line diff
--- 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