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). > 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. 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. Regards Simon -- + privacy is necessary + using gnupg http://gnupg.org + public key id: 0x92FEFDB7E44C32F9 -- -- 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.
