xorl %eax, %eax

CVE-2010-4528: Pidgin MSN Remote NULL Pointer Dereference

leave a comment »

This bug was reported by Stu Tomlinson and it affects 2.7.6 through 2.7.8 versions of the popular Instant Messaging client application. The buggy code resides at libpurple/protocols/msn/directconn.c where the code dealing with direct connections of the MSN protocol is.

static int
msn_dc_process_packet(MsnDirectConn *dc, guint32 packet_length)
{
        MsnSlpMessagePart *part;

        g_return_val_if_fail(dc != NULL, DC_PROCESS_ERROR);

        switch (dc->state) {
        case DC_STATE_CLOSED:
                break;

        case DC_STATE_FOO:
                /* FOO message is always 4 bytes long */
                if (packet_length != 4 || memcmp(dc->in_buffer, "\4\0\0\0foo", 8) != 0)
                        return DC_PROCESS_FALLBACK;

                dc->state = DC_STATE_HANDSHAKE;
                break;
      ...
        case DC_STATE_ESTABLISHED:

                if (dc->header.length) {
                        part = msn_slpmsgpart_new_from_data(dc->in_buffer + 4, dc->header.length);
			msn_slplink_process_msg(dc->slplink, part);
			msn_slpmsgpart_unref(part);
                }
      ...
        return DC_PROCESS_OK;
}

The above routine is used to process an MSN packet from a direct connection. If it’s dealing with an established direct connection (case ‘DC_STATE_ESTABLISHED’), it will check that the received header is non-zero and attempt to obtain part of the newly received data using msn_slpmsgpart_new_from_data() function located at libpurple/protocols/msn/slpmsg_part.c.

MsnSlpMessagePart *msn_slpmsgpart_new_from_data(const char *data, size_t data_len)
{
        MsnSlpMessagePart *part;
        int body_len;

        if (data_len < P2P_PACKET_HEADER_SIZE) {
                return NULL;
        }
      ...
        return part;
}

This is defined at libpurple/protocols/msn/p2p.h like this:

#define P2P_PACKET_HEADER_SIZE (6 * 4 + 3 * 8)

And it means that an MSN packet with header size less than 48 will result in returning NULL. Back to msn_dc_process_packet() we can see that the returned pointer ‘part’ will be passed to msn_slplink_process_msg() and msn_slpmsgpart_unref() without any checks being performed. Consequently, any access inside those two routines will result in NULL pointer dereference.
To fix this, the following patch was applied.

 		if (dc->header.length) {
 			part = msn_slpmsgpart_new_from_data(dc->in_buffer + 4, dc->header.length);
-			msn_slplink_process_msg(dc->slplink, part);
-			msn_slpmsgpart_unref(part);
+			if (part) {
+				msn_slplink_process_msg(dc->slplink, part);
+				msn_slpmsgpart_unref(part);
+			}
 		}

Which checks for NULL before passing the ‘part’ pointer as well as in msn_dc_recv_cb() which is the routine that handles in the incoming data while being in a direct connection state.

-		if (dc->state != DC_STATE_FOO) {
+		if (dc->state != DC_STATE_FOO && packet_length >= P2P_PACKET_HEADER_SIZE) {
 			MsnP2PHeader *context;

This was changed to make sure that only packets with valid length will be received.

Written by xorl

January 5, 2011 at 17:33

Posted in bugs

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s