/pidgin/main: 4d139ce8f7ec: yahoo: Fix reading memory locations ...

[Daniel Atallah]

Changeset: 4d139ce8f7ec484e8be9fe1ada0e021000ff00a7
Author:	 Daniel Atallah <datallah at pidgin.im>
Date:	 2013-03-16 14:02 -0400
Branch:	 release-2.x.y
URL: https://hg.pidgin.im/pidgin/main/rev/4d139ce8f7ec

Description:

yahoo: Fix reading memory locations past the buffer bounds

 * Also improve behavior when parsing malformed P2P packets (it seems like it
   might be better to just disconnect when this happens, but I'm not familiar
   with why the code as added to try to handle the malformed packets).
 * Also, add a comment about the mistaken assumption in p2p packet handling that
   reads are being buffered and that partial packets can be subsequently filled
   in.

diffstat:

 libpurple/protocols/yahoo/libymsg.c |  38 ++++++++++++++++++++++++++++--------
 1 files changed, 29 insertions(+), 9 deletions(-)

diffs (68 lines):

diff --git a/libpurple/protocols/yahoo/libymsg.c b/libpurple/protocols/yahoo/libymsg.c
--- a/libpurple/protocols/yahoo/libymsg.c
+++ b/libpurple/protocols/yahoo/libymsg.c
@@ -2536,7 +2536,7 @@ static void yahoo_p2p_read_pkt_cb(gpoint
 	int pos = 0;
 	int pktlen;
 	struct yahoo_packet *pkt;
-	guchar *start = NULL;
+	guchar *start;
 	struct yahoo_p2p_data *p2p_data;
 	YahooData *yd;
 
@@ -2558,19 +2558,29 @@ static void yahoo_p2p_read_pkt_cb(gpoint
 		return;
 	}
 
+	/* TODO: It looks like there's a bug here (and above) where an incorrect
+	 * assumtion is being made that the buffer will be added to when this
+	 * is next called, but that's not really the case! */
 	if(len < YAHOO_PACKET_HDRLEN)
 		return;
 
-	if(strncmp((char *)buf, "YMSG", MIN(4, len)) != 0) {
+	if(strncmp((char *)buf, "YMSG", 4) != 0) {
 		/* Not a YMSG packet */
+		purple_debug_warning("yahoo", "p2p: Got something other than YMSG packet\n");
+
+		start = (guchar *) g_strstr_len((char *) buf + 1, len - 1 ,"YMSG");
+		if (start == NULL) {
+			/* remove from p2p connection lists, also calls yahoo_p2p_disconnect_destroy_data */
+			if (g_hash_table_lookup(yd->peers, p2p_data->host_username))
+				g_hash_table_remove(yd->peers, p2p_data->host_username);
+			else
+				yahoo_p2p_disconnect_destroy_data(data);
+			return;
+		}
 		purple_debug_warning("yahoo","p2p: Got something other than YMSG packet\n");
 
-		start = memchr(buf + 1, 'Y', len - 1);
-		if (start == NULL)
-			return;
-
-		g_memmove(buf, start, len - (start - buf));
-		len -= start - buf;
+		len -= (start - buf);
+		g_memmove(buf, start, len);
 	}
 
 	pos += 4;	/* YMSG */
@@ -2578,7 +2588,17 @@ static void yahoo_p2p_read_pkt_cb(gpoint
 	pos += 2;
 
 	pktlen = yahoo_get16(buf + pos); pos += 2;
-	purple_debug_misc("yahoo", "p2p: %d bytes to read\n", len);
+	if (len < (YAHOO_PACKET_HDRLEN + pktlen)) {
+		purple_debug_error("yahoo", "p2p: packet length(%d) > buffer length(%d)\n",
+				pktlen, (len - pos)); 
+		/* remove from p2p connection lists, also calls yahoo_p2p_disconnect_destroy_data */
+		if (g_hash_table_lookup(yd->peers, p2p_data->host_username))
+			g_hash_table_remove(yd->peers, p2p_data->host_username);
+		else
+			yahoo_p2p_disconnect_destroy_data(data);
+		return;
+	} else
+		purple_debug_misc("yahoo", "p2p: %d bytes to read\n", pktlen);
 
 	pkt = yahoo_packet_new(0, 0, 0);
 	pkt->service = yahoo_get16(buf + pos); pos += 2;