Possible bugs

[Eric Wong]

Hello,

I’ve been reviewing the code for Pidgin 2.10.10 as part of an application security class, and may have found a couple of bugs.  I do not have exploits or any debugging information—this is strictly from review.

The first possible bug is in libpurple/protocols/gg/lib/events.c.  Here, I think a crafted packet might be able to cause a program crash (at worst).  The variable ‘host' is defined on line 974 as a 128 character array.  However, when it is used in the sscanf function on line 1251, the format string specifies 128 characters to write into this array.  I think this means that if the received string is long enough, since it comes from the socket, the null termination byte is stored in bit posting 129, causing a buffer overflow.  The fix is to change the length of ‘host’ or change the format string to %127s.

The second possible bug is in libpurple/protocols/mxit/http.c and protocol.c.  Here, I think a crafted packet might be able to cause a program crash (at worst).  The function mxit_cb_http_read declares the variable ‘bodylen' as an integer on line 102 and assigns the HTTP content length via the atoi function on line 185.  Since atoi returns an unsigned int, a packet that has a small negative (between -254 and -1) or large positive content length string (4294967042 to 4294967295) will result in a negative number (between -254 and -1) stored in ‘bodylen’.  This small negative value in ‘bodylen’ allows the function to execute through to line 238, where it calls mx_parse_packet.  Along the way, on line 206, it assigns ‘bodylen’ to 'session->rx_i’.  ‘session->rx_i’ is an unsigned int, so it converts this negative number to a large positive number.  When execution gets to mxit_parse_packet in protocol.c, it can execute the nested while-loops until it gets to set the ‘pbreak’ variable to false, where it reads from ’session->rx_dbuf’.  Getting out of the nested while-loops may take some time, but along the way memory is being allocated and may cause a crash (if given the right circumstances).

Thanks,
Eric