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 [email protected].
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
