[Fwd: Re: [Advisories] Libpurple security vulnerability CORE-2009-0727]

[Paul Aurich]

And Paul Aurich spoke on 08/01/2009 12:25 PM, saying:
> I've attached a debug log of this crashing (with no relevant changes in MSN
> from 8351342d3a30c75649de9c3d4abc7b6f45ce8f82, which is im.pidgin.pidgin
> head at mtn.pidgin.im).
> 
> I'm not familiar with MSN, but is it pertinent that they're using the base
> ID from the "reply of the first message":
> 
>         # Get the base id from the reply of the first message,..
>         base_id = sb_msnms_object['p2p_binheader'][4:8]
> 
> It looks abnormal, and the slpmsg that's found during the processing of the
> second message is the *ACK* to the first message (instead of the first
> slpmsg created, which actually does have a buffer) -- it's not visible in
> this debug log, but I verified in a second run.
> 
> Anyway, log attached
> ~Paul
> 

To prod this process along some more, I'm attaching a patch and debug log
(of receiving the second message) that, to the best of my knowledge (and
headache), fixes this (specific) issue.  I believe my initial suspicions to
be correct (the exploit is targeting the ACK of the first message, which is
a MsnSlpMessage which contains no FILE* or allocated buffer). File transfer
still works fine, so it doesn't (seem) to have any adverse impact.

Open issues (bear in mind, I don't even know what a "SLP" is):
1. send_file_cb contains the same msn_slplink_send_slpmsg without
destroying the MsnSlpMessage. That could probably be exploited... Luckily,
it looks like that requires the user to initiate a ft.

2. msn_slplink_send_ack calls msn_slplink_send_slpmsg, but
msn_slplink_process_msg calls send_ack and then send_queued_slpmsgs. Is it
OK to destroy the message before sending queued messages?

3. Why aren't MsnSlpMessage:s destroyed by the varying msn_slplink_send_*
functions? Actually...how are they destroyed? The msn_slplink_destroy()
function doesn't do anything with any slpmsgs remaining, so they must be
being freed somewhere (or I've never used my MSN account while running
Valgrind, which is way more probable).

4. There's no good reason a msg with no buffer should be allowed to get to
the memcpy. slplink_process_msg should also be changed to read:
	if (slpmsg->fp)
	{
		/* fwrite */
	}
	else if (slpmsg->size && ***slpmsg->buffer***)
	{
		/* bounds checking for oversized message and then write */
		/* memcpy */
	}
	else
	{
		/* error */
	}

5. In the above, when the bounds checking fails (and in the final else),
shouldn't the slpmsg be destroyed ?

6. A brief audit of every call to msn_slpmsg_new makes me think khc,
QuLogic, or Stu needs to look at all of those and check that the message is
destroyed when it should be. msn_slpmsg_sip_new looks like it allocates and
returns an MsnSlpMessage*, so its usages should also be looked at.

Proddingly,
~Paul
-------------- next part --------------
A non-text attachment was scrubbed...
Name: core-2009-0727.patch
Type: text/x-patch
Size: 612 bytes
Desc: not available
URL: <http://pidgin.im/cgi-bin/mailman/private/packagers/attachments/20090805/a6a5cc8e/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: no-crash.log
Type: text/x-log
Size: 5682 bytes
Desc: not available
URL: <http://pidgin.im/cgi-bin/mailman/private/packagers/attachments/20090805/a6a5cc8e/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 898 bytes
Desc: OpenPGP digital signature
URL: <http://pidgin.im/cgi-bin/mailman/private/packagers/attachments/20090805/a6a5cc8e/attachment.pgp>