On Wed, Jul 16, 2008 at 9:43 PM, Bram Moolenaar <[EMAIL PROTECTED]> wrote:
>
> Jan Minar wrote:
>
>> On Wed, Jul 16, 2008 at 3:54 PM, Bram Moolenaar <[EMAIL PROTECTED]> wrote:
>> > I'm glad to see you persist in finding more problems.
>>
>> I'm sure everybody is...
>>
>> >> 2. Background
>> >>
>> >> The shellescape() function, added by patch 7.0.111, has since been
>> >> modified in 7.2a.013 to escape special characters, so as to be useful
>> >> when sanitizing arguments of the ``execute'' command:
>> >>
>> >>
>> >> ``shellescape({string} [, {special}])
>> >>     Escape {string} for use as shell command argument.
>> >>     [...]
>> >>     When the {special} argument is present and it's a non-zero Number or
>> >>     a non-empty String [...], then special items such as "%", "#" and
>> >>     "<cword>" will be preceded by a backslash.  This backslash will be
>> >>     removed again by the :! command.  Example of use with a :! command:
>> >>         :exe '!dir ' . shellescape(expand('<cfile>'), 1)
>> >>     This results in a directory listing for the file under the cursor.''
>> >>
>> >>       -- Vim Reference Manual (``eval.txt'')
>> >>
>> >>
>> >> 3. Vulnerability
>> >>
>> >> shellescape() does not escape all special items.  In particular,
>> >> shellescape() does not escape the ``!'' character.
>> >>
>> >> The Vim documentation lacks a comprehensive explicit list of special
>> >> items.  This might have been the reason why patch 7.2a.013 failed to
>> >> acknowledge ``!'' as a special item.
>> >
>> > The "!" character is a special character in another way.  It's replaced
>> > in a different location.
>> >
>> > While looking into this I noticed that csh and tcsh also handle "!" as a
>> > special character, even within a single quoted string.  So it has to be
>> > escaped another time for these shells.
>> >
>> > It appears another character that needs to be escape is NL.  I'll try
>> > that out.  File names with an embedded newline are quite difficult to
>> > handle anyway.
>> >
>> > Please let me know if you suspect any other character needs escaping.
>>
>> It would really help if we had all the ``special items'' documented,
>> in one place, so people knew what to expect, how to sanitize, and what
>> can go wrong.  Something akin to the bash(1) man page talking about
>> various kinds of expansion -- this is what the man page says:
>>
>> ``Expansion is performed on the command line after it has been split
>> into words. There are seven kinds of expansion performed: brace
>> expansion, tilde expansion, parameter and variable expansion, command
>> substitution, arithmetic expansion, word splitting, and pathname
>> expansion.
>>
>> ``The order of expansions is: brace expansion, tilde expansion,
>> parameter, variable and arithmetic expansion and command substitution
>> (done in a left-to-right fashion), word splitting, and pathname
>> expansion.''
>>
>> And then it goes on, talking about the various types of expansion, in
>> the order they are performed.
>>
>> That would be helpful to have.
>>
>> Now with bash, all we have to do is enclose command arguments in
>> quotes, and interpose the double-dash ``--'' in between the command
>> line switches and the rest of the positional arguments.  Simple, easy,
>> provably right.  Now with Vim, it is all but easy, and so far there
>> always been a way to trick the quoting.  If we had a description of
>> how the evaluation of the command line works wrt ``special items'', at
>> least we could tell whether the sanitization is done correctly.
>>
>> Is somebody up to the task?
>
> The vi POSIX documentation has a try at this, but it's a big puzzle.
> And still wrong.

What I'd really like to see is, with every command and function,
starting with ``execute'' and system(), an exhaustive documentation of
their behaviour wrt what in their arguments may be interpreted as a
special item, instead of a literal character.  This would be part of
the :help documentation for every respective function and command.
The documentation is mostly there, but it's the bits that are missing
that are a trouble.

> There are too many exceptions, I'm afraid.  And that the value of
> 'shell' matters doesn't make it simpler.  We can't document what all the
> shells do with their arguments when explaining how system() works.

There are 80 calls to system() throughout the runtime; the following
table details counts per script file in Vim 7.2b:

35 autoload/netrw.vim
16 autoload/tar.vim
 7 autoload/gzip.vim
 5 autoload/zip.vim
 3 ftplugin/ruby.vim
 3 autoload/vimball.vim
 2 ftplugin/perl.vim
 2 ftplugin/man.vim
 2 ftplugin/gitcommit.vim
 2 ftplugin/changelog.vim
 1 ftplugin/debchangelog.vim
 1 autoload/phpcomplete.vim

Most of those calls don't use any features such as globbing -- if
system() is inherently insecure, what would you think about replacing
it with, say, a function with execve() semantics?

Cheers,
Jan Minar.

--~--~---------~--~----~------------~-------~--~----~
You received this message from the "vim_dev" maillist.
For more information, visit http://www.vim.org/maillist.php
-~----------~----~----~----~------~----~------~--~---

Raspunde prin e-mail lui