pidgin.2.7.2: 2740cbe1: *** Plucked rev 8e8ff246492e45af8f8d0808...

[markdoliner at pidgin.im]

----------------------------------------------------------------------
Revision: 2740cbe13bcf23ba66c1c9240d5a45f268299fb3
Parent:   676a11535f2f8b201adc22364e79008974ded72d
Author:   markdoliner at pidgin.im
Date:     07/20/10 22:49:45
Branch:   im.pidgin.pidgin.2.7.2
URL: http://d.pidgin.im/viewmtn/revision/info/2740cbe13bcf23ba66c1c9240d5a45f268299fb3

Changelog: 

*** Plucked rev 8e8ff246492e45af8f8d0808296d6f2906794dc0 (markdoliner at pidgin.im):
This patch attempts to fix four bugs in the oscar protocol plugin that
were introduced with the X-Status code in Pidgin 2.7.0.

Problem #1 (the remotely-triggerable crash):
The crash happens when a buddy sets an xstatus message containing <desc>
but no closing </desc>, or <title> but no closing </title>.  The fix
is to check the result of strstr(closing_tag_name) and do nothing if it
is NULL.  This is CVE-2010-2528.

Problem #2:
Fixes potential incorrect parsing of the xstatus string that could result
in an incorrect message being displayed to the libpurple user.  Happens if
an xstatus message contains </desc> before <desc>, or </title> before
<title>.  The fix is to start looking for the closing tag at the end
of the beginning tag rather than at the beginning of the xstatus xml.
Probably not a security problem, but definitely a bug.

Problem #3:
Fixes potential incorrect parsing of the xstatus string that could result
in the title not being shown to the libpurple user.  Happens if the close
title tag appears after the desc tag in the xstatus xml, because we add a
null character at the beginning of the close title tag, so strstr() for
the desc tag would stop searching there.  Probably not a security problem,
but definitely a bug.

Problem #4:
Fixes potential incorrect display of the xstatus string that could result
in an incorrect message being displayed to the libpurple user.  Happens
because we reusing the 'xml' string when preparing the string for the user,
but we copy values from xml to xml.  If those values overlap with themselves
or with each other then an incorrect value could be displayed.  Probably not
a security problem, but definitely a bug.


Changes against parent 676a11535f2f8b201adc22364e79008974ded72d

  patched  libpurple/protocols/oscar/family_icbm.c

-------------- next part --------------
============================================================
--- libpurple/protocols/oscar/family_icbm.c	6bd004ad5bc6daac2ea0213e73e62d2fd949a883
+++ libpurple/protocols/oscar/family_icbm.c	1f066254ac2585fb370bcf5ddacd0299f527c606
@@ -249,9 +249,6 @@ error(OscarData *od, FlapConnection *con
 	}
 	g_free(buf);
 
-
-
-
 	g_free(snac2->data);
 	g_free(snac2);
 
@@ -2687,7 +2684,6 @@ static int clientautoresp(OscarData *od,
 	guint16 hdrlen;
 	int curpos;
 	guint16 num1, num2;
-	char *desc, *title, *temp;
 	PurpleAccount *account;
 	PurpleBuddy *buddy;
 	PurplePresence *presence;
@@ -2714,33 +2710,41 @@ static int clientautoresp(OscarData *od,
 				xml = byte_stream_getstr(bs, bs->len - curpos);
 				purple_debug_misc("oscar", "X-Status: Received XML reply\n");
 				if (xml) {
-				/* purple_debug_misc("oscar", "X-Status: XML reply: %s\n", xml); */
-					desc = strstr(xml, "&lt;desc&gt;");
-					if (desc != NULL) {
-						temp = strstr(xml, "&lt;/desc&gt;");
-						temp[0] = 0;
-						desc = desc + 12;
+					GString *xstatus;
+					char *tmp1, *tmp2;
+
+					/* purple_debug_misc("oscar", "X-Status: XML reply: %s\n", xml); */
+
+					xstatus = g_string_new(NULL);
+
+					tmp1 = strstr(xml, "&lt;title&gt;");
+					if (tmp1 != NULL) {
+						tmp1 += 13;
+						tmp2 = strstr(tmp1, "&lt;/title&gt;");
+						if (tmp2 != NULL)
+							g_string_append_len(xstatus, tmp1, tmp2 - tmp1);
 					}
-					title = strstr(xml, "&lt;title&gt;");
-					if (title != NULL) {
-						temp = strstr(xml, "&lt;/title&gt;");
-						temp[0] = 0;
-						title = title + 13;
-					} else {
-						title = "";
+					tmp1 = strstr(xml, "&lt;desc&gt;");
+					if (tmp1 != NULL) {
+						tmp1 += 12;
+						tmp2 = strstr(tmp1, "&lt;/desc&gt;");
+						if (tmp2 != NULL) {
+							if (xstatus->len > 0)
+								g_string_append(xstatus, " - ");
+							g_string_append_len(xstatus, tmp1, tmp2 - tmp1);
+						}
 					}
-					strcpy(xml,title);
-					if (desc) {
-						strcat(xml, " - ");
-						strcat(xml, desc);
+					if (xstatus->len > 0) {
+						purple_debug_misc("oscar", "X-Status reply: %s\n", xstatus->str);
+						account = purple_connection_get_account(od->gc);
+						buddy = purple_find_buddy(account, bn);
+						presence = purple_buddy_get_presence(buddy);
+						status = purple_presence_get_active_status(presence);
+						purple_prpl_got_user_status(account, bn,
+								purple_status_get_id(status),
+								"message", xstatus->str, NULL);
 					}
-					purple_debug_misc("oscar", "X-Status reply: %s\n", xml);
-					account = purple_connection_get_account(od->gc);
-					buddy = purple_find_buddy(account, bn);
-					presence = purple_buddy_get_presence(buddy);
-					status = purple_presence_get_active_status(presence);
-					purple_prpl_got_user_status(account, bn,
-							purple_status_get_id(status), "message", xml, NULL);
+					g_string_free(xstatus, TRUE);
 				} else {
 					purple_debug_misc("oscar", "X-Status: Can't get XML reply string\n");
 				}