Prevent spoofing of iq replies by verifying that the 'from' address release-2.x.y
authorMark Doliner <mark@kingant.net>
Sun, 12 Jan 2014 22:51:33 -0800
branchrelease-2.x.y
changeset93d4bff19574 pushlog
parent ed1f9a0c0979
child 6bafdcde2b55
Prevent spoofing of iq replies by verifying that the 'from' address
matches the 'to' address of the iq request.

This full extent of this problem was realized by Thijs Alkemade, while
investigating a problem reported to us by Fabian Yamaguchi and Christian
Wressnegger of the University of Goettingen)

This change was created by Thijs Alkemade, with small shuffling and
variable renaming by me.
ChangeLog
libpurple/protocols/jabber/iq.c
libpurple/protocols/jabber/iq.h
libpurple/protocols/jabber/jabber.c
libpurple/protocols/jabber/jutil.c
libpurple/protocols/jabber/jutil.h
     1.1 --- a/ChangeLog
     1.2 +++ b/ChangeLog
     1.3 @@ -69,6 +69,10 @@
     1.4  	  (Discovered by Sourcefire VRT) (CVE-2014-NNNN)
     1.5  
     1.6  	XMPP:
     1.7 +	* Prevent spoofing of iq replies by verifying that the 'from' address
     1.8 +	  matches the 'to' address of the iq request. (Discovered by Fabian
     1.9 +	  Yamaguchi and Christian Wressnegger of the University of Goettingen)
    1.10 +	  (CVE-2014-NNNN)
    1.11  	* Fix possible crash or other erratic behavior when selecting a very
    1.12  	  small file for your own buddy icon.
    1.13  	* Fix crash if the user tries to initiate a voice/video session with a
     2.1 --- a/libpurple/protocols/jabber/iq.c
     2.2 +++ b/libpurple/protocols/jabber/iq.c
     2.3 @@ -49,6 +49,18 @@
     2.4  static GHashTable *iq_handlers = NULL;
     2.5  static GHashTable *signal_iq_handlers = NULL;
     2.6  
     2.7 +struct _JabberIqCallbackData {
     2.8 +	JabberIqCallback *callback;
     2.9 +	gpointer data;
    2.10 +	JabberID *to;
    2.11 +};
    2.12 +
    2.13 +void jabber_iq_callbackdata_free(JabberIqCallbackData *jcd)
    2.14 +{
    2.15 +	jabber_id_free(jcd->to);
    2.16 +	g_free(jcd);
    2.17 +}
    2.18 +
    2.19  JabberIq *jabber_iq_new(JabberStream *js, JabberIqType type)
    2.20  {
    2.21  	JabberIq *iq;
    2.22 @@ -98,11 +110,6 @@
    2.23  	return iq;
    2.24  }
    2.25  
    2.26 -typedef struct _JabberCallbackData {
    2.27 -	JabberIqCallback *callback;
    2.28 -	gpointer data;
    2.29 -} JabberCallbackData;
    2.30 -
    2.31  void
    2.32  jabber_iq_set_callback(JabberIq *iq, JabberIqCallback *callback, gpointer data)
    2.33  {
    2.34 @@ -125,15 +132,17 @@
    2.35  
    2.36  void jabber_iq_send(JabberIq *iq)
    2.37  {
    2.38 -	JabberCallbackData *jcd;
    2.39 +	JabberIqCallbackData *jcd;
    2.40  	g_return_if_fail(iq != NULL);
    2.41  
    2.42  	jabber_send(iq->js, iq->node);
    2.43  
    2.44  	if(iq->id && iq->callback) {
    2.45 -		jcd = g_new0(JabberCallbackData, 1);
    2.46 +		jcd = g_new0(JabberIqCallbackData, 1);
    2.47  		jcd->callback = iq->callback;
    2.48  		jcd->data = iq->callback_data;
    2.49 +		jcd->to = jabber_id_new(xmlnode_get_attrib(iq->node, "to"));
    2.50 +
    2.51  		g_hash_table_insert(iq->js->iq_callbacks, g_strdup(iq->id), jcd);
    2.52  	}
    2.53  
    2.54 @@ -276,18 +285,30 @@
    2.55  
    2.56  void jabber_iq_parse(JabberStream *js, xmlnode *packet)
    2.57  {
    2.58 -	JabberCallbackData *jcd;
    2.59 +	JabberIqCallbackData *jcd;
    2.60  	xmlnode *child, *error, *x;
    2.61  	const char *xmlns;
    2.62  	const char *iq_type, *id, *from;
    2.63  	JabberIqType type = JABBER_IQ_NONE;
    2.64  	gboolean signal_return;
    2.65 +	JabberID *from_id;
    2.66  
    2.67  	from = xmlnode_get_attrib(packet, "from");
    2.68  	id = xmlnode_get_attrib(packet, "id");
    2.69  	iq_type = xmlnode_get_attrib(packet, "type");
    2.70  
    2.71  	/*
    2.72 +	 * Ensure the 'from' attribute is valid. No point in handling a stanza
    2.73 +	 * of which we don't understand where it came from.
    2.74 +	 */
    2.75 +	from_id = jabber_id_new(from);
    2.76 +
    2.77 +	if (from && !from_id) {
    2.78 +		purple_debug_error("jabber", "Received an iq with an invalid from: %s\n", from);
    2.79 +		return;
    2.80 +	}
    2.81 +
    2.82 +	/*
    2.83  	 * child will be either the first tag child or NULL if there is no child.
    2.84  	 * Historically, we used just the 'query' subchild, but newer XEPs use
    2.85  	 * differently named children. Grabbing the first child is (for the time
    2.86 @@ -312,6 +333,7 @@
    2.87  	if (type == JABBER_IQ_NONE) {
    2.88  		purple_debug_error("jabber", "IQ with invalid type ('%s') - ignoring.\n",
    2.89  						   iq_type ? iq_type : "(null)");
    2.90 +		jabber_id_free(from_id);
    2.91  		return;
    2.92  	}
    2.93  
    2.94 @@ -342,20 +364,38 @@
    2.95  			purple_debug_error("jabber", "IQ of type '%s' missing id - ignoring.\n",
    2.96  			                   iq_type);
    2.97  
    2.98 +		jabber_id_free(from_id);
    2.99  		return;
   2.100  	}
   2.101  
   2.102  	signal_return = GPOINTER_TO_INT(purple_signal_emit_return_1(purple_connection_get_prpl(js->gc),
   2.103  			"jabber-receiving-iq", js->gc, iq_type, id, from, packet));
   2.104 -	if (signal_return)
   2.105 +	if (signal_return) {
   2.106 +		jabber_id_free(from_id);
   2.107  		return;
   2.108 +	}
   2.109  
   2.110  	/* First, lets see if a special callback got registered */
   2.111  	if(type == JABBER_IQ_RESULT || type == JABBER_IQ_ERROR) {
   2.112  		if((jcd = g_hash_table_lookup(js->iq_callbacks, id))) {
   2.113 -			jcd->callback(js, from, type, id, packet, jcd->data);
   2.114 -			jabber_iq_remove_callback_by_id(js, id);
   2.115 -			return;
   2.116 +			if(jabber_id_equal(js, jcd->to, from_id)) {
   2.117 +				jcd->callback(js, from, type, id, packet, jcd->data);
   2.118 +				jabber_iq_remove_callback_by_id(js, id);
   2.119 +				jabber_id_free(from_id);
   2.120 +				return;
   2.121 +			} else {
   2.122 +				char *expected_to;
   2.123 +
   2.124 +				if (jcd->to) {
   2.125 +					expected_to = jabber_id_get_full_jid(jcd->to);
   2.126 +				} else {
   2.127 +					expected_to = jabber_id_get_bare_jid(js->user);
   2.128 +				}
   2.129 +
   2.130 +				purple_debug_error("jabber", "Got a result iq with id %s from %s instead of expected %s!\n", id, from ? from : "(null)", expected_to);
   2.131 +
   2.132 +				g_free(expected_to);
   2.133 +			}
   2.134  		}
   2.135  	}
   2.136  
   2.137 @@ -372,12 +412,15 @@
   2.138  		if (signal_ref > 0) {
   2.139  			signal_return = GPOINTER_TO_INT(purple_signal_emit_return_1(purple_connection_get_prpl(js->gc), "jabber-watched-iq",
   2.140  					js->gc, iq_type, id, from, child));
   2.141 -			if (signal_return)
   2.142 +			if (signal_return) {
   2.143 +				jabber_id_free(from_id);
   2.144  				return;
   2.145 +			}
   2.146  		}
   2.147  
   2.148  		if(jih) {
   2.149  			jih(js, from, type, id, child);
   2.150 +			jabber_id_free(from_id);
   2.151  			return;
   2.152  		}
   2.153  	}
   2.154 @@ -404,6 +447,8 @@
   2.155  
   2.156  		jabber_iq_send(iq);
   2.157  	}
   2.158 +
   2.159 +	jabber_id_free(from_id);
   2.160  }
   2.161  
   2.162  void jabber_iq_register_handler(const char *node, const char *xmlns, JabberIqHandler *handlerfunc)
     3.1 --- a/libpurple/protocols/jabber/iq.h
     3.2 +++ b/libpurple/protocols/jabber/iq.h
     3.3 @@ -36,6 +36,7 @@
     3.4  #include "connection.h"
     3.5  
     3.6  typedef struct _JabberIq JabberIq;
     3.7 +typedef struct _JabberIqCallbackData  JabberIqCallbackData;
     3.8  
     3.9  /**
    3.10   * A JabberIqHandler is called to process an incoming IQ stanza.
    3.11 @@ -96,6 +97,7 @@
    3.12  
    3.13  void jabber_iq_parse(JabberStream *js, xmlnode *packet);
    3.14  
    3.15 +void jabber_iq_callbackdata_free(JabberIqCallbackData *jcd);
    3.16  void jabber_iq_remove_callback_by_id(JabberStream *js, const char *id);
    3.17  void jabber_iq_set_callback(JabberIq *iq, JabberIqCallback *cb, gpointer data);
    3.18  void jabber_iq_set_id(JabberIq *iq, const char *id);
     4.1 --- a/libpurple/protocols/jabber/jabber.c
     4.2 +++ b/libpurple/protocols/jabber/jabber.c
     4.3 @@ -988,7 +988,7 @@
     4.4  	js->user_jb->subscription |= JABBER_SUB_BOTH;
     4.5  
     4.6  	js->iq_callbacks = g_hash_table_new_full(g_str_hash, g_str_equal,
     4.7 -			g_free, g_free);
     4.8 +			g_free, (GDestroyNotify)jabber_iq_callbackdata_free);
     4.9  	js->chats = g_hash_table_new_full(g_str_hash, g_str_equal,
    4.10  			g_free, (GDestroyNotify)jabber_chat_free);
    4.11  	js->next_id = g_random_int();
     5.1 --- a/libpurple/protocols/jabber/jutil.c
     5.2 +++ b/libpurple/protocols/jabber/jutil.c
     5.3 @@ -508,6 +508,34 @@
     5.4  	}
     5.5  }
     5.6  
     5.7 +
     5.8 +gboolean
     5.9 +jabber_id_equal(JabberStream *js, const JabberID *jid1, const JabberID *jid2)
    5.10 +{
    5.11 +	const JabberID *j1, *j2;
    5.12 +	JabberID *bare_user_jid;
    5.13 +	gboolean equal;
    5.14 +
    5.15 +	/* If an outgoing stanza has no 'to', or an incoming has no 'from',
    5.16 +	 * then those are "the server acting as my account". This function will
    5.17 +	 * handle that correctly.
    5.18 +	 */
    5.19 +	if (!jid1 && !jid2)
    5.20 +		return TRUE;
    5.21 +
    5.22 +	bare_user_jid = jabber_id_to_bare_jid(js->user);
    5.23 +	j1 = jid1 ? jid1 : bare_user_jid;
    5.24 +	j2 = jid2 ? jid2 : bare_user_jid;
    5.25 +
    5.26 +	equal = purple_strequal(j1->node, j2->node) &&
    5.27 +			purple_strequal(j1->domain, j2->domain) &&
    5.28 +			purple_strequal(j1->resource, j2->resource);
    5.29 +
    5.30 +	jabber_id_free(bare_user_jid);
    5.31 +
    5.32 +	return equal;
    5.33 +}
    5.34 +
    5.35  char *jabber_get_domain(const char *in)
    5.36  {
    5.37  	JabberID *jid = jabber_id_new(in);
    5.38 @@ -536,6 +564,17 @@
    5.39  	return out;
    5.40  }
    5.41  
    5.42 +JabberID *
    5.43 +jabber_id_to_bare_jid(const JabberID *jid)
    5.44 +{
    5.45 +	JabberID *result = g_new0(JabberID, 1);
    5.46 +
    5.47 +	result->node = g_strdup(jid->node);
    5.48 +	result->domain = g_strdup(jid->domain);
    5.49 +
    5.50 +	return result;
    5.51 +}
    5.52 +
    5.53  char *
    5.54  jabber_get_bare_jid(const char *in)
    5.55  {
    5.56 @@ -561,6 +600,19 @@
    5.57  	                   NULL);
    5.58  }
    5.59  
    5.60 +char *
    5.61 +jabber_id_get_full_jid(const JabberID *jid)
    5.62 +{
    5.63 +	g_return_val_if_fail(jid != NULL, NULL);
    5.64 +
    5.65 +	return g_strconcat(jid->node ? jid->node : "",
    5.66 +	                   jid->node ? "@" : "",
    5.67 +	                   jid->domain,
    5.68 +	                   jid->resource ? "/" : "",
    5.69 +	                   jid->resource ? jid->resource : "",
    5.70 +	                   NULL);
    5.71 +}
    5.72 +
    5.73  gboolean
    5.74  jabber_jid_is_domain(const char *jid)
    5.75  {
     6.1 --- a/libpurple/protocols/jabber/jutil.h
     6.2 +++ b/libpurple/protocols/jabber/jutil.h
     6.3 @@ -44,12 +44,23 @@
     6.4  #include "jabber.h"
     6.5  
     6.6  JabberID* jabber_id_new(const char *str);
     6.7 +
     6.8 +/**
     6.9 + * Compare two JIDs for equality.
    6.10 + *
    6.11 + * Warning: If either JID is NULL then this function uses the user's
    6.12 + * bare JID, instead!
    6.13 + */
    6.14 +gboolean jabber_id_equal(JabberStream *js, const JabberID *jid1, const JabberID *jid2);
    6.15 +
    6.16  void jabber_id_free(JabberID *jid);
    6.17  
    6.18  char *jabber_get_domain(const char *jid);
    6.19  char *jabber_get_resource(const char *jid);
    6.20  char *jabber_get_bare_jid(const char *jid);
    6.21  char *jabber_id_get_bare_jid(const JabberID *jid);
    6.22 +char *jabber_id_get_full_jid(const JabberID *jid);
    6.23 +JabberID *jabber_id_to_bare_jid(const JabberID *jid);
    6.24  
    6.25  gboolean jabber_jid_is_domain(const char *jid);
    6.26