# HG changeset patch # User Mark Doliner # Date 1323153623 0 # Node ID 82024b6ea4650fc37548903f329b19726a86b9de # Parent 58d4a721f0c085851e9553f0760286e5966cbeef 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. diff -r 58d4a721f0c0 -r 82024b6ea465 ChangeLog --- 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) diff -r 58d4a721f0c0 -r 82024b6ea465 libpurple/protocols/oscar/family_feedbag.c --- 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);