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

feedkeys() allowed in sandbox

Expand Messages
  • Tomas Golembiovsky
    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
    Message 1 of 25 , Apr 26 2:35 PM
    View Source
    • 0 Attachment
      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?

      --

      Best regards,
      Tomas Golembiovsky

      --
      |========================|----- - -
      |
      | Alan's Law of Research
      |
      | The theory is supported as long as the funds are.
      |
      |----- - -
    • A.J.Mechelynck
      ... 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 .
      Message 2 of 25 , Apr 26 10:41 PM
      View Source
      • 0 Attachment
        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?


        Best regards,
        Tony.
        --
        Of what you see in books, believe 75%. Of newspapers, believe 50%.
        And of TV news, believe 25% -- make that 5% if the anchorman wears a
        blazer.
      • Bram Moolenaar
        ... 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,
        Message 3 of 25 , Apr 27 1:24 PM
        View Source
        • 0 Attachment
          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 -- Bram@... -- 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 ///
        • Bram Moolenaar
          ... 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.
          Message 4 of 25 , Apr 27 2:23 PM
          View Source
          • 0 Attachment
            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 -- Bram@... -- 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 ///
          • John Beckett
            ... Thanks. However, perhaps the modeline concept needs more safety - defence in depth. Perhaps modelines should only allow a VERY limited set of operations by
            Message 5 of 25 , Apr 28 4:12 AM
            View Source
            • 0 Attachment
              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
            • John Beckett
              ... 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
              Message 6 of 25 , Apr 28 4:16 AM
              View Source
              • 0 Attachment
                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
              • Bram Moolenaar
                ... Sure, simply use :set nomodeline . Even setting textwidth to 2 may already be considered harmful, or at least annoying. ... Modelines are already
                Message 7 of 25 , Apr 28 4:45 AM
                View Source
                • 0 Attachment
                  John Beckett wrote:

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

                  Sure, simply use ":set nomodeline". Even setting 'textwidth' to 2 may
                  already be considered harmful, or at least annoying.

                  > 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?

                  Modelines are already limited. You can't put commands there (like some
                  old versions of vi did). Options with an expression are a border case.
                  They are executed in the sandbox when an option was set from the
                  modeline. Perhaps we can add yet-another-option to completely disable
                  setting some options from the modeline. But I think it won't help much,
                  most users won't take the time or even know about the option. It's
                  better to make sure the sandbox works as it should.

                  --
                  How To Keep A Healthy Level Of Insanity:
                  11. Specify that your drive-through order is "to go".

                  /// Bram Moolenaar -- Bram@... -- 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 ///
                • John Beckett
                  ... 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
                  Message 8 of 25 , Apr 28 5:43 AM
                  View Source
                  • 0 Attachment
                    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
                  • Andrew Maykov
                    ... Yet another function to disable in sandbox: vi: fdm=expr fde=writefile([ ], phantom_was_here ) Proposal. Maybe it s sane to put security checks not just
                    Message 9 of 25 , Apr 28 11:05 AM
                    View Source
                    • 0 Attachment
                      On 4/28/07, Bram Moolenaar <Bram@...> wrote:
                      >It's better to make sure the sandbox works as it should.
                      Yet another function to disable in sandbox:
                      vi: fdm=expr fde=writefile([""],"phantom_was_here")

                      Proposal. Maybe it's sane to put security checks not just in
                      functions like f_writefile(), but also put it to the core of fileio,
                      e.g. if mch_fopen macro will check permissions before actual openning
                      file, then f_writefile() and freinds if any will fail to harm user.

                      i.e. replace something like this:
                      =CUT============================
                      --- macros.h.orig 2007-04-29 00:57:16.000000000 +0700
                      +++ macros.h 2007-04-29 00:58:38.000000000 +0700
                      @@ -149,7 +149,7 @@
                      #ifdef VMS
                      # define mch_access(n, p) access(vms_fixfilename(n), (p))
                      /* see mch_open() comment */
                      -# define mch_fopen(n, p) fopen(vms_fixfilename(n), (p))
                      +# define mch_fopen_impl(n, p) fopen(vms_fixfilename(n), (p))
                      # define mch_fstat(n, p) fstat(vms_fixfilename(n), (p))
                      /* VMS does not have lstat() */
                      # define mch_stat(n, p) stat(vms_fixfilename(n), (p))
                      @@ -158,7 +158,7 @@
                      # define mch_access(n, p) access((n), (p))
                      # endif
                      # if !(defined(FEAT_MBYTE) && defined(WIN3264))
                      -# define mch_fopen(n, p) fopen((n), (p))
                      +# define mch_fopen_impl(n, p) fopen((n), (p))
                      # endif
                      # define mch_fstat(n, p) fstat((n), (p))
                      # ifdef MSWIN /* has it's own mch_stat() function */
                      @@ -174,6 +174,9 @@
                      # endif
                      #endif

                      +
                      +#define mch_fopen(n, p) ( check_secure() ? NULL : mch_fopen_impl(n,p) )
                      +
                      #ifdef HAVE_LSTAT
                      # define mch_lstat(n, p) lstat((n), (p))
                      #else
                      =/CUT===========================
                    • Bram Moolenaar
                      ... Yep, you found another one. Seems some of the new functions added in Vim 7 were not properly checked for sandbox use. I think system() should also not
                      Message 10 of 25 , Apr 28 12:52 PM
                      View Source
                      • 0 Attachment
                        Andrew Maykov wrote:

                        > On 4/28/07, Bram Moolenaar <Bram@...> wrote:
                        > >It's better to make sure the sandbox works as it should.
                        > Yet another function to disable in sandbox:
                        > vi: fdm=expr fde=writefile([""],"phantom_was_here")

                        Yep, you found another one. Seems some of the new functions added in
                        Vim 7 were not properly checked for sandbox use. I think system()
                        should also not write the "input" argument to a file. It's quite
                        harmless, since you can't control the file name, but the shell command
                        is going to fail anyway. None of the others appear to be harmful.

                        > Proposal. Maybe it's sane to put security checks not just in
                        > functions like f_writefile(), but also put it to the core of fileio,
                        > e.g. if mch_fopen macro will check permissions before actual openning
                        > file, then f_writefile() and freinds if any will fail to harm user.
                        >
                        > i.e. replace something like this:
                        > =CUT============================
                        > --- macros.h.orig 2007-04-29 00:57:16.000000000 +0700
                        > +++ macros.h 2007-04-29 00:58:38.000000000 +0700
                        > @@ -149,7 +149,7 @@
                        > #ifdef VMS
                        > # define mch_access(n, p) access(vms_fixfilename(n), (p))
                        > /* see mch_open() comment */
                        > -# define mch_fopen(n, p) fopen(vms_fixfilename(n), (p))
                        > +# define mch_fopen_impl(n, p) fopen(vms_fixfilename(n), (p))
                        > # define mch_fstat(n, p) fstat(vms_fixfilename(n), (p))
                        > /* VMS does not have lstat() */
                        > # define mch_stat(n, p) stat(vms_fixfilename(n), (p))
                        > @@ -158,7 +158,7 @@
                        > # define mch_access(n, p) access((n), (p))
                        > # endif
                        > # if !(defined(FEAT_MBYTE) && defined(WIN3264))
                        > -# define mch_fopen(n, p) fopen((n), (p))
                        > +# define mch_fopen_impl(n, p) fopen((n), (p))
                        > # endif
                        > # define mch_fstat(n, p) fstat((n), (p))
                        > # ifdef MSWIN /* has it's own mch_stat() function */
                        > @@ -174,6 +174,9 @@
                        > # endif
                        > #endif
                        >
                        > +
                        > +#define mch_fopen(n, p) ( check_secure() ? NULL : mch_fopen_impl(n,p) )
                        > +
                        > #ifdef HAVE_LSTAT
                        > # define mch_lstat(n, p) lstat((n), (p))
                        > #else
                        > =/CUT===========================

                        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?

                        --
                        How To Keep A Healthy Level Of Insanity:
                        15. Five days in advance, tell your friends you can't attend their
                        party because you're not in the mood.

                        /// Bram Moolenaar -- Bram@... -- 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 ///
                      • Matthew Winn
                        On Sat, 28 Apr 2007 22:43:23 +1000, John Beckett ... I don t like the idea of preventing modelines over 100 bytes. To start with, there s no real logic
                        Message 11 of 25 , Apr 29 1:11 AM
                        View Source
                        • 0 Attachment
                          On Sat, 28 Apr 2007 22:43:23 +1000, "John Beckett"
                          <winterwaffle@...> 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
                        • John Beckett
                          ... 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
                          Message 12 of 25 , Apr 29 2:10 AM
                          View Source
                          • 0 Attachment
                            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
                          • Ciaran McCreesh
                            On Sat, 28 Apr 2007 21:52:07 +0200 ... Probably not. In a multi-user environment it can be used as a privilege escalation by inserting the contents of a
                            Message 13 of 25 , Apr 29 7:20 AM
                            View Source
                            • 0 Attachment
                              On Sat, 28 Apr 2007 21:52:07 +0200
                              Bram Moolenaar <Bram@...> 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.

                              --
                              Ciaran McCreesh
                            • Bram Moolenaar
                              ... 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
                              Message 14 of 25 , Apr 29 8:14 AM
                              View Source
                              • 0 Attachment
                                Ciaran McCreesh wrote:

                                > On Sat, 28 Apr 2007 21:52:07 +0200
                                > Bram Moolenaar <Bram@...> 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 -- Bram@... -- 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 ///
                              • A.J.Mechelynck
                                John Beckett wrote: [...] ... Folding may be useful in a modeline. (Don t know what you call a default modeline.) Depending on how the particular file is
                                Message 15 of 25 , Apr 29 11:32 PM
                                View Source
                                • 0 Attachment
                                  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.
                                • John Beckett
                                  ... 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
                                  Message 16 of 25 , Apr 30 2:09 AM
                                  View Source
                                  • 0 Attachment
                                    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
                                  • Matthew Winn
                                    On Sun, 29 Apr 2007 19:10:55 +1000, John Beckett ... It doesn t matter how many bytes are accepted. Security that depends on the assumption that an exploit
                                    Message 17 of 25 , Apr 30 4:43 AM
                                    View Source
                                    • 0 Attachment
                                      On Sun, 29 Apr 2007 19:10:55 +1000, "John Beckett"
                                      <winterwaffle@...> 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
                                    • Bram Moolenaar
                                      ... 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
                                      Message 18 of 25 , Apr 30 1:36 PM
                                      View Source
                                      • 0 Attachment
                                        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 -- Bram@... -- 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 ///
                                      • John Beckett
                                        ... 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
                                        Message 19 of 25 , May 1, 2007
                                        View Source
                                        • 0 Attachment
                                          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
                                        • A.J.Mechelynck
                                          Bram Moolenaar wrote: [...] ... [...] Are you sure? In a terminal logged-in as root, using vim 7.0.235: vim -u NONE -N ... modeline modelines=5 Modelines
                                          Message 20 of 25 , May 1, 2007
                                          View Source
                                          • 0 Attachment
                                            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)
                                          • Bram Moolenaar
                                            ... 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
                                            Message 21 of 25 , May 1, 2007
                                            View Source
                                            • 0 Attachment
                                              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 -- Bram@... -- 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 ///
                                            • Matthew Winn
                                              On Tue, 1 May 2007 19:42:02 +1000, John Beckett ... What constitutes a reasonable length ? Vim has to load the entire document including its modeline into
                                              Message 22 of 25 , May 3, 2007
                                              View Source
                                              • 0 Attachment
                                                On Tue, 1 May 2007 19:42:02 +1000, "John Beckett"
                                                <winterwaffle@...> 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 tainting mechanism is
                                                designed to prevent, and that's exactly what Vim must prevent too.

                                                --
                                                Matthew Winn
                                              • John Beckett
                                                ... 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
                                                Message 23 of 25 , May 3, 2007
                                                View Source
                                                • 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).

                                                  John
                                                • Ciaran McCreesh
                                                  On Fri, 4 May 2007 14:20:22 +1000 ... Most previous exploits have been exploitable with far below the line length that is reasonably used by sensible people.
                                                  Message 24 of 25 , May 4, 2007
                                                  View Source
                                                  • 0 Attachment
                                                    On Fri, 4 May 2007 14:20:22 +1000
                                                    "John Beckett" <winterwaffle@...> 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
                                                  • John Beckett
                                                    ... 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
                                                    Message 25 of 25 , May 4, 2007
                                                    View Source
                                                    • 0 Attachment
                                                      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
                                                    Your message has been successfully submitted and would be delivered to recipients shortly.