MXit security flaws

[Andrew Victor]

hi Daniel,

Thanks for informing us.

> CID 732102: (This is the most critical one)
> * String not null terminated. In mxit_cb_http_read: A character
> buffer that has not been null terminated is passed to a function
> expecting a null terminated string
>  * libpurple/protocols/mxit/http.c:133 -> ch = strstr( buf, HTTP_11_SEPERATOR );

Definitely a valid issue that needs fixing, and as you pointed out a possible buffer overrun.


> CID 732105:
> * Copy into fixed size buffer. In mxit_encrypt_password: A source
> buffer of statically unknown size is copied into a fixed-size
> destination buffer
>  * libpurple/protocols/mxit/cipher.c:93 -> strcat( pass,
> session->acc->password );

This code has been improved and fixed in the 3.0.0 tree.
http://hg.pidgin.im/pidgin/main/diff/b9ede4a1435b/libpurple/protocols/mxit/cipher.c
http://hg.pidgin.im/pidgin/main/diff/b4729e4322f3/libpurple/protocols/mxit/cipher.c

We could possibly just use the version from the 3.0.0 tree.


> CID 732025:
> * Explicit null dereferenced. In mxit_send_extprofile_update:
> Dereference of an explicit null value
> * libpurple/protocols/mxit/protocol.c:871 -> datalen += sprintf(
> data + datalen, "%c%s%c%s%c%s",  ...

This should not be exploitable since it's only called from action.c mxit_profile_cb() which generates both the input attributes and the number of attributes.


There are a number of MXit-specific fixes in the mxit-2.x.y branch of the Mercurial repo.  Most of them are backported from the 3.0.0-dev tree.  Would you consider merging the mxit-2.x.y branch, or do you specifically only want fixes for the above issues?


Regards,
  Andrew Victor