Loading ...
Sorry, an error occurred while loading the content.

46719Re: feedkeys() allowed in sandbox

Expand Messages
  • John Beckett
    May 3, 2007
    • 0 Attachment
      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 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).

    • Show all 25 messages in this topic