Re: feedkeys() allowed in sandbox

2007-05-04 Thread Ciaran McCreesh
On Fri, 4 May 2007 14:20:22 +1000
John Beckett [EMAIL PROTECTED] wrote:
 I mentioned that the first step for point 4 should (IMHO) be
 rejecting any modeline beyond some fairly small maximum size.

Most previous exploits have been exploitable with far below the line
length that is reasonably used by sensible people.

 What I'd really like would be a separate sanity check that
 verifies that the syntax in the modeline is boringly standard
 'set' options for a declared whitelist of things that a modeline
 is allowed to do.

http://www.vim.org/scripts/script.php?script_id=1876

 For example, 100 bytes of malware might be able to erase my
 files, but perhaps it couldn't do something more sophisticated
 like launching a hidden infiltration of my network.

100 bytes is more than enough room to download and execute a file that
contains the real malicious code.

-- 
Ciaran McCreesh



signature.asc
Description: PGP signature


Re: feedkeys() allowed in sandbox

2007-05-04 Thread John Beckett

Ciaran McCreesh wrote:

100 bytes is more than enough room to download and execute
a file that contains the real malicious code.


I actually agree that it is extremely unlikely that a length
check would make modelines more secure, but I'm being
argumentative because it's irritating to be authoritatively
assured that a length check would have no benefit in the future.

We just don't know whether some future vulnerability (perhaps
using a currently-unknown new feature) might be avoided with a
modeline length check.

John



Re: feedkeys() allowed in sandbox

2007-05-03 Thread Matthew Winn
On Tue, 1 May 2007 19:42:02 +1000, John Beckett
[EMAIL PROTECTED] wrote:

 Matthew Winn wrote:
  If there was a security problem in Vim do you really think it
  couldn't be exploited in 100 characters? That's a pretty shaky
  foundation on which to build your security.
 
 I am quite surprised at the lack of appreciation for the merits
 of defense in depth here. I am not claiming that a length
 limit would preclude damage, just that a modeline should be
 sanity checked before execution, and a reasonable length would
 be the first check.

What constitutes a reasonable length? Vim has to load the entire
document including its modeline into memory anyway, so there's no
denial-of-service implications in allowing unlimited modelines.
Your suggestion amounts to I won't use a modeline longer than X,
so nobody will use a modeline longer than X.

My objection to your idea is that it won't improve security by even
the tiniest bit. It's not defence in depth at all. It's a worthless
measure that can't achieve anything useful and can only get in the
way of legitimate uses. Any modeline long enough to be useful for a
legitimate purpose must also be long enough to be useful for a hostile
one.

 It's sensational that Vim can process files with very long
 lines, for the occasions we need that. But it would be absurd
 for Vim to process a multi-megabyte modeline.

Where do you draw the line between absurd and reasonable? When I write
options I spell out the names in full so they're easier to understand
for someone who doesn't know Vim well. That means that my modelines
are quite long. But someone who wanted to save space could use the
abbreviated form of an option. That means that if a modeline can be
long enough to satisfy me it would give an attacker the ability to use
several times as many options to craft their exploit as are needed for
general use.

 By all means abuse me for my cheeky suggestion to limit
 modelines to 100 bytes, but while doing that you might agree
 that some limit under 1MB should be enforced.

Why?

In some places there are good reasons for limiting sizes. For example,
RFC2822 places a limit of 998 characters on the length of a line. The
reason for this is so that RFC2822-conforming applications don't have
to deal with data of arbitrary length and allocate unlimited buffers
to handle it. They can allocate a buffer 1001 characters long and
discard anything that won't fit in the buffer, thereby preventing the
possibility of denial-of-service attacks from someone sending a line
several hundred megabytes long.

Vim doesn't have that issue because it must load the entire file into
memory anyway. Vim already knows how to deal with long lines, so
there's no extra penalty incurred by a multi-megabyte modeline.

  A web browser should be able to handle anything thrown at it
  in a way that doesn't compromise security. _Every_ application
  should be able to handle anything thrown at it in a way that
  doesn't compromise security.
 
 Even if a program is perfect now, a later change can introduce a
 bug. Any program which can automatically execute untrusted code
 should sanity-check the input as a separate step from
 sandboxing. That is standard Security 101 stuff - not my idea.

I've been working with computer security for over two decades. I know
about standard security stuff. I also know that security that doesn't
work is worse than no security at all, because it creates an illusion
of protection where none exists.

  Perl and Vim have exactly the same requirements: the need to
  safely handle code taken from an untrustworthy source. It
  makes no difference whether it comes directly from a network
  or from a disk. (If, like me, you use Vim as your source
  viewer for web pages, the need for the same level of security
  is obvious.)
 
 It doesn't matter, but for the record, Perl's tainting system is
 not related to the scenario you describe. Perl wants to make
 sure that untrusted input is not later used as the basis for
 some expression that could do harm, such as executing SQL code.

That's what I meant, and that's exactly what Vim needs as well. Both
applications read data from a source that can't be trusted, and both
need to ensure that untrusted data can't be used in a situation where
it could be dangerous. In Vim's case it needs to make sure that an
expression used in an option set from a modeline can't be used later
in a way that would cause harm, such as executing a command.

Take a look at the original message. It sets foldmethod to something
that triggers the execution of an external command after the modeline
has been processed. Imagine you have a web page that contains the
following (with the real command removed so it can't cause problems,
just in case someone does view this in Vim; think of rm -rf /):

!--
vim: fdm=expr fde=feedkeys(\\:!dangerous-command-here\\cr)
--

Now imagine that someone uses Vim as their browser's view source
application. That's _exactly_ the thing Perl's 

Re: feedkeys() allowed in sandbox

2007-05-03 Thread John Beckett

Matthew Winn wrote:

My objection to your idea is that it won't improve security by even
the tiniest bit. It's not defence in depth at all.


We've probably slugged this out enough, but I'm glad to have
another opportunity to promote the safe modelines message.

Bram has made the point that despite repeated modeline
vulnerabilities over several years, there are no known cases of
a malicious attack. We have only seen PoC and jokes.

I see the sense of that position - why put in a bunch of ugly
checking which is going to reduce features and upset some users?
Why do it if there are no known benefits?

My answer is essentially an appeal to a higher moral purpose.
There may never be in-the-wild exploits based on modelines, but
that would make it all the easier to direct a specific attack
against a targeted victim. The attacker would have a list of 10
or 20 slight security flaws in the victim's network. One of
those would be the fact that the victim uses Vim. An attacker
may use a Vim modeline as the coup de grace to fully own the
victim's network.

I find that situation offensive, and believe that modelines
should be REALLY fixed.

My claim is:
1. A modeline can execute untrusted code.
2. That is incredibly dangerous.
3. Any bugs in modeline handling should be fixed.
4. In addition, modelines should be sanity checked.

I think we agree on points 1-3.

I mentioned that the first step for point 4 should (IMHO) be
rejecting any modeline beyond some fairly small maximum size.

However, that was just the first part of my hoped-for sanity
check. After that, I would like the modeline to be examined to
determine whether there are any constructs that look
dangerous. I would reject any modeline with more than ten
backslashes, and would reject anything looking like an
expression or 'call'.

What I'd really like would be a separate sanity check that
verifies that the syntax in the modeline is boringly standard
'set' options for a declared whitelist of things that a modeline
is allowed to do. Note that this checking should NOT be done
only in the code that executes the modeline. The checking should
be an independent, prior step. That redundancy is likely to save
someone's foot in future years, when extra features are added.


My objection to your idea [to limit modeline length] is that
it won't improve security by even the tiniest bit.


You may be right. But if I were to accidentally execute malware,
I would prefer that the malware was short, rather than of an
essentially unlimited length. I agree that 100 bytes of malware
could do more damage than I could bear, but I would still
prefer that situation.

For example, 100 bytes of malware might be able to erase my
files, but perhaps it couldn't do something more sophisticated
like launching a hidden infiltration of my network.

I don't really know why I want to limit the modeline length.
That's the whole point of proper security measures. Just because
I can't think of a way that a long modeline might be bad, does
not mean that some attacker won't find a way, particularly in
five years after a bunch more stuff has been added to Vim.


That means that my modelines are quite long.


I'm a reasonable guyg. Let's take your longest modeline and
double it. That length should be the maximum allowed for a
modeline unless some new anything goes option is enabled.

Re Perl tainting: I think we essentially agree on this, although
I don't think Vim needs to mark an executable expression read
from a modeline as tainted. Vim should immediately reject any
modeline that might execute code (unless some new anything
goes option is enabled).

John



Re: feedkeys() allowed in sandbox

2007-05-01 Thread Bram Moolenaar

John Beckett wrote:

 A.J.Mechelynck wrote:
  Is folding really needed in a default modeline?
  Folding may be useful in a modeline.
  (Don't know what you call a default modeline.)
 
 By default modeline I mean I would like Vim to be changed so
 that its default behaviour is aggressively safe. If wanted,
 there could be a new option to enable clever features, and a
 user could choose to allow modelines with folding or expression
 evaluation, etc.

This is not true.  It just reduces the chance of a mistake being made by
an unknown factor.  It's still possible to allow an option to be set,
thinking that it is OK, but we later find out that it was not OK.  Just
like carefully removing mistakes and screening the options for mistakes
does help to make it safer.  Thus it doesn't make an essential
difference.  N times as safe still isn't 100% safe.

In other words: If we have an option run insecure nobody would set it.
Vim must be secure as-is.

 But the only long-term safe procedure is to have Vim *default*
 to work with only very restricted modelines (set tab and other
 options - no way to even get near executing code).

As they sometimes joke: The best way to protect your computer from
malicious software is to switch it off.  Likewise, the only really safe
way is to disable modelines.  Obviously you pay a price: restricted
functionality.  Options to partly disable modelines make it more
complicated and don't help much for security.

 I am wondering what the lack of comment on this topic indicates.
 Do you understand that another modeline vulnerability could
 allow the next file you open to overwrite all files under your
 home folder? Or it might overwrite all sectors on your disk, if
 you have sufficient privilege.

Don't forget that this requires someone who intentionally wants this
evil thing to happen.  So far the only examples seen are jokes and proof
of concept.  I have never seen a file with a modeline that intentionally
causes harm.

 How about if you go to another computer that you rarely use.
 Would you be happy using Vim on that computer?
 Network admins in secure environments should be prohibited
 from using Vim.

Modelines are default off when you are root.  The mail filetype plugin
also switches it off.

 If I am overlooking something, or am overly alarmist, please
 tell me. For anyone new to this, enter following in Google:
 vim vulnerability modeline

Thanks for the advertisement!  :-).

-- 
Give a man a computer program and you give him a headache,
but teach him to program computers and you give him the power
to create headaches for others for the rest of his life...
R. B. Forest

 /// Bram Moolenaar -- [EMAIL PROTECTED] -- 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///


Re: feedkeys() allowed in sandbox

2007-05-01 Thread John Beckett

Bram Moolenaar wrote:

N times as safe still isn't 100% safe.


I am not claiming that sanity-checking a modeline before
execution would make it 100% safe. But there have been many
examples in other software where minor bugs have turned into
security disasters because some simple point that could have
been checked, wasn't.

While code is working correctly, a simple check is redundant,
and indeed is offensive because it lengthens and obscures the
code. But a few simple checks may prevent disaster at some
future time, when Vim is further developed.

The Google test (searching for past instances of trouble with
Vim's modeline) proves the case that future problems are likely.


Modelines are default off when you are root.
The mail filetype plugin also switches it off.


Good grief - I didn't know that. So you *have* got sanity checks
built in! I'll go and sit in the corner now, but thanks for
confirming that multiple layers of defence are desirable.

John



Re: feedkeys() allowed in sandbox

2007-05-01 Thread John Beckett

Matthew Winn wrote:

If there was a security problem in Vim do you really think it
couldn't be exploited in 100 characters? That's a pretty shaky
foundation on which to build your security.


I am quite surprised at the lack of appreciation for the merits
of defense in depth here. I am not claiming that a length
limit would preclude damage, just that a modeline should be
sanity checked before execution, and a reasonable length would
be the first check.

It's certainly true that a modeline of 100 bytes could wreak
havoc on an unpatched Vim. But it is quite possible that a
future vulnerability might allow code to be injected from a
modeline, and limiting its length *might* make the attacker's
job harder.

It's sensational that Vim can process files with very long
lines, for the occasions we need that. But it would be absurd
for Vim to process a multi-megabyte modeline.

By all means abuse me for my cheeky suggestion to limit
modelines to 100 bytes, but while doing that you might agree
that some limit under 1MB should be enforced.


A web browser should be able to handle anything thrown at it
in a way that doesn't compromise security. _Every_ application
should be able to handle anything thrown at it in a way that
doesn't compromise security.


Even if a program is perfect now, a later change can introduce a
bug. Any program which can automatically execute untrusted code
should sanity-check the input as a separate step from
sandboxing. That is standard Security 101 stuff - not my idea.


What you're suggesting isn't much different from security
through obscurity.


Jargon is great, but not when it's misused.


Perl and Vim have exactly the same requirements: the need to
safely handle code taken from an untrustworthy source. It
makes no difference whether it comes directly from a network
or from a disk. (If, like me, you use Vim as your source
viewer for web pages, the need for the same level of security
is obvious.)


It doesn't matter, but for the record, Perl's tainting system is
not related to the scenario you describe. Perl wants to make
sure that untrusted input is not later used as the basis for
some expression that could do harm, such as executing SQL code.

John



Re: feedkeys() allowed in sandbox

2007-05-01 Thread A.J.Mechelynck

Bram Moolenaar wrote:
[...]

Modelines are default off when you are root.  The mail filetype plugin
also switches it off.

[...]

Are you sure? In a terminal logged-in as root, using vim 7.0.235:

vim -u NONE -N
:set ml? mls?
  modeline
  modelines=5

Modelines default off when 'compatible' is set, they default on (and 5) when 
'nocompatible' is set. Root login changes nothing to that AFAICT.



Best regards,
Tony.
--
You mean there really is an answer?
Yes! But you're not going to like it!
Oh do please tell us!
You're really not going to like it!
but we MUST know - tell us
Alright, the answer is
yes...
... is ...
yes... come on!
is 42!
(Douglas Adams - The Hitchhiker's Guide to the Galaxy)


Re: feedkeys() allowed in sandbox

2007-05-01 Thread Bram Moolenaar

Tony Mechelynck wrote:

 Bram Moolenaar wrote:
 [...]
  Modelines are default off when you are root.  The mail filetype plugin
  also switches it off.
 [...]
 
 Are you sure? In a terminal logged-in as root, using vim 7.0.235:
 
   vim -u NONE -N
   :set ml? mls?
modeline
modelines=5
 
 Modelines default off when 'compatible' is set, they default on (and 5) when 
 'nocompatible' is set. Root login changes nothing to that AFAICT.

Sorry, my mistake.  There is a recommendation that when working as root
you switch 'modeline' off, but it's not done automatically.

I do think that it's a good idea to make it automatic.

-- 
He who laughs last, thinks slowest.

 /// Bram Moolenaar -- [EMAIL PROTECTED] -- 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///


Re: feedkeys() allowed in sandbox

2007-04-30 Thread A.J.Mechelynck

John Beckett wrote:
[...]

Is folding really needed in a default modeline?

John



Folding may be useful in a modeline. (Don't know what you call a default 
modeline.) Depending on how the particular file is written, you may want to 
set foldmethod=marker (and which marker), foldmethod=syntax, 
foldmethod=indent, or default it to whatever (if anything) is set by the 
filetype-plugin.



Best regards,
Tony.
--
Some of these fortunes are dated: I have an ADSL connection and a 96 gig HD, 
and I don't feel it's any special kind of achievement.

--
hundred-and-one symptoms of being an internet addict:
208. Your goals for the future are obtaining an ISDN connection and
 a 6 gig hard drive.


Re: feedkeys() allowed in sandbox

2007-04-30 Thread John Beckett

A.J.Mechelynck wrote:

Is folding really needed in a default modeline?

Folding may be useful in a modeline.
(Don't know what you call a default modeline.)


By default modeline I mean I would like Vim to be changed so
that its default behaviour is aggressively safe. If wanted,
there could be a new option to enable clever features, and a
user could choose to allow modelines with folding or expression
evaluation, etc.

But the only long-term safe procedure is to have Vim *default*
to work with only very restricted modelines (set tab and other
options - no way to even get near executing code).

I am wondering what the lack of comment on this topic indicates.
Do you understand that another modeline vulnerability could
allow the next file you open to overwrite all files under your
home folder? Or it might overwrite all sectors on your disk, if
you have sufficient privilege.

How about if you go to another computer that you rarely use.
Would you be happy using Vim on that computer?
Network admins in secure environments should be prohibited
from using Vim.

If I am overlooking something, or am overly alarmist, please
tell me. For anyone new to this, enter following in Google:
vim vulnerability modeline

I just noticed that the fourth hit features Ciaran McCreesh who
discovered a vulnerability in January 2005.

John



Re: feedkeys() allowed in sandbox

2007-04-30 Thread Matthew Winn
On Sun, 29 Apr 2007 19:10:55 +1000, John Beckett
[EMAIL PROTECTED] wrote:

 Matthew Winn wrote:
  I don't like the idea of preventing modelines over 100 bytes.
 
 I imagine (haven't looked) that a modeline has no hard limit to
 its length. So multi-megabyte modelines are probably handled by
 Vim. That's potentially offering attackers extraordinary power.

It doesn't matter how many bytes are accepted. Security that depends
on the assumption that an exploit can't be written in less than an
arbitrarily chosen number of bytes is no security at all. Take a look
at some of the coding golf competitions that take place online, where
the object is to perform some complex task in the minimum number of
characters.

If there was a security problem in Vim do you really think it couldn't
be exploited in 100 characters? That's a pretty shaky foundation on
which to build your security.

 Would someone who wants a modeline longer than 100 bytes please
 show us an example. How about a 200-byte limit?

Oh, so nobody will need a long modeline? Just like they will never
need more than [insert your favourite inaccurate prediction about
the maximum amount of computing power anyone would ever need here].

 Modelines are enabled by default, and are very useful for things
 like setting tabs. So most people, and all new installs, will
 have modelines enabled. It's very poor security practice to
 offer a rich auto-execution environment with a single line of
 defence (the sandbox).

 This discussion reminds me of the days of the Code Red
 vulnerability in IIS (Microsoft web server), and of the
 years of repeated vulnerabilities in Internet Explorer.
 
 The IIS and IE developers just couldn't bring themselves to
 build in limits to what their wonderful software could do.
 But a web site might need a 100KB URL with hundreds of '../'
 paths!.

A web browser should be able to handle anything thrown at it in a way
that doesn't compromise security. _Every_ application should be able
to handle anything thrown at it in a way that doesn't compromise
security.

What you're suggesting isn't much different from security through
obscurity. You're relying on the hope that nobody would ever be able
to craft an exploit in under 100 bytes. Security doesn't work like
that. Security needs to be something people can rely on to work every
time, not something that depends on Well, let's hope nobody thinks
of this.

If Vim's modeline security is written correctly and securely then
the length of modeline it can handle safely would depend only on the
amount of memory it wants to allocate to hold it. If it isn't able to
do that then there's no security at all.

  Furthermore, what am I supposed to do if I want a long,
  complicated but legitimate modeline?
 
 I would like a default high security setting for handling
 modelines. If people want modelines that do complex stuff, I
 would recommend setting a new anything goes option.

ABSOLUTELY NOT! Are you honestly suggesting that to enter a long
modeline you have to disable security? I wouldn't touch an editor
like that.

  I like Perl's approach to untrustworthy data. It's flagged as
  tainted at the point it is read, and anything derived from it
  is also flagged as tainted.
 
 Perl has to have that system because part of its intended usage
 is to handle data entered into web pages. It's pretty complex
 and has taken years to get right.
 
 Vim is a text editor - it should not automatically execute code
 in any old file that I might accidentally open.

Perl and Vim have exactly the same requirements: the need to safely
handle code taken from an untrustworthy source. It makes no difference
whether it comes directly from a network or from a disk. (If, like me,
you use Vim as your source viewer for web pages, the need for the same
level of security is obvious.)

-- 
Matthew Winn


Re: feedkeys() allowed in sandbox

2007-04-29 Thread Matthew Winn
On Sat, 28 Apr 2007 22:43:23 +1000, John Beckett
[EMAIL PROTECTED] wrote:

 Potentially unsafe means we're pretty sure it IS safe, but
 (for example), it's simply not worthwhile allowing a modeline
 longer than 100 bytes because if another vulnerability were
 ever found, we don't want to make it easy for the attacker.

I don't like the idea of preventing modelines over 100 bytes. To start
with, there's no real logic behind it: it's an arbitrary number pulled
out of thin air, and I put it in the same category as saying it's OK
to use gets() so long as you use a long enough buffer that it'll never
overflow. A modeline that's long enough to allow useful things to be
done is long enough to allow unpleasant things to be done.

Furthermore, what am I supposed to do if I want a long, complicated
but legitimate modeline?

I like Perl's approach to untrustworthy data. It's flagged as tainted
at the point it is read, and anything derived from it is also flagged
as tainted. Tainted information cannot be used in unsafe operations,
ever. From what I've read in this thread Vim does something similar,
but in a way that's less complete. That's the right way to go about
it. Setting an arbitrary limit and hoping it'll have the effect of
improving security is far too optimistic for my tastes.

-- 
Matthew Winn


Re: feedkeys() allowed in sandbox

2007-04-29 Thread John Beckett

Matthew Winn wrote:

I don't like the idea of preventing modelines over 100 bytes.


I imagine (haven't looked) that a modeline has no hard limit to
its length. So multi-megabyte modelines are probably handled by
Vim. That's potentially offering attackers extraordinary power.

Would someone who wants a modeline longer than 100 bytes please
show us an example. How about a 200-byte limit?

Modelines are enabled by default, and are very useful for things
like setting tabs. So most people, and all new installs, will
have modelines enabled. It's very poor security practice to
offer a rich auto-execution environment with a single line of
defence (the sandbox).

This discussion reminds me of the days of the Code Red
vulnerability in IIS (Microsoft web server), and of the
years of repeated vulnerabilities in Internet Explorer.

The IIS and IE developers just couldn't bring themselves to
build in limits to what their wonderful software could do.
But a web site might need a 100KB URL with hundreds of '../'
paths!.


Furthermore, what am I supposed to do if I want a long,
complicated but legitimate modeline?


I would like a default high security setting for handling
modelines. If people want modelines that do complex stuff, I
would recommend setting a new anything goes option.


I like Perl's approach to untrustworthy data. It's flagged as
tainted at the point it is read, and anything derived from it
is also flagged as tainted.


Perl has to have that system because part of its intended usage
is to handle data entered into web pages. It's pretty complex
and has taken years to get right.

Vim is a text editor - it should not automatically execute code
in any old file that I might accidentally open.

John



Re: feedkeys() allowed in sandbox

2007-04-29 Thread Bram Moolenaar

Ciaran McCreesh wrote:

 On Sat, 28 Apr 2007 21:52:07 +0200
 Bram Moolenaar [EMAIL PROTECTED] wrote:
  I don't like this solution.  Opening some files would be OK in the
  sandbox, e.g., for reading.  readfile() would be OK in the sandbox,
  right?
 
 Probably not. In a multi-user environment it can be used as a
 privilege escalation by inserting the contents of a non-world-readable
 file into a world-readable file when the latter is edited by a user
 with elevated privileges.

In the sandbox you can't insert text into a file or buffer.  Anything
that requires saving text for undo is blocked.

You can also get the text from an already opened file with getbufline().
It's difficult to draw a line, but I think blocking everything that
writes is good enough.

-- 
`The Guide says there is an art to flying,' said Ford, `or at least a
knack. The knack lies in learning how to throw yourself at the ground
and miss.' He smiled weakly.
-- Douglas Adams, The Hitchhiker's Guide to the Galaxy

 /// Bram Moolenaar -- [EMAIL PROTECTED] -- 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///


Re: feedkeys() allowed in sandbox

2007-04-28 Thread John Beckett

Bram Moolenaar wrote:

That's pretty nasty.  I'll make a patch right away.


Thanks. However, perhaps the modeline concept needs
more safety - defence in depth.

Perhaps modelines should only allow a VERY limited set
of operations by default (even more restricted than now).

Googling for 'vim feedkeys joke' shows April 1 joke with
the following (I've replaced vim with vvv):

vvv: foldmethod=expr:foldexpr=feedkeys(
 \\esc\\x3a%!cat\\x20-n\\CR\\esc\\x3a%s/./\:)/g\\CR
 \\esc\\x3aq!\\CR):

I'm too lazy to unobfuscate this, but one glance tells you
that modelines should not be fixed - going down that path
is likely to give a new vulnerability every year.

Instead, modelines should be SEVERELY limited by default.
Examples:
Total length  100 bytes.
No expressions; no function calls; no execution.
Treat a double-quoted string as if in single quotes.
Is folding really needed in a default modeline?

John



Re: feedkeys() allowed in sandbox

2007-04-28 Thread John Beckett

Tomas Golembiovsky wrote:

today somebody came to #vim, and pasted some modeline (containig
joke or such).


Thanks for raising that issue. I found the April 1 joke with Google.

I actually noticed that posting (seeing Vim while browsing a security
list caught my attention), but there's so much waffle out there that I
didn't pay any attention to the point of the message.

John



Re: feedkeys() allowed in sandbox

2007-04-28 Thread John Beckett

Bram Moolenaar wrote:

Perhaps modelines should only allow a VERY limited set
of operations by default (even more restricted than now).


Sure, simply use :set nomodeline.


I'm suggesting defence in depth. My vimrc might have
':set nomodeline', but what if I make a mistake? What if I'm
using some other machine where I'm not sure what's in vimrc?
What if I use the -u option? And there are probably other
scenarios where a simple oversight could cause me to execute
a modeline in an untrusted file.

What if I want modelines (but never to do more than set a few
options like tabs).


Even setting 'textwidth' to 2 may already be considered
harmful, or at least annoying.


I don't mind being occasionally irritated with a stupid
modeline. But I really don't want an editor to execute code
when I open a file. Change volatile settings - no problem.
Execute - no thanks.


Perhaps we can add yet-another-option to completely disable
setting some options from the modeline.


Modelines should default to be safe (safe by design, AND safe
because of defence in depth). If another option were added, it
should be to allow expressions or other features that are
potentially unsafe (not disallow unsafe features).

Potentially unsafe means we're pretty sure it IS safe, but
(for example), it's simply not worthwhile allowing a modeline
longer than 100 bytes because if another vulnerability were
ever found, we don't want to make it easy for the attacker.


It's better to make sure the sandbox works as it should.


Of course, but bitter experience shows that defence in depth is
the best strategy. Vim has a lot of very clever features that
are too complex to ever be secure, when executing a modeline
from an untrusted source.

John



Re: feedkeys() allowed in sandbox

2007-04-27 Thread Bram Moolenaar

Tomas Golembiovsky wrote:

 today somebody came to #vim, and pasted some modeline (containig joke or
 such). He muttered something about not knowing what that means and left
 before long. But (!) what I noticed is that feedkeys() was used as part of
 foldexpression and it turned out that feedkeys() is allowed in sandbox,
 which means malicious file can run arbitrary command via modeline like
 this:
 
 vim: fdm=expr fde=feedkeys(\\:!touch\ phantom_was_here\\cr)
 
 I guess you can see the consequences. Is this known/intentional?

That's pretty nasty.  I'll make a patch right away.

-- 
Far back in the mists of ancient time, in the great and glorious days of the
former Galactic Empire, life was wild, rich and largely tax free.
Mighty starships plied their way between exotic suns, seeking adventure and
reward among the furthest reaches of Galactic space.  In those days, spirits
were brave, the stakes were high, men were real men, women were real women
and small furry creatures from Alpha Centauri were real small furry creatures
from Alpha Centauri.  And all dared to brave unknown terrors, to do mighty
deeds, to boldly split infinitives that no man had split before -- and thus
was the Empire forged.
-- Douglas Adams, The Hitchhiker's Guide to the Galaxy

 /// Bram Moolenaar -- [EMAIL PROTECTED] -- 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///


Re: feedkeys() allowed in sandbox

2007-04-27 Thread Bram Moolenaar

Tony Mechelynck wrote:

 Tomas Golembiovsky wrote:
  Greetings mortals,
  
  today somebody came to #vim, and pasted some modeline (containig joke or
  such). He muttered something about not knowing what that means and left
  before long. But (!) what I noticed is that feedkeys() was used as part of
  foldexpression and it turned out that feedkeys() is allowed in sandbox,
  which means malicious file can run arbitrary command via modeline like
  this:
  
  vim: fdm=expr fde=feedkeys(\\:!touch\ phantom_was_here\\cr)
  
  I guess you can see the consequences. Is this known/intentional?
  
 
 IIUC, feedkeys() called from sandbox should execute as if in sandbox,
 i.e., only (at most) key sequences acceptable in sandbox should be
 able to be fed. 

 Now this is what I think it ought to do. How does it actually
 behave? Did you try your example? Did it touch the file?

That was the idea: The sandbox flag is checked when the keys are
executed.  However, the sandbox flag may have been reset by then, as the
example shows.  Thus feedkeys() needs to be disallowed in the sandbox.

-- 
I have a drinking problem -- I don't have a drink!

 /// Bram Moolenaar -- [EMAIL PROTECTED] -- 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///