Mailing List Archive

Re: Tracking Heap Corruption bug FOUND!
David Hinkle wrote:

> Maybe fixing this will become trivial for someone with more experience

> First invalid write:
>
> ==30539== Invalid write of size 1
> ==30539== at 0x401BD48: memcpy (in
> /usr/local/lib/valgrind/x86-linux/vgpreload_memcheck.so)
> ==30539== by 0x4064E74: SPF_dns_resolv_lookup (spf_dns_resolv.c:404)

Hmm, I wonder if it would help to change src/libspf2/spf_dns_resolv.c
line 399 from

while ( rdlen > 0 )

to

while ( rdlen > 1 )

I need to check it when I have time...

Actually, I would *very* much like to have a check added after line 401,
something like

if (len >= rdlen) {
complan_very_very_loudly();
len=rdlen-1;
}

Eugene

-------------------------------------------
-----------------------------------------------------------------------
To unsubscribe, change your address, or temporarily deactivate your
subscription,
please go to http://v2.listbox.com/member/?member_id=1311533&id_secret=57044523-85f1f8
Powered by Listbox: http://www.listbox.com
Re: Tracking Heap Corruption bug FOUND! [ In reply to ]
David Hinkle wrote:

> Maybe fixing this will become trivial for someone with more experience

> First invalid write:
>
> ==30539== Invalid write of size 1
> ==30539== at 0x401BD48: memcpy (in
> /usr/local/lib/valgrind/x86-linux/vgpreload_memcheck.so)
> ==30539== by 0x4064E74: SPF_dns_resolv_lookup (spf_dns_resolv.c:404)

My theory here is that in the file src/libspf2/spf_dns_resolv.c after
the line 387, the size of the data in the parsed RR (variable 'rdlen')
may under some circumstances be larger than the sum of lengths of text
strings inside. Internal text strings are concatenated in the while loop
at line 399, but if there is some junk after the last (or only) string,
or if the length prefix of some string is set incorrectly, then the code
may try to copy data from beyond the end of 'rdata' field. This is
because the value of the length prefix (len = *src; line 401) is not
checked for "sanity". Here sanity is being more than zero, and less then
the remainder of rdata. I added this check; I will report if it helps.
Usually, I get a few segfaults a week, so that will take a while.

Eugene

-------------------------------------------
-----------------------------------------------------------------------
To unsubscribe, change your address, or temporarily deactivate your
subscription,
please go to http://v2.listbox.com/member/?member_id=1311533&id_secret=57176749-42cedb
Powered by Listbox: http://www.listbox.com
RE: Tracking Heap Corruption bug FOUND! [ In reply to ]
Eugene,

If it's any help you'll see that it was always a particular spammers SPF records that crashed on the bug in my bug reports. If you go back through the archive a bit you should see me mention some of the domains he runs. I would get crashes from his domains very, very consistently, but not 100% of the time. More like 80-90%.

David


-----Original Message-----
From: Eugene Crosser [mailto:crosser@average.org]
Sent: Wed 10/24/2007 2:47 PM
To: spf-devel@v2.listbox.com
Cc: wayne
Subject: Re: [spf-devel] Tracking Heap Corruption bug FOUND!

David Hinkle wrote:

> Maybe fixing this will become trivial for someone with more experience

> First invalid write:
>
> ==30539== Invalid write of size 1
> ==30539== at 0x401BD48: memcpy (in
> /usr/local/lib/valgrind/x86-linux/vgpreload_memcheck.so)
> ==30539== by 0x4064E74: SPF_dns_resolv_lookup (spf_dns_resolv.c:404)

My theory here is that in the file src/libspf2/spf_dns_resolv.c after
the line 387, the size of the data in the parsed RR (variable 'rdlen')
may under some circumstances be larger than the sum of lengths of text
strings inside. Internal text strings are concatenated in the while loop
at line 399, but if there is some junk after the last (or only) string,
or if the length prefix of some string is set incorrectly, then the code
may try to copy data from beyond the end of 'rdata' field. This is
because the value of the length prefix (len = *src; line 401) is not
checked for "sanity". Here sanity is being more than zero, and less then
the remainder of rdata. I added this check; I will report if it helps.
Usually, I get a few segfaults a week, so that will take a while.

Eugene

-------------------------------------------
-----------------------------------------------------------------------
To unsubscribe, change your address, or temporarily deactivate your
subscription,
please go to http://v2.listbox.com/member/?&
Powered by Listbox: http://www.listbox.com

-------------------------------------------
-----------------------------------------------------------------------
To unsubscribe, change your address, or temporarily deactivate your
subscription,
please go to http://v2.listbox.com/member/?member_id=1311533&id_secret=57335919-153a80
Powered by Listbox: http://www.listbox.com
Re: Tracking Heap Corruption bug FIXED! [ In reply to ]
David Hinkle wrote:

> ==30539== Invalid write of size 1
> ==30539== at 0x401BD48: memcpy (in
> /usr/local/lib/valgrind/x86-linux/vgpreload_memcheck.so)
> ==30539== by 0x4064E74: SPF_dns_resolv_lookup (spf_dns_resolv.c:404)

OK, I think that my change indeed fixes the problem. At least, I do not
see segfaults anymore. Plus, the change looks like a Right Thing (tm) to
me. Please whoever manages the package these days consider applying the
attached diff (and maybe also fixing indentation in the 'while' block).

Eugene