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

patch to fix buffer overflow in vim-7.1.148

Expand Messages
  • Dominique Pelle
    Valgrind memory checker detects the following buffer overflow in vim-7.1.148. ==7107== Invalid write of size 1 ==7107== at 0x4023F0A: strncat
    Message 1 of 2 , Nov 7, 2007
    • 0 Attachment
      Valgrind memory checker detects the following buffer overflow in vim-7.1.148.

      ==7107== Invalid write of size 1
      ==7107== at 0x4023F0A: strncat (mc_replace_strmem.c:218)
      ==7107== by 0x80A5240: do_one_cmd (ex_docmd.c:2661)
      ==7107== by 0x80A28DE: do_cmdline (ex_docmd.c:1099)
      ==7107== by 0x80A1F90: do_cmdline_cmd (ex_docmd.c:705)
      ==7107== by 0x80E720C: exe_commands (main.c:2663)
      ==7107== by 0x80E4C06: main (main.c:875)
      ==7107== Address 0x4AD4809 is 0 bytes after a block of size 1,025 alloc'd
      ==7107== at 0x4022765: malloc (vg_replace_malloc.c:149)
      ==7107== by 0x81127B4: lalloc (misc2.c:857)
      ==7107== by 0x81126D6: alloc (misc2.c:756)
      ==7107== by 0x80E44B3: main (main.c:246)

      The code where write overflow happens is:

      ex_docmd.c:

      2661 STRNCAT(errormsg, *cmdlinep, IOSIZE - STRLEN(IObuff));

      STRNCAT(...) is a wrapper to strncat(...).

      IOSIZE is defined in vim.h:

      #define IOSIZE (1024+1) /* file i/o and sprintf buffer size */

      So IOSIZE is 1025, which corresponds to what valgrind
      reports (1,025 allocated bytes)

      The overflow happens here because strncat(s1, s2, n)
      concatenates n characters + the NUL end of string
      (so it actually writes n + 1 bytes! That is it can write
      here IOSIZE + 1 bytes => hence 1 byte write overflow.

      I attach a simple patch which fixes the problem.

      I can reproduce this issue by injecting some random
      garbage into the -c command line option of vim (which
      may be a bit silly but it was just to stress vim and find
      that kind of bugs):

      $ valgrind vim -c "$(cat garbage-random-binary-file)" 2> vg.log

      Doing a "grep STRNCAT" suggests that there is probably
      the same kind of bug in a couple of other places (at least
      STRNCAT(...) seems to be used in a similar incorrect way
      in ex_cmds.c).

      I am using vim-7.1.148 on Linux, built with -O0 (no
      optimization) and with -DEXITFREE, configured
      with "./configure --with-features=huge".

      -- Dominique

      --~--~---------~--~----~------------~-------~--~----~
      You received this message from the "vim_dev" maillist.
      For more information, visit http://www.vim.org/maillist.php
      -~----------~----~----~----~------~----~------~--~---
    • Bram Moolenaar
      ... Thanks for reporthing this. I ll look into it. -- From know your smileys : 8-O Omigod!! (done rm -rf * ?) /// Bram Moolenaar -- Bram@Moolenaar.net
      Message 2 of 2 , Nov 8, 2007
      • 0 Attachment
        Dominique Pelle wrote:

        > Valgrind memory checker detects the following buffer overflow in vim-7.1.148.
        >
        > ==7107== Invalid write of size 1
        > ==7107== at 0x4023F0A: strncat (mc_replace_strmem.c:218)
        > ==7107== by 0x80A5240: do_one_cmd (ex_docmd.c:2661)
        > ==7107== by 0x80A28DE: do_cmdline (ex_docmd.c:1099)
        > ==7107== by 0x80A1F90: do_cmdline_cmd (ex_docmd.c:705)
        > ==7107== by 0x80E720C: exe_commands (main.c:2663)
        > ==7107== by 0x80E4C06: main (main.c:875)
        > ==7107== Address 0x4AD4809 is 0 bytes after a block of size 1,025 alloc'd
        > ==7107== at 0x4022765: malloc (vg_replace_malloc.c:149)
        > ==7107== by 0x81127B4: lalloc (misc2.c:857)
        > ==7107== by 0x81126D6: alloc (misc2.c:756)
        > ==7107== by 0x80E44B3: main (main.c:246)
        >
        > The code where write overflow happens is:
        >
        > ex_docmd.c:
        >
        > 2661 STRNCAT(errormsg, *cmdlinep, IOSIZE - STRLEN(IObuff));
        >
        > STRNCAT(...) is a wrapper to strncat(...).
        >
        > IOSIZE is defined in vim.h:
        >
        > #define IOSIZE (1024+1) /* file i/o and sprintf buffer size */
        >
        > So IOSIZE is 1025, which corresponds to what valgrind
        > reports (1,025 allocated bytes)
        >
        > The overflow happens here because strncat(s1, s2, n)
        > concatenates n characters + the NUL end of string
        > (so it actually writes n + 1 bytes! That is it can write
        > here IOSIZE + 1 bytes => hence 1 byte write overflow.
        >
        > I attach a simple patch which fixes the problem.
        >
        > I can reproduce this issue by injecting some random
        > garbage into the -c command line option of vim (which
        > may be a bit silly but it was just to stress vim and find
        > that kind of bugs):
        >
        > $ valgrind vim -c "$(cat garbage-random-binary-file)" 2> vg.log
        >
        > Doing a "grep STRNCAT" suggests that there is probably
        > the same kind of bug in a couple of other places (at least
        > STRNCAT(...) seems to be used in a similar incorrect way
        > in ex_cmds.c).
        >
        > I am using vim-7.1.148 on Linux, built with -O0 (no
        > optimization) and with -DEXITFREE, configured
        > with "./configure --with-features=huge".

        Thanks for reporthing this. I'll look into it.

        --
        From "know your smileys":
        8-O "Omigod!!" (done "rm -rf *" ?)

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

        --~--~---------~--~----~------------~-------~--~----~
        You received this message from the "vim_dev" maillist.
        For more information, visit http://www.vim.org/maillist.php
        -~----------~----~----~----~------~----~------~--~---
      Your message has been successfully submitted and would be delivered to recipients shortly.