changeset 32380:231c9635b82a

Fix crashes and memory leaks when receiving malformed voice and video requests. Thanks to Thijs Alkemade for reporting this!
author Mark Doliner <mark@kingant.net>
date Tue, 06 Dec 2011 07:06:01 +0000
parents 76b7754ba906
children f08f5a803268
files ChangeLog libpurple/protocols/jabber/jingle/jingle.c libpurple/protocols/jabber/jingle/rtp.c libpurple/protocols/jabber/jingle/session.c
diffstat 4 files changed, 91 insertions(+), 36 deletions(-) [+]
line wrap: on
line diff
--- a/ChangeLog	Tue Dec 06 06:58:41 2011 +0000
+++ b/ChangeLog	Tue Dec 06 07:06:01 2011 +0000
@@ -18,6 +18,10 @@
 	* Fix a leak when admitting UTF-8 text with a non-UTF-8 primary
 	  encoding.  (#14700)
 
+	Jabber:
+	* Fix crashes and memory leaks when receiving malformed voice
+	  and video requests.  Thanks to Thijs Alkemade for reporting this!
+
 	Sametime:
 	* Separate "username" and "server" when adding new Sametime accounts.
 	  (#14608)
--- a/libpurple/protocols/jabber/jingle/jingle.c	Tue Dec 06 06:58:41 2011 +0000
+++ b/libpurple/protocols/jabber/jingle/jingle.c	Tue Dec 06 07:06:01 2011 +0000
@@ -126,7 +126,7 @@
 		if (local_content != NULL) {
 			const gchar *senders = xmlnode_get_attrib(content, "senders");
 			gchar *local_senders = jingle_content_get_senders(local_content);
-			if (strcmp(senders, local_senders))
+			if (!purple_strequal(senders, local_senders))
 				jingle_content_modify(local_content, senders);
 			g_free(local_senders);
 		} else {
--- a/libpurple/protocols/jabber/jingle/rtp.c	Tue Dec 06 06:58:41 2011 +0000
+++ b/libpurple/protocols/jabber/jingle/rtp.c	Tue Dec 06 07:06:01 2011 +0000
@@ -590,6 +590,16 @@
 	senders = jingle_content_get_senders(content);
 	transport = jingle_content_get_transport(content);
 
+	if (media_type == NULL) {
+		g_free(name);
+		g_free(remote_jid);
+		g_free(senders);
+		g_free(params);
+		g_object_unref(transport);
+		g_object_unref(session);
+		return FALSE;
+	}
+
 	if (JINGLE_IS_RAWUDP(transport))
 		transmitter = "rawudp";
 	else if (JINGLE_IS_ICEUDP(transport))
@@ -598,17 +608,17 @@
 		transmitter = "notransmitter";
 	g_object_unref(transport);
 
-	is_audio = !strcmp(media_type, "audio");
+	is_audio = g_str_equal(media_type, "audio");
 
-	if (!strcmp(senders, "both"))
-		type = is_audio == TRUE ? PURPLE_MEDIA_AUDIO
+	if (purple_strequal(senders, "both"))
+		type = is_audio ? PURPLE_MEDIA_AUDIO
 				: PURPLE_MEDIA_VIDEO;
-	else if ((strcmp(senders, "initiator") == 0) ==
+	else if (purple_strequal(senders, "initiator") ==
 			jingle_session_is_initiator(session))
-		type = is_audio == TRUE ? PURPLE_MEDIA_SEND_AUDIO
+		type = is_audio ? PURPLE_MEDIA_SEND_AUDIO
 				: PURPLE_MEDIA_SEND_VIDEO;
 	else
-		type = is_audio == TRUE ? PURPLE_MEDIA_RECV_AUDIO
+		type = is_audio ? PURPLE_MEDIA_RECV_AUDIO
 				: PURPLE_MEDIA_RECV_VIDEO;
 
 	params =
@@ -616,7 +626,17 @@
 			NULL, NULL, &num_params);
 
 	creator = jingle_content_get_creator(content);
-	if (!strcmp(creator, "initiator"))
+	if (creator == NULL) {
+		g_free(name);
+		g_free(media_type);
+		g_free(remote_jid);
+		g_free(senders);
+		g_free(params);
+		g_object_unref(session);
+		return FALSE;
+	}
+
+	if (g_str_equal(creator, "initiator"))
 		is_creator = jingle_session_is_initiator(session);
 	else
 		is_creator = !jingle_session_is_initiator(session);
@@ -625,6 +645,8 @@
 	if(!purple_media_add_stream(media, name, remote_jid,
 			type, is_creator, transmitter, num_params, params)) {
 		purple_media_end(media, NULL, NULL);
+		/* TODO: How much clean-up is necessary here? (does calling
+		         purple_media_end lead to cleaning up Jingle structs?) */
 		return FALSE;
 	}
 
@@ -646,9 +668,22 @@
 	const char *encoding_name,*id, *clock_rate;
 	PurpleMediaCodec *codec;
 	const gchar *media = xmlnode_get_attrib(description, "media");
-	PurpleMediaSessionType type =
-			!strcmp(media, "video") ? PURPLE_MEDIA_VIDEO :
-			!strcmp(media, "audio") ? PURPLE_MEDIA_AUDIO : 0;
+	PurpleMediaSessionType type;
+
+	if (media == NULL) {
+		purple_debug_warning("jingle-rtp", "missing media type\n");
+		return NULL;
+	}
+
+	if (g_str_equal(media, "video")) {
+		type = PURPLE_MEDIA_VIDEO;
+	} else if (g_str_equal(media, "audio")) {
+		type = PURPLE_MEDIA_AUDIO;
+	} else {
+		purple_debug_warning("jingle-rtp", "unknown media type: %s\n",
+				media);
+		return NULL;
+	}
 
 	for (codec_element = xmlnode_get_child(description, "payload-type") ;
 		 codec_element ;
@@ -769,19 +804,19 @@
 	switch (action) {
 		case JINGLE_SESSION_ACCEPT:
 		case JINGLE_SESSION_INITIATE: {
-			JingleSession *session = jingle_content_get_session(content);
-			JingleTransport *transport = jingle_transport_parse(
-					xmlnode_get_child(xmlcontent, "transport"));
-			xmlnode *description = xmlnode_get_child(xmlcontent, "description");
-			GList *candidates = jingle_rtp_transport_to_candidates(transport);
-			GList *codecs = jingle_rtp_parse_codecs(description);
-			gchar *name = jingle_content_get_name(content);
-			gchar *remote_jid =
-					jingle_session_get_remote_jid(session);
+			JingleSession *session;
+			JingleTransport *transport;
+			xmlnode *description;
+			GList *candidates;
+			GList *codecs;
+			gchar *name;
+			gchar *remote_jid;
 			PurpleMedia *media;
 
+			session = jingle_content_get_session(content);
+
 			if (action == JINGLE_SESSION_INITIATE &&
-					jingle_rtp_init_media(content) == FALSE) {
+					!jingle_rtp_init_media(content)) {
 				/* XXX: send error */
 				jabber_iq_send(jingle_session_terminate_packet(
 						session, "general-error"));
@@ -789,6 +824,14 @@
 				break;
 			}
 
+			transport = jingle_transport_parse(
+					xmlnode_get_child(xmlcontent, "transport"));
+			description = xmlnode_get_child(xmlcontent, "description");
+			candidates = jingle_rtp_transport_to_candidates(transport);
+			codecs = jingle_rtp_parse_codecs(description);
+			name = jingle_content_get_name(content);
+			remote_jid = jingle_session_get_remote_jid(session);
+
 			media = jingle_rtp_get_media(session);
 			purple_media_set_remote_codecs(media,
 					name, remote_jid, codecs);
--- a/libpurple/protocols/jabber/jingle/session.c	Tue Dec 06 06:58:41 2011 +0000
+++ b/libpurple/protocols/jabber/jingle/session.c	Tue Dec 06 07:06:01 2011 +0000
@@ -288,7 +288,7 @@
 	if (!js->sessions) {
 		purple_debug_info("jingle",
 				"Creating hash table for sessions\n");
-		js->sessions = g_hash_table_new(g_str_hash, g_str_equal);
+		js->sessions = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, NULL);
 	}
 	purple_debug_info("jingle",
 			"inserting session with key: %s into table\n", sid);
@@ -411,26 +411,24 @@
 			xmlnode_new("jingle");
 	gchar *local_jid = jingle_session_get_local_jid(session);
 	gchar *remote_jid = jingle_session_get_remote_jid(session);
+	gchar *sid = jingle_session_get_sid(session);
 
 	xmlnode_set_namespace(jingle, JINGLE);
 	xmlnode_set_attrib(jingle, "action", jingle_get_action_name(action));
 
 	if (jingle_session_is_initiator(session)) {
-		xmlnode_set_attrib(jingle, "initiator",
-				jingle_session_get_local_jid(session));
-		xmlnode_set_attrib(jingle, "responder",
-				jingle_session_get_remote_jid(session));
+		xmlnode_set_attrib(jingle, "initiator", local_jid);
+		xmlnode_set_attrib(jingle, "responder", remote_jid);
 	} else {
-		xmlnode_set_attrib(jingle, "initiator",
-				jingle_session_get_remote_jid(session));
-		xmlnode_set_attrib(jingle, "responder",
-				jingle_session_get_local_jid(session));
+		xmlnode_set_attrib(jingle, "initiator", remote_jid);
+		xmlnode_set_attrib(jingle, "responder", local_jid);
 	}
 
+	xmlnode_set_attrib(jingle, "sid", sid);
+
 	g_free(local_jid);
 	g_free(remote_jid);
-
-	xmlnode_set_attrib(jingle, "sid", jingle_session_get_sid(session));
+	g_free(sid);
 
 	return jingle;
 }
@@ -508,11 +506,16 @@
 JingleContent *
 jingle_session_find_content(JingleSession *session, const gchar *name, const gchar *creator)
 {
-	GList *iter = session->priv->contents;
+	GList *iter;
+
+	if (name == NULL)
+		return NULL;
+
+	iter = session->priv->contents;
 	for (; iter; iter = g_list_next(iter)) {
 		JingleContent *content = iter->data;
 		gchar *cname = jingle_content_get_name(content);
-		gboolean result = !strcmp(name, cname);
+		gboolean result = g_str_equal(name, cname);
 		g_free(cname);
 
 		if (creator != NULL) {
@@ -530,11 +533,16 @@
 JingleContent *
 jingle_session_find_pending_content(JingleSession *session, const gchar *name, const gchar *creator)
 {
-	GList *iter = session->priv->pending_contents;
+	GList *iter;
+
+	if (name == NULL)
+		return NULL;
+
+	iter = session->priv->pending_contents;
 	for (; iter; iter = g_list_next(iter)) {
 		JingleContent *content = iter->data;
 		gchar *cname = jingle_content_get_name(content);
-		gboolean result = !strcmp(name, cname);
+		gboolean result = g_str_equal(name, cname);
 		g_free(cname);
 
 		if (creator != NULL) {