Buffer overflow in jabber prpl

[Mark Doliner]

Thanks for finding and reporting this, Thijs and Paul. Your diff looks
good to me--I just committed it to our private 2.x.y repo.

It seems crazy that the stringprep functions would read more than
length, but I believe you. I'll request a CVE for this along with the
others.

On Mon, Oct 13, 2014 at 11:29 PM, Thijs Alkemade <thijs at adium.im> wrote:
>
> On 14 okt. 2014, at 03:37, Paul Aurich <paul at darkrain42.org> wrote:
>
>> Thanks for writing this up!  The patch looks correct to me.
>>
>> On 10/13/2014 01:16 PM, Thijs Alkemade wrote:
>>> Hello,
>>>
>>> While investigating [1], darkrain and I discovered a off-by-one error in
>>> jabber_idn_validate that may lead to libpurple reading memory outside of the
>>> bounds of an array.
>>>
>>> The problem is caused by [2]. The "+ 1" here is wrong and causes domain_len to
>>> be one too few. This can lead to an incomplete UTF-8 codepoint for
>>> internationalized TLDs such as .рф, which, for unclear reasons, causes
>>> stringprep_nameprep to skip the terminating NULL byte by interpreting it as
>>> part of the codepoint. By doing this at the end of the idn_buffer, it is
>>> possible to read more than sizeof(idn_buffer) bytes, despite claims from the
>>> stringprep documentation that it will not read bytes past the maxlen.
>>
>> I'm thinking this through as I type (please bear with me):
>>
>> idn_buffer is statically-allocated; parts of the JID are copied into
>> it.  The source string itself should always be null-terminated, so even
>> with the off-by-one calculations, strncpy should never copy past the end
>> of the original string.
>>
>> So it shouldn't ever over-read from the heap-allocated source string
>> (right?)
>>
>> As demonstrated in the ticket, or based on your analysis, it is possible
>> either to read data previously-normalized, or potentially beyond the
>> static buffer (to whatever has been allocated there).
>>
>> ~Paul
>
> Correct, it would overread into whatever static data happens to be placed
> right after idn_buffer. I'm attaching a small test case that demonstrates the
> problem. All function calls are the same as in libpurple, but the buffer is
> only 64 bytes.
>
> Executing it results in:
>
> $ ./a.out
> (65) bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbрф
> (59) bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbррaaa
>
> The "aaa" are copied from another static array that happens to be right after
> the buffer.
>
> (I'm also tempted to call it a bug in libidn. The pre-conditions of
> stringprep() aren't satisfied (the input is not an UTF-8 encoded zero-
> terminated string), but the claim that "This function will not read or write
> to characters outside that size." is simply wrong.)
>
> [1] https://www.gnu.org/software/libidn/reference/libidn-stringprep.html#stringprep
>
> Regards,
> Thijs
>
>
>
>
>
> _______________________________________________
> security mailing list
> security at pidgin.im
> https://pidgin.im/cgi-bin/mailman/listinfo/security