CVE-2009-1376: Pidgin MSN SLP Integer Truncation
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.
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