xorl %eax, %eax

CVE-2009-1376: Pidgin MSN SLP Integer Truncation

with one comment

This vulnerability was initially disclosed as CVE-2008-2927 by some anonymous user reporting it through ZDI. This was not patched properly and they managed to fix it in 2.5.6 release. Here is the first vulnerable code from libpurple/protocols/msn/slplink.c of 2.5.5 release of pidgin:

1 /**
2  * @file slplink.c MSNSLP Link support
3  *
4  * purple
        ...
491 void
492 msn_slplink_process_msg(MsnSlpLink *slplink, MsnMessage *msg)
493 {
494         MsnSlpMessage *slpmsg;
495         const char *data;
496         gsize offset;
497         gsize len;
498
        ...
521         offset = msg->msnslp_header.offset;
        ...
591                 if (G_MAXSIZE - len < offset || (offset + len) > slpmsg->size)
        ...
598                 else
599                         memcpy(slpmsg->buffer + offset, data, len);
        ...
653 }


At line 496, variable offset is declared as of gsize data type. At line 521 it is initialized with the value of the user controlled MSN SLP header’s offset. This structure is defined at libpurple/protocols/msn/msg.h like this:

27 typedef struct _MsnMessage MsnMessage;
        ...
94 /**
95  * A message.
96  */
97 struct _MsnMessage
98 {
        ...
115         MsnSlpHeader msnslp_header;
116         MsnSlpFooter msnslp_footer;
        ...
136 };

This new data type is a structure defined in the same header file like this:

74 typedef struct
75 {
76         guint32 session_id;
77         guint32 id;
78         guint64 offset;
79         guint64 total_size;
80         guint32 length;
81         guint32 flags;
82         guint32 ack_id;
83         guint32 ack_sub_id;
84         guint64 ack_size;
85
86 } MsnSlpHeader;

It is clear that offset is defined as guint64 here. Obviously, this GLibC data type represents a 64 bit long, unsigned integer but on the other hand, gsize at line 496 represents an unsigned integer. This can clearly lead to truncation during the assignment at line 521 on systems where the size of unsigned integer is 4 bytes long. To fix this, they applied the following patch:

     const char *data;
-    gsize offset;
+    guint64 offset;
     gsize len;

They also fixed a memory leak in the same routine. In case of an SLP message with no data in its buffer they didn’t free the allocated space like this:

491 void
492 msn_slplink_process_msg(MsnSlpLink *slplink, MsnMessage *msg)
493 {
494         MsnSlpMessage *slpmsg;
         ...
513         slpmsg = NULL;
         ...
523         if (offset == 0)
524         {
525                 slpmsg = msn_slpmsg_new(slplink);
526                 slpmsg->id = msg->msnslp_header.id;
527                 slpmsg->session_id = msg->msnslp_header.session_id;
528                 slpmsg->size = msg->msnslp_header.total_size;
529                 slpmsg->flags = msg->msnslp_header.flags;
         ...
562                 if (!slpmsg->fp && slpmsg->size)
563                 {
564                         slpmsg->buffer = g_try_malloc(slpmsg->size);
565                         if (slpmsg->buffer == NULL)
566                         {
567                                 purple_debug_error("msn", "Failed to allocate buffer for slpmsg\n");
568                                 return;
569                         }
570                 }
         ...
653 }


As you can see, slpmsg is a pointer to a MsnSlpMessage structure that is set to NULL and if the offset is zero, it is initialized with the MSN SLP header information. If the allocation of the requested size at line 564 fails, it will print an error message and immediately return. However, the rest of the elements of that MSN SLP message will not be freed. This memory leak was fixed like this:

                 purple_debug_error("msn", "Failed to allocate buffer for slpmsg\n");
+                msn_slpmsg_destroy(slpmsg);
                 return;

Function msn_slpmsg_destroy() is defined in libpurple/protocols/msn/slpmsg.c and it is used to destroy/free the allocated space of the members that an SLP message has. This bug was not included in CVE-2008-2927 or CVE-2009-1376, but another similar integer truncation one was. Specifically, in CVE-2008-2927 the following bug was present at libpurple/protocols/msnp9/slplink.c. The bug is still there in 2.5.5 and it’s pretty much the same:

508 void
509 msn_slplink_process_msg(MsnSlpLink *slplink, MsnMessage *msg)
510 {
511         MsnSlpMessage *slpmsg;
512         const char *data;
513         gsize offset;
514         gsize len;
     ...
664 }

And it isn’t patched in the equivalent file of the 2.5.6 release. Correct me if I’m wrong, but if this is the case, this will be a some kind of a record of three CVE IDs to patch the same vulnerability. I hope I missed something and this is patched.

Written by xorl

May 28, 2009 at 13:45

Posted in bugs

One Response

Subscribe to comments with RSS.

  1. msnp9/ is not built by default anymore, and I don’t believe it’s used. Said file has already been patched on trunk, but we forgot to backport that to the release branch. Since the next release will be from trunk anyway, there isn’t much that I am going to do now.

    Sorry for the confusion.

    khc

    May 30, 2009 at 18:35


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