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

SEGV in msg_may_trunc()

Expand Messages
  • Pawel S. Veselov
    Hi, VIM (6.3.85p0) on openBSD 3.8, built from /usr/ports. in message.c there is a probable SEGV in msg_may_trunc() function. If multibyte string is passed in,
    Message 1 of 5 , Dec 21, 2005
    • 0 Attachment
      Hi,

      VIM (6.3.85p0) on openBSD 3.8, built from /usr/ports.

      in message.c there is a probable SEGV in msg_may_trunc() function.
      If multibyte string is passed in, and the size of the string in characters
      is less than room, but size in bytes is more than room, the (s-1) address
      is then written to, as (n) becomes -1.

      The attached patch should help. Should work on 6.4 as well.

      What I still don't understand is how it is OK to replace some position
      in asciiz string with '>'. Does anything guarantee that the position the
      '>' is written to is not a part of a multibyte character ?

      Thanks,
      Pawel.


      Bye.
      --
      Pawel S. Veselov [vps], Sun Microsystems, Inc.
      Staff Engineer, Java Mobile Systems and Services Engineering __ __(O) _ __
      (408) 276-5410 e-mail: Pawel.Veselov@... \ V /| || ' \
      fax(408) 276-6090 HomePage: http://manticore.2y.net \_/ |_||_|_|_|
    • Bram Moolenaar
      ... Strange that this has gone unnoticed so long. It s probably because it only happens when using IObuff as the buffer, which is never freed. Then using one
      Message 2 of 5 , Dec 21, 2005
      • 0 Attachment
        Pawel S. Veselov wrote:

        > VIM (6.3.85p0) on openBSD 3.8, built from /usr/ports.
        >
        > in message.c there is a probable SEGV in msg_may_trunc() function.
        > If multibyte string is passed in, and the size of the string in characters
        > is less than room, but size in bytes is more than room, the (s-1) address
        > is then written to, as (n) becomes -1.
        >
        > The attached patch should help. Should work on 6.4 as well.
        >
        > What I still don't understand is how it is OK to replace some position
        > in asciiz string with '>'. Does anything guarantee that the position the
        > '>' is written to is not a part of a multibyte character ?

        Strange that this has gone unnoticed so long. It's probably because it
        only happens when using IObuff as the buffer, which is never freed.
        Then using one byte before the buffer writes in the length of the
        buffer, and if you don't free it that is not causing trouble. But
        of course it's bad anyway, just an explanation why we haven't seen
        crashes all around.

        A simpler solution is to check for "n" being negative and not doing
        anything then. Like this:

        Index: message.c
        ===================================================================
        RCS file: /cvsroot/vim/vim7/src/message.c,v
        retrieving revision 1.32
        diff -u -r1.32 message.c
        --- message.c 3 Oct 2005 21:50:54 -0000 1.32
        +++ message.c 21 Dec 2005 21:40:36 -0000
        @@ -727,6 +727,10 @@
        size -= (*mb_ptr2cells)(s + n);
        n += (*mb_ptr2len)(s + n);
        }
        +
        + /* there may be room anyway when there are multi-byte chars */
        + if (n == 0)
        + return s;
        --n;
        }
        #endif

        --
        It might look like I'm doing nothing, but at the cellular level
        I'm really quite busy.

        /// 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://www.ICCF.nl ///
      • Pawel S. Veselov
        Bram, this will work, but it will still count the length of the string twice which I tried to avoid... Still, what about the case when the is placed into
        Message 3 of 5 , Dec 21, 2005
        • 0 Attachment
          Bram,

          this will work, but it will still count the length of the string twice
          which I tried to avoid...

          Still, what about the case when the '>' is placed into one of the bytes
          of a multibyte character ?

          I guess it was never noticed before, as usually the bytes just before
          a malloc'ed address belong to the malloc() system data, which isn't true on
          BSD. And how many people use variable multibyte output locale on openBSD :)

          Thanks,
          Pawel.

          On Wed, Dec 21, 2005 at 10:55:21PM +0100, Bram Moolenaar wrote:
          >> VIM (6.3.85p0) on openBSD 3.8, built from /usr/ports.
          >>
          >> in message.c there is a probable SEGV in msg_may_trunc() function.
          >> If multibyte string is passed in, and the size of the string in characters
          >> is less than room, but size in bytes is more than room, the (s-1) address
          >> is then written to, as (n) becomes -1.
          >>
          >> The attached patch should help. Should work on 6.4 as well.
          >>
          >> What I still don't understand is how it is OK to replace some position
          >> in asciiz string with '>'. Does anything guarantee that the position the
          >> '>' is written to is not a part of a multibyte character ?
          >
          >Strange that this has gone unnoticed so long. It's probably because it
          >only happens when using IObuff as the buffer, which is never freed.
          >Then using one byte before the buffer writes in the length of the
          >buffer, and if you don't free it that is not causing trouble. But
          >of course it's bad anyway, just an explanation why we haven't seen
          >crashes all around.
          >
          >A simpler solution is to check for "n" being negative and not doing
          >anything then. Like this:
          >
          >Index: message.c
          >===================================================================
          >RCS file: /cvsroot/vim/vim7/src/message.c,v
          >retrieving revision 1.32
          >diff -u -r1.32 message.c
          >--- message.c 3 Oct 2005 21:50:54 -0000 1.32
          >+++ message.c 21 Dec 2005 21:40:36 -0000
          >@@ -727,6 +727,10 @@
          > size -= (*mb_ptr2cells)(s + n);
          > n += (*mb_ptr2len)(s + n);
          > }
          >+
          >+ /* there may be room anyway when there are multi-byte chars */
          >+ if (n == 0)
          >+ return s;
          > --n;
          > }
          > #endif

          [ skipped ]
        • Pawel S. Veselov
          ... I m sorry, I get it now. Thanks, Pawel. [ skipped ]
          Message 4 of 5 , Dec 21, 2005
          • 0 Attachment
            >Still, what about the case when the '>' is placed into one of the bytes
            >of a multibyte character ?

            I'm sorry, I get it now.

            Thanks,
            Pawel.

            [ skipped ]
          • Bram Moolenaar
            ... The first time it s counted with strlen(), which is really fast. If it fits in the room when counting bytes, then it will also fit when counting
            Message 5 of 5 , Dec 22, 2005
            • 0 Attachment
              Pawel S. Veselov wrote:

              > this will work, but it will still count the length of the string twice
              > which I tried to avoid...

              The first time it's counted with strlen(), which is really fast. If it
              fits in the room when counting bytes, then it will also fit when
              counting characters (there may be more bytes than characters, but not
              the other way around). That avoids calling the slow vim_strsize() for
              every string.

              I do notice that the change I made does put in a ">" when the string
              just fits. I'll change that.

              > Still, what about the case when the '>' is placed into one of the bytes
              > of a multibyte character ?

              It is placed in the last byte of a multibyte character and then the
              pointer is adjusted to start displaying the '>'. That will always work.
              For example, if the message has "aaa123bbbb", where "123" is a multibyte
              character, then it may become "aaa12>bbbb" and ">bbbb" is displayed.

              --
              "I've been teaching myself to play the piano for about 5 years and now write
              most of my songs on it, mainly because I can never find any paper."
              Jeff Lynne, ELO's greatest hits

              /// 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://www.ICCF.nl ///
            Your message has been successfully submitted and would be delivered to recipients shortly.