oss-sec mailing list archives

Re: Fw: Security risk of vim swap files


From: Jakub Wilk <jwilk () jwilk net>
Date: Sun, 5 Nov 2017 19:14:31 +0100

* Christian Brabandt <cb () 256bit org>, 2017-11-05, 18:17:
In general, what vim does (copying mode bits) in not enough to ensure that the swapfile is readable only by the users who had access to the original file. It would have to copy also group ownership and ACLs.
I think patch https://github.com/vim/vim/releases/tag/v8.0.1263 fixes the group ownership problem.

So the code in question looks like this:

  /*
   * If the group-read bit is set but not the world-read bit, then
   * the group must be equal to the group of the original file.  If
   * we can't make that happen then reset the group-read bit.  This
   * avoids making the swap file readable to more users when the
   * primary group of the user is too permissive.
   */
  if ((swap_mode & 044) == 040)
  {
      stat_T    swap_st;

      if (mch_stat((char *)swap_fname, &swap_st) >= 0
        && st.st_gid != swap_st.st_gid
        && fchown(curbuf->b_ml.ml_mfp->mf_fd, -1, st.st_gid)
                                                         == -1)
          swap_mode &= 0600;
  }

  (void)mch_setperm(swap_fname, (long)swap_mode);

The logic here is based on the assumption that the 040 bit in the mode implies that everyone in the group can read the file. Somewhat surprisingly, this assumption is incorrect in the world with ACLs:

  $ id -gn
  users

  $ ls -l foo
  -rw-r-----+ 1 root users 0 Nov  5 18:34 foo

  $ cat foo
  cat: foo: Permission denied

  $ getfacl foo
  # file: foo
  # owner: root
  # group: users
  user::rw-
  user:nobody:r--
  group::---
  mask::r--
  other::---

I don't understand why this chmodding is needed at all.
Couldn't vim create swapfiles with mode 0600 and be done with it?

--
Jakub Wilk


Current thread: