[patch] fixed bug in free_crypt_key()

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[patch] fixed bug in free_crypt_key()

Dominique Pellé
Hi

Using Vim-7.3a (2199:d0ddf7ba1630), I see the following
error with Valgrind memory checker:

==20219== Conditional jump or move depends on uninitialised value(s)
==20219==    at 0x8117C52: free_crypt_key (misc2.c:3794)
==20219==    by 0x8117D7E: get_crypt_key (misc2.c:3843)
==20219==    by 0x80B466C: ex_X (ex_docmd.c:11127)
==20219==    by 0x80A7864: do_one_cmd (ex_docmd.c:2629)
==20219==    by 0x80A513D: do_cmdline (ex_docmd.c:1098)
==20219==    by 0x812A99E: nv_colon (normal.c:5226)
==20219==    by 0x812421F: normal_cmd (normal.c:1188)
==20219==    by 0x80E75E2: main_loop (main.c:1208)
==20219==    by 0x80E70D9: main (main.c:952)
==20219==  Uninitialised value was created by a heap allocation
==20219==    at 0x4024F70: malloc (vg_replace_malloc.c:236)
==20219==    by 0x8114BFC: lalloc (misc2.c:919)
==20219==    by 0x8114B06: alloc (misc2.c:818)
==20219==    by 0x80BB110: alloc_cmdbuff (ex_getln.c:2509)
==20219==    by 0x80B76DB: getcmdline (ex_getln.c:229)
==20219==    by 0x80BA5FC: getcmdline_prompt (ex_getln.c:1961)
==20219==    by 0x8117CD0: get_crypt_key (misc2.c:3819)
==20219==    by 0x80B466C: ex_X (ex_docmd.c:11127)
==20219==    by 0x80A7864: do_one_cmd (ex_docmd.c:2629)
==20219==    by 0x80A513D: do_cmdline (ex_docmd.c:1098)
==20219==    by 0x812A99E: nv_colon (normal.c:5226)
==20219==    by 0x812421F: normal_cmd (normal.c:1188)
==20219==    by 0x80E75E2: main_loop (main.c:1208)
==20219==    by 0x80E70D9: main (main.c:952)
(more errors after that)

Steps to reproduce:

1) Run:
   valgrind --num-callers=50 --track-origins=yes 2> vg.log vim -u NONE
2) Type Ex command  :X
3) Enter an encryption key which contains an _odd_ number of char (ex: "123")
4) Observe Valgrind errors in vg.log

misc2.c:

3786     void
3787 free_crypt_key(key)
3788     char_u *key;
3789 {
3790     char_u *p;
3791
3792     if (key != NULL)
3793     {
3794         for (p = key; *p != NUL; ++p)
3795             *p++ = 0;
3796         vim_free(key);
3797     }
3798 }

free_crypt_key() is clearing only half the characters since 'p' is
incremented twice per iteration. When key has odd number of characters,
memory is read beyond allocated memory and may also be cleared
beyond the allocated memory.

Attached patch fixes it.

Regards
-- Dominique

--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

fixed-bug-free_crypt_key-7.3a.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [patch] fixed bug in free_crypt_key()

Bram Moolenaar

Dominique Pelle wrote:

> Using Vim-7.3a (2199:d0ddf7ba1630), I see the following
> error with Valgrind memory checker:
>
> ==20219== Conditional jump or move depends on uninitialised value(s)
> ==20219==    at 0x8117C52: free_crypt_key (misc2.c:3794)
> ==20219==    by 0x8117D7E: get_crypt_key (misc2.c:3843)
> ==20219==    by 0x80B466C: ex_X (ex_docmd.c:11127)
> ==20219==    by 0x80A7864: do_one_cmd (ex_docmd.c:2629)
> ==20219==    by 0x80A513D: do_cmdline (ex_docmd.c:1098)
> ==20219==    by 0x812A99E: nv_colon (normal.c:5226)
> ==20219==    by 0x812421F: normal_cmd (normal.c:1188)
> ==20219==    by 0x80E75E2: main_loop (main.c:1208)
> ==20219==    by 0x80E70D9: main (main.c:952)
> ==20219==  Uninitialised value was created by a heap allocation
> ==20219==    at 0x4024F70: malloc (vg_replace_malloc.c:236)
> ==20219==    by 0x8114BFC: lalloc (misc2.c:919)
> ==20219==    by 0x8114B06: alloc (misc2.c:818)
> ==20219==    by 0x80BB110: alloc_cmdbuff (ex_getln.c:2509)
> ==20219==    by 0x80B76DB: getcmdline (ex_getln.c:229)
> ==20219==    by 0x80BA5FC: getcmdline_prompt (ex_getln.c:1961)
> ==20219==    by 0x8117CD0: get_crypt_key (misc2.c:3819)
> ==20219==    by 0x80B466C: ex_X (ex_docmd.c:11127)
> ==20219==    by 0x80A7864: do_one_cmd (ex_docmd.c:2629)
> ==20219==    by 0x80A513D: do_cmdline (ex_docmd.c:1098)
> ==20219==    by 0x812A99E: nv_colon (normal.c:5226)
> ==20219==    by 0x812421F: normal_cmd (normal.c:1188)
> ==20219==    by 0x80E75E2: main_loop (main.c:1208)
> ==20219==    by 0x80E70D9: main (main.c:952)
> (more errors after that)
>
> Steps to reproduce:
>
> 1) Run:
>    valgrind --num-callers=50 --track-origins=yes 2> vg.log vim -u NONE
> 2) Type Ex command  :X
> 3) Enter an encryption key which contains an _odd_ number of char (ex: "123")
> 4) Observe Valgrind errors in vg.log
>
> misc2.c:
>
> 3786     void
> 3787 free_crypt_key(key)
> 3788     char_u *key;
> 3789 {
> 3790     char_u *p;
> 3791
> 3792     if (key != NULL)
> 3793     {
> 3794         for (p = key; *p != NUL; ++p)
> 3795             *p++ = 0;
> 3796         vim_free(key);
> 3797     }
> 3798 }
>
> free_crypt_key() is clearing only half the characters since 'p' is
> incremented twice per iteration. When key has odd number of characters,
> memory is read beyond allocated memory and may also be cleared
> beyond the allocated memory.
>
> Attached patch fixes it.

Thanks, I'll include it.

--
Where do you want to crash today?

 /// Bram Moolenaar -- [hidden email] -- 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.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
Reply | Threaded
Open this post in threaded view
|

Re: [patch] fixed bug in free_crypt_key()

Mosh-3
In reply to this post by Dominique Pellé
Thanks Dominque.

2010/5/22 Dominique Pellé <[hidden email]>:

> Hi
>
> Using Vim-7.3a (2199:d0ddf7ba1630), I see the following
> error with Valgrind memory checker:
>
> ==20219== Conditional jump or move depends on uninitialised value(s)
> ==20219==    at 0x8117C52: free_crypt_key (misc2.c:3794)
> ==20219==    by 0x8117D7E: get_crypt_key (misc2.c:3843)
> ==20219==    by 0x80B466C: ex_X (ex_docmd.c:11127)
> ==20219==    by 0x80A7864: do_one_cmd (ex_docmd.c:2629)
> ==20219==    by 0x80A513D: do_cmdline (ex_docmd.c:1098)
> ==20219==    by 0x812A99E: nv_colon (normal.c:5226)
> ==20219==    by 0x812421F: normal_cmd (normal.c:1188)
> ==20219==    by 0x80E75E2: main_loop (main.c:1208)
> ==20219==    by 0x80E70D9: main (main.c:952)
> ==20219==  Uninitialised value was created by a heap allocation
> ==20219==    at 0x4024F70: malloc (vg_replace_malloc.c:236)
> ==20219==    by 0x8114BFC: lalloc (misc2.c:919)
> ==20219==    by 0x8114B06: alloc (misc2.c:818)
> ==20219==    by 0x80BB110: alloc_cmdbuff (ex_getln.c:2509)
> ==20219==    by 0x80B76DB: getcmdline (ex_getln.c:229)
> ==20219==    by 0x80BA5FC: getcmdline_prompt (ex_getln.c:1961)
> ==20219==    by 0x8117CD0: get_crypt_key (misc2.c:3819)
> ==20219==    by 0x80B466C: ex_X (ex_docmd.c:11127)
> ==20219==    by 0x80A7864: do_one_cmd (ex_docmd.c:2629)
> ==20219==    by 0x80A513D: do_cmdline (ex_docmd.c:1098)
> ==20219==    by 0x812A99E: nv_colon (normal.c:5226)
> ==20219==    by 0x812421F: normal_cmd (normal.c:1188)
> ==20219==    by 0x80E75E2: main_loop (main.c:1208)
> ==20219==    by 0x80E70D9: main (main.c:952)
> (more errors after that)
>
> Steps to reproduce:
>
> 1) Run:
>   valgrind --num-callers=50 --track-origins=yes 2> vg.log vim -u NONE
> 2) Type Ex command  :X
> 3) Enter an encryption key which contains an _odd_ number of char (ex: "123")
> 4) Observe Valgrind errors in vg.log
>
> misc2.c:
>
> 3786     void
> 3787 free_crypt_key(key)
> 3788     char_u *key;
> 3789 {
> 3790     char_u *p;
> 3791
> 3792     if (key != NULL)
> 3793     {
> 3794         for (p = key; *p != NUL; ++p)
> 3795             *p++ = 0;
> 3796         vim_free(key);
> 3797     }
> 3798 }
>
> free_crypt_key() is clearing only half the characters since 'p' is
> incremented twice per iteration. When key has odd number of characters,
> memory is read beyond allocated memory and may also be cleared
> beyond the allocated memory.
>
> Attached patch fixes it.
>
> Regards
> -- Dominique
>
> --
> You received this message from the "vim_dev" maillist.
> Do not top-post! Type your reply below the text you are replying to.
> For more information, visit http://www.vim.org/maillist.php
>

--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php