Hi, 2017/11/1 Wed 12:32:43 UTC+9 Ken Takata wrote: > Hi, > > 2017/11/1 Wed 6:20:27 UTC+9 Bram Moolenaar wrote: > > Hanno Böck wrote: > > > > > I wanted to point out an issue here with vim swap files that make them > > > a security problem. > > > > > > By default vim creates a file with the name .filename.swp in the same > > > directory while editing. They contain the full content of the edited > > > file. This usually gets deleted upon exit, but not if vim crashes or > > > gets killed (e.g. due to a reboot). > > > > > > On web servers this can be a severe security risk. One can e.g. scan > > > for web hosts that have swap files of PHP configuration files and thus > > > expose settings like database passwords. (e.g. wget > > > http://example.com/.wp-config.php.swp ) > > > > Why would a web server expose and serve such a file? That clearly is > > the problem, not that Vim happens to create swap files (and undo and > > backup files, depending on your configuration). > > > > You probably also create new files and copies of files that should not > > be served. If you care about security, the web server must always use > > whitelisting, only serve files that were intentionally made public. > > > > > In a scan of the alexa top 1 million I found ~750 instances of such > > > files. I tried to inform affected people as best as I could. I also > > > discovered such scans in my own web server logs, so I assume black hats > > > are already aware of this and it's actively exploitet. > > > > > > I was wondering how to best avoid this on my own servers and I first > > > thought about saving the swap files to tmp ( with "set directory"). > > > However on multiuser systems this creates another security problem. > > > These files are world readable, thus instead of leaking information to > > > the world it's now leaking information to other users on the same > > > system. Thus even if one is aware of the issue it's nontrivial to get > > > secure settings (I've now worked around this by having per-user tmp > > > dirs with secure permissions.) > > > > > > I think vim should change the behavior of swap files: > > > 1. they should be stored in /tmp by default > > > > No, on Linux this is wiped clean on reboot. You lose your work on a > > system crash. > > > > > 2. they should have secure permissions (tmp file security is > > > a tricky thing and needs careful consideration to avoid symlink attacks > > > and the like, but there are dedicated functions for this like mkstemp). > > > > The permissions are the same as the original file, and that is exactly > > how it should be. > > > > > 3. Ideally they also shouldn't leak currently edited filenames (e.g. > > > they shouldn't be called /tmp/.test.txt.swp, but more something > > > like /tmp/.vim_swap.123782173) > > > > Infeasible, Vim can't find that file when trying to recover the original > > file. > > An issue related to this (not the same) is filed as CVE-2017-1000382: > http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-1000382 > https://security-tracker.debian.org/tracker/CVE-2017-1000382 > > It seems that the problem is that Vim ignores umask: > http://www.openwall.com/lists/oss-security/2017/10/31/15 > > (Similar one for Emacs: > http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-1000383 )
How about the attached patch? This makes Vim to respect umask when creating a swapfile. (I'm not sure this is actually needed, though.) Regards, Ken Takata -- -- You received this message from the "vim_dev" maillist. Do not top-post! Type your reply below the text you are replying to. For more information, visit http://www.vim.org/maillist.php --- You received this message because you are subscribed to the Google Groups "vim_dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
# HG changeset patch # Parent 8a009ace74e43d2117cc4a17600b3de60ab08de4 diff --git a/src/fileio.c b/src/fileio.c --- a/src/fileio.c +++ b/src/fileio.c @@ -509,9 +509,16 @@ readfile( * (they must not write the swapfile). * Add the "read" and "write" bits for the user, otherwise we may * not be able to write to the file ourselves. + * Also respect umask. * Setting the bits is done below, after creating the swap file. */ - swap_mode = (st.st_mode & 0644) | 0600; + { + mode_t umask_save; + + umask_save = umask(0); /* Get current umask. */ + (void)umask(umask_save); + swap_mode = (st.st_mode & 0644 & ~umask_save) | 0600; + } #endif #ifdef FEAT_CW_EDITOR /* Get the FSSpec on MacOS