Simon Ruderich wrote:
> On Sat, Nov 11, 2017 at 04:45:37PM +0100, Bram Moolenaar wrote:
> > Simon Ruderich wrote:
> >> Making viminfo readable by other users is most likely not useful. To
> >> prevent information leakage enforce mode 0600.
> >>
> >> The race condition fixed in the last patch could also cause viminfo
> >> files readable by other uses. Enforcing mode 0600 restores the
> >> originally indented permissions.
> >
> > I still don't see how the original problem could occur.
>
> Is there something I can do to explain it more clearly? I thought
> the patch (the original one, not this one which is just a
> follow-up) and its description explain the problem in enough
> detail. If not please tell me what should be explained better.
>
> > If we can't write a test for it,
>
> We can't write a test for it (or at least I don't know how)
> because it's a race condition which cannot be observed most of
> the time. It only happens if two Vim try to write the viminfo
> concurrently and if the one which can't create the .viminfo.tmp
> (and therefore uses the fallback method to create a new tempfile)
> renames its tempfile (which has the wrong permissions) after the
> first Vim, then the permissions are incorrect (readable by _all_
> users).
If .viminfo.tmp already exists, Vim will try .viminfz.tmp, .viminfy.tmp,
etc.
if .viminfo.tmp does not exist at first, but shows up in between the
stat() and the open() then the user is doing something really weird.
Or there is some corner case I can't imagine?
> > I don't see a reason to take away the documented
> > feature, that two users (likely to be the same person with two user
> > accounts) can share a viminfo file. This won't be used often, but can
> > be very annoying if it stops working.
>
> I agree, that's why this patch (which doesn't fix the original
> bug but enforces stricter permissions in general and fixes
> .viminfo files which were affected by the bug in the past) is
> just a follow-up and I only sent it to discuss if this use case
> is relevant enough.
>
> As this doesn't seem to be the case, please just ignore this
> patch and only apply the original one.
>
> > If the problem somehow does occur, then the problem will fix itself with
> > the new Vim when it happens again.
>
> No, it won't because the permissions are kept if the .viminfo
> exists. Therefore the .viminfo will stay readable by _all_ users!
>
> > I'll make the check for the group stricter, like what we did with the
> > swap file.
>
> I just saw the patch and I'm not sure what it's supposed to do.
> It only enforces the same user/group but doesn't affect any
> permissions and therefore can't fix the original issue.
It fixes the case where the user's primary group gives more permissions
than intended, and the viminfo has been given group read and/or write
permission, and somehow has a different group. Lots of conditions, so
no surprise nobody ran into this.
> TL;DR: please ignore this patch and apply "Possible security
> issue: [PATCH] viminfo: create fallback tempfile with restrictive
> umask" (Message-ID:
> <975991f5f00163b45d9ae1bb108a1a0064fa1f09.1510059861.git.si...@ruderich.org>)
> to fix the original race condition which makes the .viminfo
> readable by _all_ users.
That's already done. But keeping the group bits if possible.
--
The acknowledged parents of reengineering are Michael Hammer and James Champy.
When I say they're the "parents" I don't mean they had sex - and I apologize
for making you think about it. I mean they wrote the best-selling business
book _Reengineering the Corporation_, which was published in 1993.
Businesses flocked to reengineering like frat boys to a drunken
cheerleader. (This analogy wasn't necessary, but I'm trying to get my mind
off that Hammer and Champy thing.)
(Scott Adams - The Dilbert principle)
/// Bram Moolenaar -- [email protected] -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///
--
--
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.
