yahoo: Fix reading memory locations past the buffer bounds release-2.x.y
authorDaniel Atallah <datallah@pidgin.im>
Sat, 16 Mar 2013 14:02:10 -0400
branchrelease-2.x.y
changeset4d139ce8f7ec pushlog
parent 944ec96bb103
child 932b985540e9
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.
libpurple/protocols/yahoo/libymsg.c
     1.1 --- a/libpurple/protocols/yahoo/libymsg.c
     1.2 +++ b/libpurple/protocols/yahoo/libymsg.c
     1.3 @@ -2536,7 +2536,7 @@
     1.4  	int pos = 0;
     1.5  	int pktlen;
     1.6  	struct yahoo_packet *pkt;
     1.7 -	guchar *start = NULL;
     1.8 +	guchar *start;
     1.9  	struct yahoo_p2p_data *p2p_data;
    1.10  	YahooData *yd;
    1.11  
    1.12 @@ -2558,19 +2558,29 @@
    1.13  		return;
    1.14  	}
    1.15  
    1.16 +	/* TODO: It looks like there's a bug here (and above) where an incorrect
    1.17 +	 * assumtion is being made that the buffer will be added to when this
    1.18 +	 * is next called, but that's not really the case! */
    1.19  	if(len < YAHOO_PACKET_HDRLEN)
    1.20  		return;
    1.21  
    1.22 -	if(strncmp((char *)buf, "YMSG", MIN(4, len)) != 0) {
    1.23 +	if(strncmp((char *)buf, "YMSG", 4) != 0) {
    1.24  		/* Not a YMSG packet */
    1.25 +		purple_debug_warning("yahoo", "p2p: Got something other than YMSG packet\n");
    1.26 +
    1.27 +		start = (guchar *) g_strstr_len((char *) buf + 1, len - 1 ,"YMSG");
    1.28 +		if (start == NULL) {
    1.29 +			/* remove from p2p connection lists, also calls yahoo_p2p_disconnect_destroy_data */
    1.30 +			if (g_hash_table_lookup(yd->peers, p2p_data->host_username))
    1.31 +				g_hash_table_remove(yd->peers, p2p_data->host_username);
    1.32 +			else
    1.33 +				yahoo_p2p_disconnect_destroy_data(data);
    1.34 +			return;
    1.35 +		}
    1.36  		purple_debug_warning("yahoo","p2p: Got something other than YMSG packet\n");
    1.37  
    1.38 -		start = memchr(buf + 1, 'Y', len - 1);
    1.39 -		if (start == NULL)
    1.40 -			return;
    1.41 -
    1.42 -		g_memmove(buf, start, len - (start - buf));
    1.43 -		len -= start - buf;
    1.44 +		len -= (start - buf);
    1.45 +		g_memmove(buf, start, len);
    1.46  	}
    1.47  
    1.48  	pos += 4;	/* YMSG */
    1.49 @@ -2578,7 +2588,17 @@
    1.50  	pos += 2;
    1.51  
    1.52  	pktlen = yahoo_get16(buf + pos); pos += 2;
    1.53 -	purple_debug_misc("yahoo", "p2p: %d bytes to read\n", len);
    1.54 +	if (len < (YAHOO_PACKET_HDRLEN + pktlen)) {
    1.55 +		purple_debug_error("yahoo", "p2p: packet length(%d) > buffer length(%d)\n",
    1.56 +				pktlen, (len - pos)); 
    1.57 +		/* remove from p2p connection lists, also calls yahoo_p2p_disconnect_destroy_data */
    1.58 +		if (g_hash_table_lookup(yd->peers, p2p_data->host_username))
    1.59 +			g_hash_table_remove(yd->peers, p2p_data->host_username);
    1.60 +		else
    1.61 +			yahoo_p2p_disconnect_destroy_data(data);
    1.62 +		return;
    1.63 +	} else
    1.64 +		purple_debug_misc("yahoo", "p2p: %d bytes to read\n", pktlen);
    1.65  
    1.66  	pkt = yahoo_packet_new(0, 0, 0);
    1.67  	pkt->service = yahoo_get16(buf + pos); pos += 2;