Re: [gmime-devel] crash in g_mime_iconv_strndup

[Jeffrey Stedfast <fejj gnome org>]

Hi John,

On 11/12/2011 11:23 AM, John Feuerstein wrote:
> [Please keep me in CC as I'm not subscribed]
>
> Hi, I'm trying to understand a crash of DBMail[1], and it looks like the
> problem lies in libgmime. I'm using gmime-2.4.14 (Debian Squeeze). I
> realize that this version is old, however, looking up the git history of
> gmime did not reveal any follow-up changes to g_mime_iconv_strndup()
> on the 2.4 branch up until today, so this might still be a problem.
>
> The code positions referenced below:
> http://git.gnome.org/browse/gmime/tree/gmime/gmime-iconv-utils.c?h=gmime-2-4&id=GMIME_2_4_14#n102
>
> Program received signal SIGSEGV, Segmentation fault.
>
> (gdb) bt 3
> #0  0x00007ffff5aee52a in memset () from /lib/libc.so.6
> #1  0x00007ffff7957e89 in g_mime_iconv_strndup (cd=0x798730,
>      str=0x7fffd81e8b10 "Änderungen an der Artikel-Detailseite", n=38) at gmime-iconv-utils.c:161
> #2  0x00007ffff7957f30 in g_mime_iconv_strdup (cd=0x798730,
>      str=0x7fffd81e8b10 "Änderungen an der Artikel-Detailseite") at gmime-iconv-utils.c:199
> (More stack frames follow...)
>
> I could not find a problem in DBMail, it simply calls _strdup on the
> string with cd being g_mime_iconv_open("UTF-8","UTF-8"):
>
> dbmail ...
>    g_mime_iconv_strdup(cd, str)
>      g_mime_iconv_strndup (cd, str, strlen (str));
>
> Some info about the str:
>
> (gdb) frame 1
> #1  0x00007ffff7957e89 in g_mime_iconv_strndup (cd=0x798730,
>      str=0x7fffd81e8b10 "Änderungen an der Artikel-Detailseite", n=38) at gmime-iconv-utils.c:161
> 161     in gmime-iconv-utils.c
> (gdb) p str
> $64 = 0x7fffd81e8b10 "Änderungen an der Artikel-Detailseite"
> (gdb) p n
> $65 = 38
> (gdb) x/38c str
> 0x7fffd81e8b10: -61 '\303'      -124 '\204'     110 'n' 100 'd' 101 'e' 114 'r' 117 'u' 110 'n'
> 0x7fffd81e8b18: 103 'g' 101 'e' 110 'n' 32 ' '  97 'a'  110 'n' 32 ' '  100 'd'
> 0x7fffd81e8b20: 101 'e' 114 'r' 32 ' '  65 'A'  114 'r' 116 't' 105 'i' 107 'k'
> 0x7fffd81e8b28: 101 'e' 108 'l' 45 '-'  68 'D'  101 'e' 116 't' 97 'a'  105 'i'
> 0x7fffd81e8b30: 108 'l' 115 's' 101 'e' 105 'i' 116 't' 101 'e'
>
> This looks ok. In fact, I've rebuilt the package on the same host after
> patching the string into test-iconv.c:
>
>    { "\303\204nderungen an der Artikel-Detailseite", "utf-8" },
>
> The test passed, so I can't reproduce this manually.

Ugh, that sucks. That was the first thing I was going to try doing :-(

> Now, here is the interesting part that indicates a bug in
> g_mime_iconv_strndup:
>
> (gdb) frame 1
> #1  0x00007ffff7957e89 in g_mime_iconv_strndup (cd=0x798730,
>      str=0x7fffd81e8b10 "Änderungen an der Artikel-Detailseite", n=38) at gmime-iconv-utils.c:161
> 161     in gmime-iconv-utils.c
>
> n=38 is the result of strlen(str) in g_mime_iconv_strdup, which looks
> good as strlen() counts bytes up to the terminating NUL.
>
> gmime-iconv-utils.c:161 is:
>
>      /* nul-terminate the string */
>      memset (outbuf, 0, 4);
>
> Let's have a look at the locals:
>
> (gdb) info locals
> inleft = 0
> outleft = 140736817703276
> converted = 134665270
> out = 0x7fffd806a910 ""
> outbuf = 0x0
> inbuf = 0x7fffd81e8b36 ""
> outlen = 92
> errnosav = 32767
>
> The value of "outbuf" looks broken and seems to have caused the
> segfault when used in memset() above. Somehow the counters are off, too.
>
> Please let me know if you need more details.
>
> [1] http://www.dbmail.org/mantis/view.php?id=940

inleft looks reasonable

outleft looks ... wrong. is this the snapshot of the locals just before 
memset is called?

converted looks way too large, I wouldn't expect a value larger than 38 
(I'm not sure what counts as "irreversible conversion", but I think it 
reasonable that it wouldn't be larger than the number of bytes in the 
input). I guess, in the end, it likely doesn't matter what the value is 
since it's not being used.

I would expect 'out' to contain a copy (more-or-less) of the input 
string, not be ""

As you noted, outbuf shouldn't be null.

inbuf looks correct (and seems to compute correctly based on the address 
of the input string)

outlen is definitely correct

we can ignore errnosav as it shouldn't have been used.



what values do the locals have just before the iconv flush is called?

Jeff