Crash in Pidgin and Adium on XMPP MUC Presence

[Elliott Sales de Andrade]

On Mon, Feb 6, 2012 at 2:07 PM, Kevin Stange <kevin at steadfast.net> wrote:

> A coworker of mine has caused a remotely triggerable crash (he can crash
> everyone in the office) when joining a MUC using empathy.  I'm not
> familiar enough with XMPP to know if this might be the XMPP server's
> doing instead of the client's.  I haven't been able to find the steps to
> recreate a configuration that causes the situation.
>
> I was not able to determine why a rename presence is being sent
> initially, but it is the first thing sent to the users in the MUC and
> Pidgin and Adium dereference a null pointer when trying to update the
> user list within the UI.  Within libpurple, conversation.c seems to
> handle this anomaly correctly.
>
> The initial presence message looks like this:
>
> <presence from="chitchat at conference.steadfast.net/brad"
> to="kevin at steadfast.net/Workstation" type="unavailable"><x
> xmlns="vcard-temp:x:update"><photo/></x><x
> xmlns="http://jabber.org/protocol/muc#user"><item
> jid="brad at steadfast.net/b74de8f7" affiliation="member"
> role="participant" nick="brad at steadfast.net"/><status
> code="303"/></x></presence>
>
> I've attached an Adium and Pidgin backtrace each showing effectively the
> same issue.  It seems like get_iter_from_chatbuddy could be adjusted to
> check if cb is NULL before dereferencing it as cb->ui_data and return
> FALSE.
>
>
On i.p.p, this has been replaced with a call to
purple_conv_chat_cb_get_ui_data due to struct hiding. This function has a
NULL-check, so it won't crash.


> Alternatively, the check for if(!old_cbuddy) could be moved up in
> pidgin_conv_chat_rename_user() to before attempting to complete the rename.
>
>
This change has also already been made on i.p.p. See ticket #14392. The
existence of this ticket may or may not increase the severity of this
issue. Since it was not disclosing an actual exploit, it may not change
things.


> Someone else should verify which of these fixes would be appropriate.
>
>
The simplest change is the second one; the first is conveniently handled by
struct hiding, but I don't know how much of that has appeared on the 2.10.x
branch.


> --
> Kevin Stange
> Chief Technology Officer
> Steadfast Networks
> http://steadfast.net
> Phone: 312-602-2689 ext. 203 | Fax: 312-602-2688 | Cell: 312-320-5867
>
>
-- 
Elliott aka QuLogic
Pidgin developer
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://pidgin.im/cgi-bin/mailman/private/security/attachments/20120206/1950576b/attachment.html>