Potential security issue: Yahoo authorisation requests with invalid encoding

[Sulabh Mahajan]

Hi,

While looking into the issue, I find several instances of untreated
strings. Also, there are some places where the string returned by
yahoo_string_decode has not been freed, causing memory leaks.
To fix these issues I have an alternate proposal than what is currently
being followed:

Current approach:

For every packet received from Yahoo, yahoo_packet_process is called which
figures out the type of packet (status update/message/notification/..) and
through a switch case calls the appropriate function (eg
yahoo_process_notify for notifications), lets call these as action
functions. The packets are composed of several "field - value" pairs.
Ideally all the strings in the value pairs should be validated as UTF-8
using yahoo_string_decode, but the action functions processing these
packets might miss to check or leak memory by misusing yahoo_string_decode.

What I propose:

Rather than validating strings in the action functions, we should pass all
the packets through a helper function, which will make sure that the
strings are UTF-8. Later on we can get rid of then redundant checks in the
action functions.

If someone has knowledge of Y! protocol and/or finds this
approach problematic, let me know. Otherwise I will go ahead with the
proposed fix.

Regards,
Sulabh Mahajan

On Sat, Dec 29, 2012 at 7:35 PM, Sulabh Mahajan <sulabh.dev at gmail.com>wrote:

> Hi Mark,
>
> I looked at this issue earlier when there was a discussion regarding this.
> I can take a look at the code tomorrow, and fix all the untreated strings
> that I can find.
>
> Regards,
> Sulabh Mahajan
>
> On Sat, Dec 29, 2012 at 3:15 AM, Mark Doliner <mark at kingant.net> wrote:
>
>> (+to Sulabh, who has done some work on our Yahoo! protocol plugin.)
>>
>> On Mon, Sep 24, 2012 at 8:52 AM, Ethan Blanton <elb at pidgin.im> wrote:
>> > The fix for this particular crash is easy, although I'm not sure
>> > whether the incoming message should be sanitized with
>> > yahoo_string_decode or purple_utf8_salvage
>>
>> I don't know the answer, but since the code currently treats the
>> strings as utf8 it seems reasonable to use purple_utf8_salvage.
>>
>> > However, in looking through
>> > the yahoo prpl, it looks likely to me that there are a LOT of places
>> > where this is likely to be a problem.  As an example, in the very same
>> > notification messages, the incoming nickname fields are not sanitized.
>>
>> If anyone is interested in working on this in the next day or two let
>> me know, otherwise I'll take a stab at it and send out a patch for
>> review.
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://pidgin.im/cgi-bin/mailman/private/security/attachments/20121231/e47e432b/attachment.html>