Re: feedkeys() allowed in sandbox
- On Tue, 1 May 2007 19:42:02 +1000, "John Beckett"
> Matthew Winn wrote:What constitutes a "reasonable length"? Vim has to load the entire
> > 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.
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
> It's sensational that Vim can process files with very longWhere do you draw the line between absurd and reasonable? When I write
> lines, for the occasions we need that. But it would be absurd
> for Vim to process a multi-megabyte modeline.
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
> By all means abuse me for my cheeky suggestion to limitWhy?
> modelines to 100 bytes, but while doing that you might agree
> that some limit under 1MB should be enforced.
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 itI've been working with computer security for over two decades. I know
> > 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.
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 toThat's what I meant, and that's exactly what Vim needs as well. Both
> > 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.
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 tainting mechanism is
designed to prevent, and that's exactly what Vim must prevent too.
- Matthew Winn wrote:
> My objection to your idea is that it won't improve security by evenWe've probably slugged this out enough, but I'm glad to have
> the tiniest bit. It's not defence in depth at all.
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
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 thatYou may be right. But if I were to accidentally execute malware,
> it won't improve security by even the tiniest bit.
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 guy<g>. 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).
- On Fri, 4 May 2007 14:20:22 +1000
"John Beckett" <winterwaffle@...> wrote:
> I mentioned that the first step for point 4 should (IMHO) beMost previous exploits have been exploitable with far below the line
> rejecting any modeline beyond some fairly small maximum size.
length that is reasonably used by sensible people.
> What I'd really like would be a separate sanity check thathttp://www.vim.org/scripts/script.php?script_id=1876
> 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.
> For example, 100 bytes of malware might be able to erase my100 bytes is more than enough room to download and execute a file that
> files, but perhaps it couldn't do something more sophisticated
> like launching a hidden infiltration of my network.
contains the real malicious code.
- Ciaran McCreesh wrote:
> 100 bytes is more than enough room to download and executeI actually agree that it is extremely unlikely that a length
> a file that contains the real malicious code.
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.