pidgin/ljfisher-ssl-client-auth

Fix remotely-triggerable crashes by validating strings in a few
release-2.x.y
2011-12-06, Mark Doliner
8431da66063b
Parents 0d126855edbf
Children 9d90f89322ea
Fix remotely-triggerable crashes by validating strings in a few
messages related to buddy list management. Fixes #14682

I changed the four functions that parse incoming authorization-related
SNACs. The changes are:

- Make sure we have a buddy name and it is valid UTF-8. If not, we
drop the SNAC and log a debug message (we can't do much with an empty,
invalid or incorrect buddy name). This wasn't a part of the bug
report and I doubt it's actually a problem, but it seems like a good
idea regardless.

- If the incoming message is not valid UTF-8 then use
purple_utf8_salvage() to replace invalid bytes with question marks. I
believe this fixes the bug in question.
2 files changed, 91 insertions(+), 28 deletions(-)
  • +4 -0
    ChangeLog
  • +87 -28
    libpurple/protocols/oscar/family_feedbag.c
    • --- a/ChangeLog Wed Nov 30 01:04:16 2011 +0000
    +++ b/ChangeLog Tue Dec 06 06:40:23 2011 +0000
    @@ -4,6 +4,10 @@
    Finch:
    * Fix compilation on OpenBSD.
    + AIM and ICQ:
    + * Fix remotely-triggerable crashes by validating strings in a few
    + messages related to buddy list management. (#14682)
    +
    Bonjour:
    * IPv6 fixes (Linus Lüssing)
    --- a/libpurple/protocols/oscar/family_feedbag.c Wed Nov 30 01:04:16 2011 +0000
    +++ b/libpurple/protocols/oscar/family_feedbag.c Tue Dec 06 06:40:23 2011 +0000
    @@ -1650,18 +1650,35 @@
    int ret = 0;
    aim_rxcallback_t userfunc;
    guint16 tmp;
    - char *bn, *msg;
    + char *bn, *msg, *tmpstr;
    /* Read buddy name */
    - if ((tmp = byte_stream_get8(bs)))
    - bn = byte_stream_getstr(bs, tmp);
    - else
    - bn = NULL;
    + tmp = byte_stream_get8(bs);
    + if (!tmp) {
    + purple_debug_warning("oscar", "Dropping auth grant SNAC "
    + "because username was empty\n");
    + return 0;
    + }
    + bn = byte_stream_getstr(bs, tmp);
    + if (!g_utf8_validate(bn, -1, NULL)) {
    + purple_debug_warning("oscar", "Dropping auth grant SNAC "
    + "because the username was not valid UTF-8\n");
    + g_free(bn);
    + }
    - /* Read message (null terminated) */
    - if ((tmp = byte_stream_get16(bs)))
    + /* Read message */
    + tmp = byte_stream_get16(bs);
    + if (tmp) {
    msg = byte_stream_getstr(bs, tmp);
    - else
    + if (!g_utf8_validate(msg, -1, NULL)) {
    + /* Ugh, msg isn't UTF8. Let's salvage. */
    + purple_debug_warning("oscar", "Got non-UTF8 message in auth "
    + "grant from %s\n", bn);
    + tmpstr = purple_utf8_salvage(msg);
    + g_free(msg);
    + msg = tmpstr;
    + }
    + } else
    msg = NULL;
    /* Unknown */
    @@ -1724,18 +1741,35 @@
    int ret = 0;
    aim_rxcallback_t userfunc;
    guint16 tmp;
    - char *bn, *msg;
    + char *bn, *msg, *tmpstr;
    /* Read buddy name */
    - if ((tmp = byte_stream_get8(bs)))
    - bn = byte_stream_getstr(bs, tmp);
    - else
    - bn = NULL;
    + tmp = byte_stream_get8(bs);
    + if (!tmp) {
    + purple_debug_warning("oscar", "Dropping auth request SNAC "
    + "because username was empty\n");
    + return 0;
    + }
    + bn = byte_stream_getstr(bs, tmp);
    + if (!g_utf8_validate(bn, -1, NULL)) {
    + purple_debug_warning("oscar", "Dropping auth request SNAC "
    + "because the username was not valid UTF-8\n");
    + g_free(bn);
    + }
    - /* Read message (null terminated) */
    - if ((tmp = byte_stream_get16(bs)))
    + /* Read message */
    + tmp = byte_stream_get16(bs);
    + if (tmp) {
    msg = byte_stream_getstr(bs, tmp);
    - else
    + if (!g_utf8_validate(msg, -1, NULL)) {
    + /* Ugh, msg isn't UTF8. Let's salvage. */
    + purple_debug_warning("oscar", "Got non-UTF8 message in auth "
    + "request from %s\n", bn);
    + tmpstr = purple_utf8_salvage(msg);
    + g_free(msg);
    + msg = tmpstr;
    + }
    + } else
    msg = NULL;
    /* Unknown */
    @@ -1808,21 +1842,38 @@
    aim_rxcallback_t userfunc;
    guint16 tmp;
    guint8 reply;
    - char *bn, *msg;
    + char *bn, *msg, *tmpstr;
    /* Read buddy name */
    - if ((tmp = byte_stream_get8(bs)))
    - bn = byte_stream_getstr(bs, tmp);
    - else
    - bn = NULL;
    + tmp = byte_stream_get8(bs);
    + if (!tmp) {
    + purple_debug_warning("oscar", "Dropping auth reply SNAC "
    + "because username was empty\n");
    + return 0;
    + }
    + bn = byte_stream_getstr(bs, tmp);
    + if (!g_utf8_validate(bn, -1, NULL)) {
    + purple_debug_warning("oscar", "Dropping auth reply SNAC "
    + "because the username was not valid UTF-8\n");
    + g_free(bn);
    + }
    /* Read reply */
    reply = byte_stream_get8(bs);
    - /* Read message (null terminated) */
    - if ((tmp = byte_stream_get16(bs)))
    + /* Read message */
    + tmp = byte_stream_get16(bs);
    + if (tmp) {
    msg = byte_stream_getstr(bs, tmp);
    - else
    + if (!g_utf8_validate(msg, -1, NULL)) {
    + /* Ugh, msg isn't UTF8. Let's salvage. */
    + purple_debug_warning("oscar", "Got non-UTF8 message in auth "
    + "reply from %s\n", bn);
    + tmpstr = purple_utf8_salvage(msg);
    + g_free(msg);
    + msg = tmpstr;
    + }
    + } else
    msg = NULL;
    /* Unknown */
    @@ -1848,10 +1899,18 @@
    char *bn;
    /* Read buddy name */
    - if ((tmp = byte_stream_get8(bs)))
    - bn = byte_stream_getstr(bs, tmp);
    - else
    - bn = NULL;
    + tmp = byte_stream_get8(bs);
    + if (!tmp) {
    + purple_debug_warning("oscar", "Dropping 'you were added' SNAC "
    + "because username was empty\n");
    + return 0;
    + }
    + bn = byte_stream_getstr(bs, tmp);
    + if (!g_utf8_validate(bn, -1, NULL)) {
    + purple_debug_warning("oscar", "Dropping 'you were added' SNAC "
    + "because the username was not valid UTF-8\n");
    + g_free(bn);
    + }
    if ((userfunc = aim_callhandler(od, snac->family, snac->subtype)))
    ret = userfunc(od, conn, frame, bn);