Re: MANPAGER can't handle man pages with upper case letters in name

2017-11-07 Thread Enno
Le mardi 10 octobre 2017 05:51:46 UTC-3, LCD 47 a écrit :
> Per title: if MANPAGER is set to "env MAN_PN=1 vim -M +MANPAGER -",
> running "man Xorg" results in a message "Cannot find a 'xorg'.".
> 
> The culprit is a "tolower()" in plugin/manpager.vim.  Man page names
> are case-sensitive on most UNIX systems.  The patch below seems to fix
> the problem.
> 
> /lcd
> 
> 
> diff --git a/runtime/plugin/manpager.vim b/runtime/plugin/manpager.vim
> index be6e30b70..9d30fab2f 100644
> --- a/runtime/plugin/manpager.vim
> +++ b/runtime/plugin/manpager.vim
> @@ -20,7 +20,7 @@ function! s:MANPAGER()
>  let manpage = expand('$MAN_PN')
>endif
>  
> -  let page_sec = matchlist(tolower(manpage), '^' . pagesec_pattern  . '$')
> +  let page_sec = matchlist(manpage, '^' . pagesec_pattern  . '$')
>  
>bwipe!


Dear LCD47 and Kazunobu Kuriyama,

the pull request that cherry-picks on your patches submitted here, 

  https://github.com/vim/vim/pull/2296

should resolve the problems about

- man pages not opening on Mac and 
- man pages containing a colon not being recognized.

It also documents that it is preferable NOT to set MAN_PN=1 when man-db 
is used, like on most Linux systems.

LCD47, you also had improvements on the manpage plugin. This is maintained by 
a different author, SungHyun Nam. The manpager.vim plugin only invokes his :Man 
command, where all the hard work is done.

If a shell script, for example in $VIMRUNTIME/macros, to work around the error 
appearing for manpages that contain capital letters on systems that do not set 
MAN_PN, can be made a stable replacement for the current Vim script, then this 
is the correct approach. The manpager.vim plugin is a mere makeshift solution 
to leverage the manpage.vim implementation. 

  Enno

-- 
-- 
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.


Re: Security risk of vim swap files

2017-11-07 Thread Christian Brabandt

On Di, 07 Nov 2017, Nikolay Aleksandrovich Pavlov wrote:

> - Yet another case *against* the idea of `set directory=.`: consider
> the simple script which simulates renaming directory without closing
> the file:

It is not about renaming a directory while simultaneously editing a 
file. What is meant with renaming a directory is, that if you have a 
swapfile from a crashed session laying around (and the path has changed 
because of moving/renaming a path component in between), Vim will 
automatically notice that a previous editing session crashed and offer 
you a way to recover your file.

Christian
-- 
Sein Jahrhundert kann man nicht verändern, aber man kann sich
dagegenstellen und glückliche Wirkungen vorbereiten.
-- Johann Wolfgang von Goethe (Lehrjahre)

-- 
-- 
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.


Re: Security risk of vim swap files

2017-11-07 Thread Bram Moolenaar

Nikolay Pavlov wrote:

> 2017-11-06 23:33 GMT+03:00 Bram Moolenaar :
> >
> > Scott Court wrote:
> >
> >> Those are very valid points, and I agree that the way Neovim handles
> >> .swp files is better. I've already explained on here and on Openwall
> >> numerous reasons why I believe that is the best solution and made the
> >> case that .swp files should be stored in ~/.vim/swap by default. However
> >> Bram has veto power and shot that idea down.
> >>
> >> So instead I'm trying to find the next best way to address this.
> >> /var/lib being writable only by root and therefore requiring cooperation
> >> from packagers did not occur to me, but that's definitely a problem.
> >> Maybe it would be doable as a major change in the next major release of
> >> Vim, but you're right; that definitely won't work as a security patch.
> >> So much for that idea.
> >>
> >> I maintain a Linux Distribution (Cucumber Linux) and have already
> >> adopted the Neovim style ~/.vim/swap approach on there. Maybe we'll just
> >> have to hope that other distributions independently start doing
> >> something similar, as Bram seems pretty convinced this problem is
> >> completely a user error and has nothing to do with the way Vim works;
> >> he's also pretty set on not changing the default .swp location.
> >
> > There are a few situations you need to consider:
> > - Removable media, editing a file on a USB stick.
> > - Remote file systems (where the mount point may change over time).
> > - Multiple users editing a shared file.
> > - Renaming directories.
> >
> > There are likely many more
> 
> - Remote file systems is a case *against* using swap files in the
> current directory, should they be slow Vim starts being unresponsive
> when it does something with swap file.

I don't get this.  I use remote file systems all the time, from
different systems.  Having the swap file in the same directory has saved
my work quite a number of times.  Sudden disconnects are the main thing,
having to go to the file from a different system and use "vim -r".  That
is impossible if the swap file is on a system that became unreachable.
If the original file becomes unreachable you can't do anything anyway.

On top of that, having the warning that a swap file exists, meaning you
already changed the file from another system, also often is a life
saver.  Happens for me quite often when working on a test that fails on
MS-Windows.

> - Removable media is a case against swap files as well: while nowadays
> you are unlikely to exhaust write cycles or cause unresponsiveness by
> using swap files there, there are still considerations like
> 
>   1. Whether “user saved and removed the media without properly
> closing the editor or wiping out buffer” is less common scenario then
> “user removed the media without …, but did not save and now wants to
> restore on another machine”. Swap files there are welcome only if
> second scenario is more common, but there is a second point as well.

That's exactly what happens: you edit a file on a USB stick, do
something else, have to catch a train, close the laptop without saving,
take the stick elsewhere, and now your swap file is on a system at home
while you are at work...

>   2. Removable media is also commonly used for sharing not between
> computers belonging to one user, but between computers belonging to
> different users. Swap files would be nothing other then annoying
> garbage when shared with a user not using *Vim and either using
> Windows (i.e. OS which does not hide dotfiles) or having his file
> manager configured to show hidden files. Also if files with swap files
> nearby were edited on such a machine trying to restore something from
> them will do more harm then good.

That does not match my experience, sounds uncommon to me.

> - Swap files are utterly useless for preventing editing a single file
> by multiple users: way too many conditions need to be true for that to
> work: users need to both use *Vim, users need to both have configured
> to save swap files in the current directory, users need to pay
> attention and not discard the message thinking e.g. that it was an
> artifact from previous disconnect.

It works very well for me.  In multi-user setups it does get more
tricky, but using your personal store for swap files isn't going to be
an improvement in any way.

The only situation I'm aware of is when the swap file gets picked up by
a tool in an unwanted way.  E.g. when creating an archive from a
directory or a version control system that isn't setup to ignore swap
files.  In that case you want to set 'directory', that's what it is for.

>   Also Vim does not provide any way to do anything sensible with the
> situation “two users simultaneously edit the same file”. False
> positives and negatives do not make the situation better as well:
> neither Vim checks whether currently running process with PID stored
> in swap file has anything to do with the creator of swap files, nor
> there is any way to d

Possible security issue: [PATCH] viminfo: create fallback tempfile with restrictive umask

2017-11-07 Thread Simon Ruderich
If the initial tempfile creation fails, possibly during a race condition
with two parallel vims writing the tempfile, then a fallback code is
used. However while the normal code uses a restrictive umask (or the
mode of the existing viminfo), the fallback code uses the default umask
of the user. This can result in a viminfo which is readable by all
users possibly leaking sensitive information.
---
 src/ex_cmds.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

Hello,

We noticed this issue on a multi-user system where viminfos
became readable by other users from time to time. We think this
issue is caused by the fallback code in write_viminfo() which
doesn't enforce a strict umask. This patch should fix this issue.

However this patch won't help users which were already affected
by this race condition as write_viminfo() retains the permissions
of an existing viminfo. As the use-case for a viminfo readable by
others doesn't seem very relevant to me, I recommend changing the
viminfo code to always enforce 0600 permissions to prevent this
kind of information leak (for affected users and in general).

Regards
Simon

diff --git a/src/ex_cmds.c b/src/ex_cmds.c
index 154372883..cc50409cf 100644
--- a/src/ex_cmds.c
+++ b/src/ex_cmds.c
@@ -2012,8 +2012,15 @@ write_viminfo(char_u *file, int forceit)
if (fp_out == NULL)
{
vim_free(tempname);
-   if ((tempname = vim_tempname('o', TRUE)) != NULL)
+   if ((tempname = vim_tempname('o', TRUE)) != NULL) {
+#if defined(UNIX) || defined(VMS)
+   umask_save = umask(077);
+#endif
fp_out = mch_fopen((char *)tempname, WRITEBIN);
+#if defined(UNIX) || defined(VMS)
+   (void)umask(umask_save);
+#endif
+   }
}
 
 #if defined(UNIX) && defined(HAVE_FCHOWN)
-- 
2.15.0

-- 
+ 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 vim_dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Possible security issue: [PATCH] viminfo: create fallback tempfile with restrictive umask

2017-11-07 Thread Bram Moolenaar

Simon Ruderich wrote:

> If the initial tempfile creation fails, possibly during a race condition
> with two parallel vims writing the tempfile, then a fallback code is
> used. However while the normal code uses a restrictive umask (or the
> mode of the existing viminfo), the fallback code uses the default umask
> of the user. This can result in a viminfo which is readable by all
> users possibly leaking sensitive information.
> ---
>  src/ex_cmds.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> Hello,
> 
> We noticed this issue on a multi-user system where viminfos
> became readable by other users from time to time. We think this
> issue is caused by the fallback code in write_viminfo() which
> doesn't enforce a strict umask. This patch should fix this issue.

Thanks.  The temp file is safe, since only the user can read the temp
directory, but since it's move to the right place with a rename.

Can you write a test for this?
 
> However this patch won't help users which were already affected
> by this race condition as write_viminfo() retains the permissions
> of an existing viminfo. As the use-case for a viminfo readable by
> others doesn't seem very relevant to me, I recommend changing the
> viminfo code to always enforce 0600 permissions to prevent this
> kind of information leak (for affected users and in general).

I wonder if this would ever cause problems.  I can't think of something.

-- 
Mushrooms always grow in damp places and so they look like umbrellas.

 /// Bram Moolenaar -- b...@moolenaar.net -- 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 vim_dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Possible security issue: [PATCH] viminfo: create fallback tempfile with restrictive umask

2017-11-07 Thread Simon Ruderich
On Tue, Nov 07, 2017 at 10:21:12PM +0100, Bram Moolenaar wrote:
> Thanks.  The temp file is safe, since only the user can read the temp
> directory, but since it's move to the right place with a rename.

Hello,

I don't understand what you're saying here. Do you mean the
creation of the temp file in the fallback path is safe because
it's created in the vim temp directory which is only readable by
the user? But when it's renamed outside it's an issue (as
described above in the patch) because then the file which is
readable by all users becomes visible?

> Can you write a test for this?

I don't know how. It's a race condition which is difficult to
reproduce (file must not exist during mch_stat, but exist during
mch_open).

>> However this patch won't help users which were already affected
>> by this race condition as write_viminfo() retains the permissions
>> of an existing viminfo. As the use-case for a viminfo readable by
>> others doesn't seem very relevant to me, I recommend changing the
>> viminfo code to always enforce 0600 permissions to prevent this
>> kind of information leak (for affected users and in general).
>
> I wonder if this would ever cause problems.  I can't think of something.

Sounds good, I'll attach a second patch which will change the
behavior and enforce 0600.

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 vim_dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[PATCH] viminfo: always enforce viminfo with mode 0600

2017-11-07 Thread Simon Ruderich
Making viminfo readable by other users is most likely not useful. To
prevent information leakage enforce mode 0600.

The race condition fixed in the previous patch could also cause
viminfo files readable by other uses. Enforcing mode 0600
restores the originally indented permissions.
---
 src/ex_cmds.c | 14 +-
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/src/ex_cmds.c b/src/ex_cmds.c
index cc50409cf..c250a784f 100644
--- a/src/ex_cmds.c
+++ b/src/ex_cmds.c
@@ -1984,21 +1984,9 @@ write_viminfo(char_u *file, int forceit)
 #else
int fd;
 
-   /* Use mch_open() to be able to use O_NOFOLLOW and set file
-* protection:
-* Unix: same as original file, but strip s-bit.  Reset umask to
-* avoid it getting in the way.
-* Others: r&w for user only. */
-# ifdef UNIX
-   umask_save = umask(0);
-   fd = mch_open((char *)tempname,
-   O_CREAT|O_EXTRA|O_EXCL|O_WRONLY|O_NOFOLLOW,
-  (int)((st_old.st_mode & 0777) | 0600));
-   (void)umask(umask_save);
-# else
+   /* Use mch_open() to be able to use O_NOFOLLOW. */
fd = mch_open((char *)tempname,
O_CREAT|O_EXTRA|O_EXCL|O_WRONLY|O_NOFOLLOW, 0600);
-# endif
if (fd < 0)
fp_out = NULL;
else
-- 
2.15.0

-- 
+ 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 vim_dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.