Bug with v_K and potentially K command

110 views
Skip to first unread message

Ben Schmidt

unread,
Aug 19, 2008, 11:38:27 PM8/19/08
to vim...@googlegroups.com, Bram Moolenaar
Ex command substitutions (:help cmdline-special) seem to be done on the keyword
when using the K command. Due to normal settings for iskeyword this won't usually
show up for K, but will for {Visual}K if you, e.g., highlight a URL with a # in it
and use K on it (with keywordprg set to 'firefox' or something--'open' for me on
Mac OS X--this makes sense: in fact it is very useful). If there is no alternate
file you get an error in Vim, but if there is one, nonsense just gets passed to
the shell. It should be checked that the keyword is properly shell-escaped, too. I
can't quickly think of a way to easily exploit this one, so I don't think it's a
security risk, but it's definitely a bug.

Ben.


Jan Minář

unread,
Aug 20, 2008, 12:51:54 AM8/20/08
to vim...@googlegroups.com
Hi, Ben!

Thanks for pointing this out.

On Wed, Aug 20, 2008 at 4:38 AM, Ben Schmidt
<mail_ben...@yahoo.com.au> wrote:
> the shell. It should be checked that the keyword is properly shell-escaped, too. I
> can't quickly think of a way to easily exploit this one, so I don't think it's a
> security risk, but it's definitely a bug.

This is very much a security bug. One that allows arbitrary shell
commands execution.

Your usage is a very good example, because one in general doesn't
control the URL of the object one wants to access. According to my
reading of RFC 3986, valid URI characters, on top of letters and
numbers, are:

:,?#[]@!$&'()*+;=._~/%

Opening the following URL using the K command will launch the
xclock(1x) program:

http://www.google.co.uk/search?q=&xclock&

But, of course, it's much worse: Since the URL is inside a buffer, we
can assume the whole of the buffer can be controlled by the attacker.
They can use a modeline to set 'iskeyword' to contain any characters
needed for a particular shell command:

/* We use an obscure glibc function -- check out the man page! */
clockface = (xclock&)pwnme(a, b, x + y);
[...]
/* vim:iskeyword:a-z,&,),(: */

Cheers,
Jan.

Bram Moolenaar

unread,
Aug 20, 2008, 1:14:16 AM8/20/08
to Ben Schmidt, vim...@googlegroups.com

Ben Schmidt wrote:

Currently some characters are escaped, such as a space. I suppose # and
% should also be escaped. More general, it should probably work like
shellescape(). I'll make a note in the todo list.

--
Scientists decoded the first message from an alien civilization:
SIMPLY SEND 6 TIMES 10 TO THE 50 ATOMS OF HYDROGEN TO THE STAR
SYSTEM AT THE TOP OF THE LIST, CROSS OFF THAT STAR SYSTEM, THEN PUT
YOUR STAR SYSTEM AT THE BOTTOM OF THE LIST AND SEND IT TO 100 OTHER
STAR SYSTEMS. WITHIN ONE TENTH GALACTIC ROTATION YOU WILL RECEIVE
ENOUGH HYDROGREN TO POWER YOUR CIVILIZATION UNTIL ENTROPY REACHES ITS
MAXIMUM! IT REALLY WORKS!

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ download, build and distribute -- http://www.A-A-P.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

Ben Schmidt

unread,
Aug 20, 2008, 1:17:42 AM8/20/08
to vim...@googlegroups.com
Jan Minář wrote:
> On Wed, Aug 20, 2008 at 4:38 AM, Ben Schmidt
> <mail_ben...@yahoo.com.au> wrote:
>> the shell. It should be checked that the keyword is properly
>> shell-escaped, too. I can't quickly think of a way to easily exploit
>> this one, so I don't think it's a security risk, but it's definitely
>> a bug.
>
> This is very much a security bug. One that allows arbitrary shell
> commands execution.

Without being properly shell-escaped, yes. I hadn't tested that aspect,
and in my mind, my security comment didn't refer to it, but assumed
escaping was done.

In the actual email, the security comment does seem to refer to the
shell escaping...

And in real life it seems the keyword isn't fully escaped, though it
partly is. Wrong type(s) of escaping perhaps.

Ouch.

Ben.


Tony Mechelynck

unread,
Aug 20, 2008, 3:18:27 AM8/20/08
to vim...@googlegroups.com
On 20/08/08 06:51, Jan Minář wrote:
[...]

> Opening the following URL using the K command will launch the
> xclock(1x) program:
>
> http://www.google.co.uk/search?q=&xclock&

Pasting this into the SeaMonkey location bar opens a Google page.
Hitting K on it in gvim with 'keywordprg' set to "seamonkey" invokes
":!seamonkey http" which gives a page from the site http://www.http.com/
In neither case do I see any xclock process, even though the program is
in my $PATH.

>
> But, of course, it's much worse: Since the URL is inside a buffer, we
> can assume the whole of the buffer can be controlled by the attacker.
> They can use a modeline to set 'iskeyword' to contain any characters
> needed for a particular shell command:
>
> /* We use an obscure glibc function -- check out the man page! */
> clockface = (xclock&)pwnme(a, b, x + y);
> [...]
> /* vim:iskeyword:a-z,&,),(: */

Error detected while processing modelines
line 4:
E518: Unknown option: a-z,&,),(


Hitting K on line 2 (with 'keywordprg' still set to "seamonkey") gives
":!seamonkey clockface" which for some reason loads the page
http://www.apple.com/

>
> Cheers,
> Jan.

Best regards,
Tony.
--
Dear Miss Manners:
My home economics teacher says that one must never place one's
elbows on the table. However, I have read that one elbow, in between
courses, is all right. Which is correct?

Gentle Reader:
For the purpose of answering examinations in your home
economics class, your teacher is correct. Catching on to this
principle of education may be of even greater importance to you now
than learning correct current table manners, vital as Miss Manners
believes that is.

Jan Minář

unread,
Aug 20, 2008, 3:47:14 AM8/20/08
to vim...@googlegroups.com
On Wed, Aug 20, 2008 at 8:18 AM, Tony Mechelynck
<antoine.m...@gmail.com> wrote:
>
> On 20/08/08 06:51, Jan Minář wrote:
> [...]
>> Opening the following URL using the K command will launch the
>> xclock(1x) program:
>>
>> http://www.google.co.uk/search?q=&xclock&
>
> Pasting this into the SeaMonkey location bar opens a Google page.
> Hitting K on it in gvim with 'keywordprg' set to "seamonkey" invokes
> ":!seamonkey http" which gives a page from the site http://www.http.com/
> In neither case do I see any xclock process, even though the program is
> in my $PATH.

You need to have 'iskeyword' set to a sensible value. Sensible, that
is, for handling URLs. A version using a modeline to set the
'iskeyword':

http://www.example.com&xclock&
vim: iskeyword=58,?,#,[,],@,!,$,&,',(,),*,+,44,;,=,45,.,_,~,/,48-57,A-Z,a-z,%


>> But, of course, it's much worse: Since the URL is inside a buffer, we
>> can assume the whole of the buffer can be controlled by the attacker.
>> They can use a modeline to set 'iskeyword' to contain any characters
>> needed for a particular shell command:
>>
>> /* We use an obscure glibc function -- check out the man page! */
>> clockface = (xclock&)pwnme(a, b, x + y);
>> [...]
>> /* vim:iskeyword:a-z,&,),(: */

The above will of course not work. The following will:

/* We use an obscure glibc function -- check out the man page! */

clockface = &(xclock)&pwnme (a, b, x + y);
/* :vim:iskeyword=a-z,&,),(: */

Cheers,
Jan.

Tony Mechelynck

unread,
Aug 20, 2008, 4:33:40 AM8/20/08
to vim...@googlegroups.com
On 20/08/08 09:47, Jan Minář wrote:
> On Wed, Aug 20, 2008 at 8:18 AM, Tony Mechelynck
> <antoine.m...@gmail.com> wrote:
>> On 20/08/08 06:51, Jan Minář wrote:
>> [...]
>>> Opening the following URL using the K command will launch the
>>> xclock(1x) program:
>>>
>>> http://www.google.co.uk/search?q=&xclock&
>> Pasting this into the SeaMonkey location bar opens a Google page.
>> Hitting K on it in gvim with 'keywordprg' set to "seamonkey" invokes
>> ":!seamonkey http" which gives a page from the site http://www.http.com/
>> In neither case do I see any xclock process, even though the program is
>> in my $PATH.
>
> You need to have 'iskeyword' set to a sensible value. Sensible, that
> is, for handling URLs. A version using a modeline to set the
> 'iskeyword':
>
> http://www.example.com&xclock&
> vim: iskeyword=58,?,#,[,],@,!,$,&,',(,),*,+,44,;,=,45,.,_,~,/,48-57,A-Z,a-z,%

This time Vim says ":!seamonkey www.example.com&xclock& which apparently
doesn't do anything. Pasting the URL into the Location bar gives

Address Not Found

www.example.com&xclock& could not be found. Please check the name and
try again.

The browser could not find the host server for the provided address.

* Did you make a mistake when typing the domain? (e.g.
"ww.mozilla.org" instead of "www.mozilla.org")
* Are you certain this domain address exists? Its registration may
have expired.
* Are you unable to browse other sites? Check your network
connection and DNS server settings.
* Is your computer or network protected by a firewall or proxy?
Incorrect settings can interfere with Web browsing.

[ Try Again ]

"ps -lC xclock" still doesn't show anything.


>
>
>>> But, of course, it's much worse: Since the URL is inside a buffer, we
>>> can assume the whole of the buffer can be controlled by the attacker.
>>> They can use a modeline to set 'iskeyword' to contain any characters
>>> needed for a particular shell command:
>>>
>>> /* We use an obscure glibc function -- check out the man page! */
>>> clockface = (xclock&)pwnme(a, b, x + y);
>>> [...]
>>> /* vim:iskeyword:a-z,&,),(: */
>
> The above will of course not work. The following will:
>
> /* We use an obscure glibc function -- check out the man page! */

> clockface =&(xclock)&pwnme (a, b, x + y);
> /* :vim:iskeyword=a-z,&,),(: */

No error this time, but still says ":!seamonkey clockface" and loads
http://www.apple.com/

>
> Cheers,
> Jan.

Well, I couldn't reproduce your exploit with the Mozilla SeaMonkey
2.0a1pre browser. You can see its UA string in the headers of this post.


Best regards,
Tony.
--
God is Dead
-- Nietzsche
Nietzsche is Dead
-- God
Nietzsche is God
-- The Dead

Matt Wozniski

unread,
Aug 20, 2008, 7:09:24 AM8/20/08
to vim...@googlegroups.com

Jan got the exploit right, but formatted his modeline wrong. Try this document:


/* We use an obscure glibc function -- check out the man page! */

clockface = &(xclock)&pwnme (a, b, x + y);
/* vim: set iskeyword=a-z,&,),(: */

Make sure ":verbose set isk?" correctly says
iskeyword=a-z,&,),(
Last set from modeline

place your cursor on 'pwnme', and press K. xclock appears.

> Well, I couldn't reproduce your exploit with the Mozilla SeaMonkey
> 2.0a1pre browser. You can see its UA string in the headers of this post.

The browser being used has nothing to do with the exploit; it's all in
the shell expansions before the browser is launched. In fact, in
cases like this I don't think we should be using the shell at all, for
reasons just like this one. I can see no real argument for why K
ought to behave like:
exe '!' . &kp . ' ' . expand("<cword>")

Is there any reason why we would ever want shell syntax to affect
keyword lookups? I think that K ought to behave more like
execlp(&kp, &kp, expand("<cword>"), (char *)NULL);
Of course, this is muddled C and Vimscript pseudo-code, but you get the idea.

OTOH, :! probably ought to continue using the shell so that you can
do, for instance,
:!ls | grep foo

> Best regards,
> Tony.

~Matt

Ben Schmidt

unread,
Aug 20, 2008, 7:32:42 AM8/20/08
to vim...@googlegroups.com
> This time Vim says ":!seamonkey www.example.com&xclock& which apparently
> doesn't do anything. Pasting the URL into the Location bar gives

I bet you're in gvim, right, Tony? I expect you're encountering the
"can't run processes in the background" problem that a number of users
have commented on recently, and for that reason not seeing the exploit.
In console Vim you will see it, or if you just do

www.example.com&xclock

which will wait for xclock to exit (close it!) before returning to Vim,
rather than deallocating the pty as soon as control returns to vim, thus
killing all the processes started by the command (both the browser and
the clock--though if you already have Seamonkey open, the new process
will just send it a message and exit, I believe, much like gvim
--remote).

Ben.


Bram Moolenaar

unread,
Aug 21, 2008, 12:55:06 AM8/21/08
to Matt Wozniski, vim...@googlegroups.com

Matt Wozniski wrote:

> On Wed, Aug 20, 2008 at 4:33 AM, Tony Mechelynck wrote:
> >
> > On 20/08/08 09:47, Jan Minář wrote:
> >>
> >> The above will of course not work. The following will:
> >>
> >> /* We use an obscure glibc function -- check out the man page! */
> >> clockface =&(xclock)&pwnme (a, b, x + y);
> >> /* :vim:iskeyword=a-z,&,),(: */
> >
> > No error this time, but still says ":!seamonkey clockface" and loads
> > http://www.apple.com/
>
> Jan got the exploit right, but formatted his modeline wrong. Try this document:
> /* We use an obscure glibc function -- check out the man page! */
> clockface = &(xclock)&pwnme (a, b, x + y);
> /* vim: set iskeyword=a-z,&,),(: */
>
> Make sure ":verbose set isk?" correctly says
> iskeyword=a-z,&,),(
> Last set from modeline
>
> place your cursor on 'pwnme', and press K. xclock appears.

Yeah, this is the kind of exploit where you have to tell the user to do
something stupid and them blame Vim that the user is stupid.

> > Well, I couldn't reproduce your exploit with the Mozilla SeaMonkey
> > 2.0a1pre browser. You can see its UA string in the headers of this post.
>
> The browser being used has nothing to do with the exploit; it's all in
> the shell expansions before the browser is launched. In fact, in
> cases like this I don't think we should be using the shell at all, for
> reasons just like this one. I can see no real argument for why K
> ought to behave like:
> exe '!' . &kp . ' ' . expand("<cword>")
>
> Is there any reason why we would ever want shell syntax to affect
> keyword lookups? I think that K ought to behave more like
> execlp(&kp, &kp, expand("<cword>"), (char *)NULL);
> Of course, this is muddled C and Vimscript pseudo-code, but you get the idea.
>
> OTOH, :! probably ought to continue using the shell so that you can
> do, for instance,
> :!ls | grep foo

The command executed can be an shell alias. The command may not be in
$PATH. And a few other reasons we don't see right now.

--
I once paid $12 to peer at the box that held King Tutankhamen's little
bandage-covered midget corpse at the De Young Museum in San Francisco. I
remember thinking how pleased he'd be about the way things turned out in his
afterlife.
(Scott Adams - The Dilbert principle)

Matt Wozniski

unread,
Aug 21, 2008, 1:18:44 AM8/21/08
to vim...@googlegroups.com
On Thu, Aug 21, 2008 at 12:55 AM, Bram Moolenaar <Br...@moolenaar.net> wrote:

>
> Matt Wozniski wrote:
>
>> Jan got the exploit right, but formatted his modeline wrong. Try this document:
>> /* We use an obscure glibc function -- check out the man page! */
>> clockface = &(xclock)&pwnme (a, b, x + y);
>> /* vim: set iskeyword=a-z,&,),(: */
>>
>> Make sure ":verbose set isk?" correctly says
>> iskeyword=a-z,&,),(
>> Last set from modeline
>>
>> place your cursor on 'pwnme', and press K. xclock appears.
>
> Yeah, this is the kind of exploit where you have to tell the user to do
> something stupid and them blame Vim that the user is stupid.

Yeah, not a terribly serious exploit, more of an inconvenience when a
command does something other than you'd expect because of shell
expansion.

>> The browser being used has nothing to do with the exploit; it's all in
>> the shell expansions before the browser is launched. In fact, in
>> cases like this I don't think we should be using the shell at all, for
>> reasons just like this one. I can see no real argument for why K
>> ought to behave like:
>> exe '!' . &kp . ' ' . expand("<cword>")
>>
>> Is there any reason why we would ever want shell syntax to affect
>> keyword lookups? I think that K ought to behave more like
>> execlp(&kp, &kp, expand("<cword>"), (char *)NULL);
>> Of course, this is muddled C and Vimscript pseudo-code, but you get the idea.
>>
>> OTOH, :! probably ought to continue using the shell so that you can
>> do, for instance,
>> :!ls | grep foo
>
> The command executed can be an shell alias. The command may not be in
> $PATH. And a few other reasons we don't see right now.

execlp() handles commands not in $PATH just fine, as long as its first
parameter is an absolute or relative path, rather than just a
filename. And a shell alias being used there is definitely not a good
thing - and is not done by default. If you want to replace or create
a program, you create a script, if you want to change the way a
program is used interactively, you use an alias. Aliases are
certainly not designed to ever be called from an external program, and
forcing bash to pretend to be an interactive shell when it's not, in
fact, being used interactively, just for the sake of expanding
aliases, is a Bad Thing.

~Matt

Ben Schmidt

unread,
Aug 21, 2008, 1:56:53 AM8/21/08
to vim...@googlegroups.com
>>> place your cursor on 'pwnme', and press K. xclock appears.
>> Yeah, this is the kind of exploit where you have to tell the user to do
>> something stupid and them blame Vim that the user is stupid.

Yes. Still...that seems to be the current trend...

>> The command executed can be an shell alias. The command may not be in
>> $PATH. And a few other reasons we don't see right now.
>
> execlp() handles commands not in $PATH just fine, as long as its first
> parameter is an absolute or relative path, rather than just a
> filename. And a shell alias being used there is definitely not a good
> thing - and is not done by default. If you want to replace or create
> a program, you create a script, if you want to change the way a
> program is used interactively, you use an alias. Aliases are
> certainly not designed to ever be called from an external program, and
> forcing bash to pretend to be an interactive shell when it's not, in
> fact, being used interactively, just for the sake of expanding
> aliases, is a Bad Thing.

I agree aliases should be interactive, and not used here. However, $PATH
and so on is often set/adjusted from shell startup files, and
particularly when using gvim started via GUI, etc., this can be relevant.

On the other side, shell escaping when exactly which shell is being used
is not known is a nightmare, too.

I think it's one of those issues where there are so many good arguments
on both sides, it comes down to a matter of opinion.

Ben.


Matt Wozniski

unread,
Aug 21, 2008, 2:25:44 AM8/21/08
to vim...@googlegroups.com
On Thu, Aug 21, 2008 at 1:56 AM, Ben Schmidt wrote:
>
>>>> place your cursor on 'pwnme', and press K. xclock appears.
>>> Yeah, this is the kind of exploit where you have to tell the user to do
>>> something stupid and them blame Vim that the user is stupid.
>
> Yes. Still...that seems to be the current trend...
>
>>> The command executed can be an shell alias. The command may not be in
>>> $PATH. And a few other reasons we don't see right now.
>>
>> execlp() handles commands not in $PATH just fine, as long as its first
>> parameter is an absolute or relative path, rather than just a
>> filename. And a shell alias being used there is definitely not a good
>> thing - and is not done by default. If you want to replace or create
>> a program, you create a script, if you want to change the way a
>> program is used interactively, you use an alias. Aliases are
>> certainly not designed to ever be called from an external program, and
>> forcing bash to pretend to be an interactive shell when it's not, in
>> fact, being used interactively, just for the sake of expanding
>> aliases, is a Bad Thing.
>
> I agree aliases should be interactive, and not used here. However, $PATH
> and so on is often set/adjusted from shell startup files, and
> particularly when using gvim started via GUI, etc., this can be relevant.

PATH should be set from .profile, not from .bashrc - Interactive vs
noninteractive should have nothing to do with whether or not PATH is
correct; either your x session or your login shell at a tty should set
up such things.

> On the other side, shell escaping when exactly which shell is being used
> is not known is a nightmare, too.

And it's here that I'm convinced we'll be able to find a ton of
security vulnerabilities as soon as someone with some security
expertise decides to look.

> I think it's one of those issues where there are so many good arguments
> on both sides, it comes down to a matter of opinion.
>
> Ben.

In that vein, perhaps using the shell should be an option... but
doubtless the best default behavior is to use system(3) for places
like :! where shell expansion is good,and execlp() for those places
where we decidedly don't want any shell expansion. Relying on uniform
escaping required for /bin/sh is still not terribly easy, but it's a
much less moving target than escaping for every possible shell... It
just strikes me that vim's present use of the user's shell is
inherently impossible to sanitize, and thus inherently insecure for at
least the simplistic "file name has a command embedded" sort of
attack...

~Matt

Tony Mechelynck

unread,
Aug 21, 2008, 12:35:23 PM8/21/08
to vim...@googlegroups.com
On 20/08/08 13:09, Matt Wozniski wrote:
> On Wed, Aug 20, 2008 at 4:33 AM, Tony Mechelynck wrote:
>> On 20/08/08 09:47, Jan Minář wrote:
>>> The above will of course not work. The following will:
>>>
>>> /* We use an obscure glibc function -- check out the man page! */
>>> clockface =&(xclock)&pwnme (a, b, x + y);
>>> /* :vim:iskeyword=a-z,&,),(: */
>> No error this time, but still says ":!seamonkey clockface" and loads
>> http://www.apple.com/
>
> Jan got the exploit right, but formatted his modeline wrong. Try this document:
> /* We use an obscure glibc function -- check out the man page! */
> clockface = &(xclock)&pwnme (a, b, x + y);
> /* vim: set iskeyword=a-z,&,),(: */
>
> Make sure ":verbose set isk?" correctly says
> iskeyword=a-z,&,),(
> Last set from modeline

yes, that's what it says

>
> place your cursor on 'pwnme', and press K. xclock appears.

[...]

Ah, yes, this time I see a clock but I can't give it focus, even by
clicking its titlebar; and at the bottom of the gvim window I see

:! seamonkey &(xclock)&pwnme
/bin/bash: pwnme: command not found

shell returned 127


But you got to have the cursor "near enough" to where the "exploiter"
wants it -- previously I put it on clockface and it didn't work -- and
then the user has to manually hit K. Looks to me like the so-called
"exploit" requires quite some cooperation by the user.

Best regards,
Tony.
--
Love's Drug

My love is like an iron wand
That conks me on the head,
My love is like the valium
That I take before my bed,
My love is like the pint of scotch
That I drink when I be dry;
And I shall love thee still, my dear,
Until my wife is wise.

Tony Mechelynck

unread,
Aug 21, 2008, 1:05:22 PM8/21/08
to vim...@googlegroups.com
On 21/08/08 08:25, Matt Wozniski wrote:
[...]

> In that vein, perhaps using the shell should be an option... but
> doubtless the best default behavior is to use system(3) for places
> like :! where shell expansion is good,and execlp() for those places
> where we decidedly don't want any shell expansion. Relying on uniform
> escaping required for /bin/sh is still not terribly easy, but it's a
> much less moving target than escaping for every possible shell... It
> just strikes me that vim's present use of the user's shell is
> inherently impossible to sanitize, and thus inherently insecure for at
> least the simplistic "file name has a command embedded" sort of
> attack...
>
> ~Matt

From man 3 execlp

[...]
> The exec() family of functions replaces the current process image with
> a new process image. [...]
>
> If any of the exec() functions returns, an error will have occurred.
> The return value is -1, and the global variable errno will be set to
> indicate the error.
[...]

Are you sure that where we used to call the shell, you want to replace
the whole Vim process by something else unless an error occurs?


Best regards,
Tony.
--
Character Density, n.:
The number of very weird people in the office.

Matt Wozniski

unread,
Aug 21, 2008, 1:09:49 PM8/21/08
to vim...@googlegroups.com

Well, after a vfork(), of course!

~Matt

Jan Minář

unread,
Aug 22, 2008, 7:37:19 AM8/22/08
to vim...@googlegroups.com
Hi!

Thanks again to Ben for reporting this.

It's not just the K command. The <C-]> and g] commands are vulnerable
too. Patch attached.

Attack vectors:

(1) K -- arbitrary shell command execution via additional shell
commands (insufficient sanitization of a shell command string)
(the original vulnerability)

(2) K -- arbitrary shell command execution via man(1) command line
switches (such as ``--pager'' in GNU man -- cf. manpage)

(3) <C-]>, g] -- arbitrary Vim Script command execution via additional
Ex statements (insufficient escaping of an argument)

(4) Unknown vulnerabilities stemming from using unknown shell, and,
by extension, an unknown man command

This patch solves (1) and (3), and partially solves (2). Unfortunately,
the fix for (2) is a hardcoded double-dash (--) inserted between the
program name and the command line arguments. This will break for man
commands that do not understand double-dash. A more clever solution is
needed.

The discussion of (the feasibility of) a fix for (4) has been going on
for some time. All proposed solutions seem to have irreconcilable
downsides.

Cheers,
Jan.

K-arbitrary-command-execution.patch

Tony Mechelynck

unread,
Aug 22, 2008, 10:34:57 AM8/22/08
to vim...@googlegroups.com

Maybe you should set a config-time option (or create one) to avoid any
interaction with the shell?

Even better: If you don't want ever to become the victim of any exploit,
turn your computer off at the wall switch and leave it off.

:-b

Regards,
Tony.
--
Bore, n.:
A person who talks when you wish him to listen.
-- Ambrose Bierce, "The Devil's Dictionary"

Bram Moolenaar

unread,
Aug 22, 2008, 2:25:09 PM8/22/08
to Jan Minář, vim...@googlegroups.com

Jan Minar wrote:

Thanks. I'll have a good look at it later. One thing I noticed: you
don't need to give an error message for running out of memory at this
level, it's already done at a lower level in alloc(). There it also
avoids that the message fills up your screen.

--
Sometimes you can protect millions of dollars in your budget simply by buying
a bag of cookies, dropping it on the budget anylyst's desk, and saying
something deeply personal such as "How was your weekend, big guy?"

Jan Minář

unread,
Aug 22, 2008, 3:20:28 PM8/22/08
to Bram Moolenaar, Vim Developers
> Thanks. I'll have a good look at it later. One thing I noticed: you
> don't need to give an error message for running out of memory at this
> level, it's already done at a lower level in alloc(). There it also

That's what I thought, too. But vim_strsave_shellescape() is
documented to return NULL when out of memory:

src/misc2.c:

1279 * Returns the result in allocated memory, NULL if we have run out.
1280 */
1281 char_u *
1282 vim_strsave_shellescape(string, do_special)

Cheers,
Jan.

Bram Moolenaar

unread,
Aug 22, 2008, 5:25:02 PM8/22/08
to Jan Minář, Vim Developers

Jan Minar wrote:

Yeah, but the out-of-memory message will already been given. So you can
just abort the operation.

--
Article in the first Free Software Magazine: "Bram Moolenaar studied electrical
engineering at the Technical University of Delft and graduated in 1985 on a
multi-processor Unix architecture."
Response by "dimator": Could the school not afford a proper stage for the
ceremony?

Tony Mechelynck

unread,
Aug 22, 2008, 6:41:25 PM8/22/08
to vim...@googlegroups.com, Bram Moolenaar

Well, if you have run out of memory, it's probably too late to allocate
a new memory block in order to return an error message, isn't it?

Best regards,
Tony.
--
The end of the human race will be that it will eventually die of
civilization.
-- Ralph Waldo Emerson

Matt Wozniski

unread,
Aug 22, 2008, 6:54:42 PM8/22/08
to vim...@googlegroups.com
On Fri, Aug 22, 2008 at 6:41 PM, Tony Mechelynck wrote:
>
> On 22/08/08 21:20, Jan Minář wrote:
>>> Thanks. I'll have a good look at it later. One thing I noticed: you
>>> don't need to give an error message for running out of memory at this
>>> level, it's already done at a lower level in alloc(). There it also
>>
>> That's what I thought, too. But vim_strsave_shellescape() is
>> documented to return NULL when out of memory:
>>
>> src/misc2.c:
>>
>> 1279 * Returns the result in allocated memory, NULL if we have run out.
>> 1280 */
>> 1281 char_u *
>> 1282 vim_strsave_shellescape(string, do_special)
>>
>> Cheers,
>> Jan.
>
> Well, if you have run out of memory, it's probably too late to allocate
> a new memory block in order to return an error message, isn't it?
>
> Best regards,
> Tony.

Yes, but... in the case of the line in question:
printf("ERROR: out of memory\n");
Jan wasn't trying to dynamically allocate more memory from the heap;
he was printing out a string that was already allocated by the OS
before vim even reached its main() function. In C, unlike in some
interpreted language like, IIRC, Python, all string literals are
loaded into memory before the program gets control, and any reference
to them is just a matter of dereferencing a pointer.

~Matt

John Beckett

unread,
Aug 22, 2008, 7:58:49 PM8/22/08
to vim...@googlegroups.com
Tony Mechelynck wrote:
> Maybe you should set a config-time option (or create one) to
> avoid any interaction with the shell?
>
> Even better: If you don't want ever to become the victim of
> any exploit, turn your computer off at the wall switch and
> leave it off.
>
> :-b

I haven't studied this example, but as I understand it, the suggestion is that I
could send you a file with a message like "What's the deal with this weird message
that Vim gives? Open file xxx and search for yyy then press K."

Jan is saying (I think) that following those instructions could execute malware.

Sure, it will never happen to me or you, but if we were discussing Microsoft Word,
most people would have no hesitation in declaring that such a vulnerability (press a
key in a document to get owned) is just NOT acceptable.

We aren't talking about mapping K to execute "system('dodgyfile')". K is performing
its default function, but that function could exploit you if executed on certain
text, with a certain file present.

If my understanding is correct, I don't think it's reasonable to write this off with
the "switch power off" joke (if I've got this wrong, please correct me).

Once again, thanks Jan!

John

Jan Minář

unread,
Aug 23, 2008, 2:24:59 AM8/23/08
to Bram Moolenaar, Vim Developers
On Fri, Aug 22, 2008 at 10:25 PM, Bram Moolenaar <Br...@moolenaar.net> wrote:
>
> Jan Minar wrote:
>
>> > Thanks. I'll have a good look at it later. One thing I noticed: you
>> > don't need to give an error message for running out of memory at this
>> > level, it's already done at a lower level in alloc(). There it also
>>
>> That's what I thought, too. But vim_strsave_shellescape() is
>> documented to return NULL when out of memory:
>>
>> src/misc2.c:
>>
>> 1279 * Returns the result in allocated memory, NULL if we have run out.
>> 1280 */
>> 1281 char_u *
>> 1282 vim_strsave_shellescape(string, do_special)
>
> Yeah, but the out-of-memory message will already been given. So you can
> just abort the operation.

Thanks for the feedback.

My understanding of how vim malloc() wrappers worked was incorrect.
I've updated the patch. These are the changes from the previous
version:

--- - 2008-08-23 07:10:35.931412125 +0100
+++ src/normal.c 2008-08-23 07:07:44.000000000 +0100
@@ -5518,12 +5518,10 @@
if (cmdchar == 'K' && !kp_help)
{
/* Sanitize properly */
- if ((p = vim_strsave_shellescape(ptr, TRUE)) == NULL);
- {
- printf("ERROR: out of memory\n");
- exit(1);
- }
- (char_u *)vim_realloc(buf, STRLEN(buf) + STRLEN(p) + 1);
+ if ((p = vim_strsave_shellescape(ptr, TRUE)) == NULL ||
+ (char_u *)vim_realloc(buf, STRLEN(buf) + STRLEN(p) + 1) == NULL)
+ /* Out of memory */
+ return;
STRCAT(buf, p);
vim_free(p);
}

Earlier in this function, the code does just return when out of
memory, so that's what I'll do as well. The realloc() return value
was not checked, so I fixed that as well. The updated patch (version
2) is attached.

Cheers,
Jan.

Jan Minář

unread,
Aug 23, 2008, 2:26:25 AM8/23/08
to Bram Moolenaar, Vim Developers
> was not checked, so I fixed that as well. The updated patch (version
> 2) is attached.

It is now.

Cheers,
Jan.

K-arbitrary-command-execution.patch.v2

Bram Moolenaar

unread,
Aug 23, 2008, 3:59:39 AM8/23/08
to John Beckett, vim...@googlegroups.com

John Becket wrote:

It's more like the "execute this attachment to see a movie of xyz nude". Or
the signature virus:

Hi! I'm a signature virus. Please add me to your signature and help me
spread!

Or this one:

This is the polymorph virus! Follow these instructions carefully:
1. Send this message to everybody you know.
2. Format your harddisk.
Thank you for your cooperation in spreading the most powerful virus ever!

The problem with K might have less success...

There even is a wikipedia article on it:
http://en.wikipedia.org/wiki/Honor_system_virus

--
The fastest way to get an engineer to solve a problem is to declare that the
problem is unsolvable. No engineer can walk away from an unsolvable problem
until it's solved.


(Scott Adams - The Dilbert principle)

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\

Jan Minář

unread,
Aug 23, 2008, 5:55:08 AM8/23/08
to vim...@googlegroups.com
On Sat, Aug 23, 2008 at 8:59 AM, Bram Moolenaar <Br...@moolenaar.net> wrote:
>
>
> John Becket wrote:
>
>> Tony Mechelynck wrote:
>> > Maybe you should set a config-time option (or create one) to
>> > avoid any interaction with the shell?
>> >
>> > Even better: If you don't want ever to become the victim of
>> > any exploit, turn your computer off at the wall switch and
>> > leave it off.
>> >
>> > :-b
>>
>> I haven't studied this example, but as I understand it, the suggestion is that I
>> could send you a file with a message like "What's the deal with this weird message
>> that Vim gives? Open file xxx and search for yyy then press K."
>>
>> Jan is saying (I think) that following those instructions could execute malware.
>>
>> Sure, it will never happen to me or you, but if we were discussing Microsoft Word,
>> most people would have no hesitation in declaring that such a vulnerability (press a
>> key in a document to get owned) is just NOT acceptable.
>>
>> We aren't talking about mapping K to execute "system('dodgyfile')". K is performing
>> its default function, but that function could exploit you if executed on certain
>> text, with a certain file present.
>>
>> If my understanding is correct, I don't think it's reasonable to write this off with
>> the "switch power off" joke (if I've got this wrong, please correct me).
>
> It's more like the "execute this attachment to see a movie of xyz nude". Or
> the signature virus:

Please! It's an editor. What kind of vulnerabilities would you
expect? You have to open a file, have a reasonable feature set
enabled, and do something. The real question is, why does the impact
always have to be arbitrary Ex/shell commands execution? And why are
there so many?

But it's a feature really: We all know it can take a long time for a
package maintainer to include a patch. Or perhaps, said package
maintainer thinks our patch sucks. Pperhaps he's even right! Not to
worry. With all these vulnerabilities, as long as they're is using
Vim, we can have all our patches included _as soon as they view them_.
We can even have the packages built and uploaded at the same time!

Cheers,
Jan.

Pınar Yanardağ

unread,
Aug 23, 2008, 9:45:33 PM8/23/08
to vim_dev
Hi,

After applying this patch to Vim 7.2, I got following errors while
trying to use K command (and shell also freezes after getting the
errors). I tried to reproduce them with a stable scenario, but
couldn't find a reasonable one. And also, K command sometimes works as
expected, too.

I don't know if they're Vim-related or am I missing something, though.
I'd be happy if you can give me one or two clues.

----
No manual entry for xml version="1.0" ?>

shell returned 16

Press ENTER or type command to continueVim: Caught deadly signal SEGV

----

and another one:

----

No manual entry for appName = "Kaptan Wallpaper"

shell returned 16

Press ENTER or type command to continueVim: Caught deadly signal ABRT
Vim: preserving files...
----

Cheers,

--
Pınar Yanardağ (a.k.a PINguAR)
http://pinguar.org
_____________________________

I am rarely happier than when spending entire day programming my
computer to perform automatically a task that it would otherwise take
me a good ten seconds to do by hand.

Douglas Adams
>  K-arbitrary-command-execution.patch.v2
> 5KViewDownload

Jan Minář

unread,
Aug 24, 2008, 2:26:38 AM8/24/08
to vim...@googlegroups.com
On Sun, Aug 24, 2008 at 2:45 AM, Pınar Yanardağ <moonq...@gmail.com> wrote:
> After applying this patch to Vim 7.2, I got following errors while
> trying to use K command (and shell also freezes after getting the
> errors). I tried to reproduce them with a stable scenario, but
> couldn't find a reasonable one. And also, K command sometimes works as
> expected, too.

> No manual entry for xml version="1.0" ?>
>
> shell returned 16

This looks like you are visually selecting ``xml version="1.0" ?>'',
and pressing K. man says "No manual entry for <foo>", as it does,
then returns 16.

> Press ENTER or type command to continueVim: Caught deadly signal SEGV

Since the command has already been executed, this is past the function
that the patch affects, and therefore seems like an unrelated bug. A
backtrace would be nice.

> No manual entry for appName = "Kaptan Wallpaper"
>
> shell returned 16
>
> Press ENTER or type command to continueVim: Caught deadly signal ABRT
> Vim: preserving files...

Same as above. Why would anybody think there were a manual entry for
``appName = "Kaptan Wallpaper"'' is not clear to me...

If you can't reproduce the behaviour 100% of time, it's still better
to describe it. Otherwise, it's just guesswork.

Cheers,
Jan.

Jan Minář

unread,
Aug 24, 2008, 3:33:54 AM8/24/08
to vim...@googlegroups.com
On Sun, Aug 24, 2008 at 7:26 AM, Jan Minář <rda...@rdancer.org> wrote:
> On Sun, Aug 24, 2008 at 2:45 AM, Pınar Yanardağ <moonq...@gmail.com> wrote:
>> After applying this patch to Vim 7.2, I got following errors while
>> trying to use K command (and shell also freezes after getting the
>> errors). I tried to reproduce them with a stable scenario, but
>> couldn't find a reasonable one. And also, K command sometimes works as
>> expected, too.
>
>> No manual entry for xml version="1.0" ?>
>>
>> shell returned 16
>
> This looks like you are visually selecting ``xml version="1.0" ?>'',
> and pressing K. man says "No manual entry for <foo>", as it does,
> then returns 16.
>
>> Press ENTER or type command to continueVim: Caught deadly signal SEGV

Thanks for reporting this. Forget my last email. This is the fix:

/*
* Now grab the chars in the identifier
*/
! if (cmdchar == 'K' && !kp_help)
! {
! /* Sanitize properly */
-! if ((p = vim_strsave_shellescape(ptr, TRUE)) == NULL ||
-! (char_u *)vim_realloc(buf, STRLEN(buf) + STRLEN(p) + 1) == NULL)
+! if ((p = vim_strsave_shellescape(ptr, TRUE)) == NULL)
! /* Out of memory */
! return;
+! if ((buf = (char_u *)vim_realloc(buf, STRLEN(buf) + STRLEN(p) + 1))
+! == NULL)
+! {
+! /* Out of memory */
+! /* XXX Print an Out of Memory error message here */
+! vim_free(p);
+! return;
+! }
! STRCAT(buf, p);
! vim_free(p);
! }

The updated patch (version 3) attached.

Cheers,
Jan.

K-arbitrary-command-execution.patch.v3

Pınar Yanardağ

unread,
Aug 24, 2008, 5:31:22 AM8/24/08
to vim...@googlegroups.com
On Sun, Aug 24, 2008 at 10:33 AM, Jan Minář <rda...@rdancer.org> wrote:
On Sun, Aug 24, 2008 at 7:26 AM, Jan Minář <rda...@rdancer.org> wrote:
(...)
Thanks for reporting this.  Forget my last email.  This is the fix:



It works fine after v3 patch. Thanks.


 
Reply all
Reply to author
Forward
0 new messages