changeset 32376:82024b6ea465

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.
author Mark Doliner <mark@kingant.net>
date Tue, 06 Dec 2011 06:40:23 +0000
parents 58d4a721f0c0
children 9c9df712b191
files ChangeLog libpurple/protocols/oscar/family_feedbag.c
diffstat 2 files changed, 91 insertions(+), 28 deletions(-) [+]
line wrap: on
line diff
--- 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);