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). John
