HGKeeper

pidgin/pidgin

Prevent spoofing of iq replies by verifying that the 'from' address
release-2.x.y
2014-01-12, Mark Doliner

93d4bff19574

Parents ed1f9a0c0979
Children 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.

6 files changed, 128 insertions(+), 14 deletions(-)

+4 -0

ChangeLog

+58 -13

libpurple/protocols/jabber/iq.c

+2 -0

libpurple/protocols/jabber/iq.h

+1 -1

libpurple/protocols/jabber/jabber.c

+52 -0

libpurple/protocols/jabber/jutil.c

+11 -0

libpurple/protocols/jabber/jutil.h

~~--- a/ChangeLog Sun Jan 12 19:29:36 2014 -0800~~

+++ b/ChangeLog Sun Jan 12 22:51:33 2014 -0800

@@ -69,6 +69,10 @@

(Discovered by Sourcefire VRT) (CVE-2014-NNNN)

XMPP:

+ * Prevent spoofing of iq replies by verifying that the 'from' address

+ matches the 'to' address of the iq request. (Discovered by Fabian

+ Yamaguchi and Christian Wressnegger of the University of Goettingen)

+ (CVE-2014-NNNN)

* Fix possible crash or other erratic behavior when selecting a very

small file for your own buddy icon.

* Fix crash if the user tries to initiate a voice/video session with a

~~--- a/libpurple/protocols/jabber/iq.c Sun Jan 12 19:29:36 2014 -0800~~

+++ b/libpurple/protocols/jabber/iq.c Sun Jan 12 22:51:33 2014 -0800

@@ -49,6 +49,18 @@

static GHashTable *iq_handlers = NULL;

static GHashTable *signal_iq_handlers = NULL;

+struct _JabberIqCallbackData {

+ JabberIqCallback *callback;

+ gpointer data;

+ JabberID *to;

+};

+void jabber_iq_callbackdata_free(JabberIqCallbackData *jcd)

+ jabber_id_free(jcd->to);

+ g_free(jcd);

JabberIq *jabber_iq_new(JabberStream *js, JabberIqType type)

{

JabberIq *iq;

@@ -98,11 +110,6 @@

return iq;

}

~~-typedef struct _JabberCallbackData {~~

~~- JabberIqCallback *callback;~~

~~- gpointer data;~~

~~-} JabberCallbackData;~~

void

jabber_iq_set_callback(JabberIq *iq, JabberIqCallback *callback, gpointer data)

{

@@ -125,15 +132,17 @@

void jabber_iq_send(JabberIq *iq)

{

~~- JabberCallbackData *jcd;~~

+ JabberIqCallbackData *jcd;

g_return_if_fail(iq != NULL);

jabber_send(iq->js, iq->node);

if(iq->id && iq->callback) {

~~- jcd = g_new0(JabberCallbackData, 1);~~

+ jcd = g_new0(JabberIqCallbackData, 1);

jcd->callback = iq->callback;

jcd->data = iq->callback_data;

+ jcd->to = jabber_id_new(xmlnode_get_attrib(iq->node, "to"));

g_hash_table_insert(iq->js->iq_callbacks, g_strdup(iq->id), jcd);

}

@@ -276,18 +285,30 @@

void jabber_iq_parse(JabberStream *js, xmlnode *packet)

{

~~- JabberCallbackData *jcd;~~

+ JabberIqCallbackData *jcd;

xmlnode *child, *error, *x;

const char *xmlns;

const char *iq_type, *id, *from;

JabberIqType type = JABBER_IQ_NONE;

gboolean signal_return;

+ JabberID *from_id;

from = xmlnode_get_attrib(packet, "from");

id = xmlnode_get_attrib(packet, "id");

iq_type = xmlnode_get_attrib(packet, "type");

+ * Ensure the 'from' attribute is valid. No point in handling a stanza

+ * of which we don't understand where it came from.

+ */

+ from_id = jabber_id_new(from);

+ if (from && !from_id) {

+ purple_debug_error("jabber", "Received an iq with an invalid from: %s\n", from);

+ return;

+ }

+ /*

* child will be either the first tag child or NULL if there is no child.

* Historically, we used just the 'query' subchild, but newer XEPs use

* differently named children. Grabbing the first child is (for the time

@@ -312,6 +333,7 @@

if (type == JABBER_IQ_NONE) {

purple_debug_error("jabber", "IQ with invalid type ('%s') - ignoring.\n",

iq_type ? iq_type : "(null)");

+ jabber_id_free(from_id);

return;

}

@@ -342,20 +364,38 @@

purple_debug_error("jabber", "IQ of type '%s' missing id - ignoring.\n",

iq_type);

+ jabber_id_free(from_id);

return;

}

signal_return = GPOINTER_TO_INT(purple_signal_emit_return_1(purple_connection_get_prpl(js->gc),

"jabber-receiving-iq", js->gc, iq_type, id, from, packet));

~~- if (signal_return)~~

+ if (signal_return) {

+ jabber_id_free(from_id);

return;

+ }

/* First, lets see if a special callback got registered */

if(type == JABBER_IQ_RESULT || type == JABBER_IQ_ERROR) {

if((jcd = g_hash_table_lookup(js->iq_callbacks, id))) {

~~- jcd->callback(js, from, type, id, packet, jcd->data);~~

~~- jabber_iq_remove_callback_by_id(js, id);~~

~~- return;~~

+ if(jabber_id_equal(js, jcd->to, from_id)) {

+ jcd->callback(js, from, type, id, packet, jcd->data);

+ jabber_iq_remove_callback_by_id(js, id);

+ jabber_id_free(from_id);

+ return;

+ } else {

+ char *expected_to;

+ if (jcd->to) {

+ expected_to = jabber_id_get_full_jid(jcd->to);

+ } else {

+ expected_to = jabber_id_get_bare_jid(js->user);

+ }

+ purple_debug_error("jabber", "Got a result iq with id %s from %s instead of expected %s!\n", id, from ? from : "(null)", expected_to);

+ g_free(expected_to);

+ }

}

@@ -372,12 +412,15 @@

if (signal_ref > 0) {

signal_return = GPOINTER_TO_INT(purple_signal_emit_return_1(purple_connection_get_prpl(js->gc), "jabber-watched-iq",

js->gc, iq_type, id, from, child));

~~- if (signal_return)~~

+ if (signal_return) {

+ jabber_id_free(from_id);

return;

+ }

}

if(jih) {

jih(js, from, type, id, child);

+ jabber_id_free(from_id);

return;

}

@@ -404,6 +447,8 @@

jabber_iq_send(iq);

}

+ jabber_id_free(from_id);

}

void jabber_iq_register_handler(const char *node, const char *xmlns, JabberIqHandler *handlerfunc)

~~--- a/libpurple/protocols/jabber/iq.h Sun Jan 12 19:29:36 2014 -0800~~

+++ b/libpurple/protocols/jabber/iq.h Sun Jan 12 22:51:33 2014 -0800

@@ -36,6 +36,7 @@

#include "connection.h"

typedef struct _JabberIq JabberIq;

+typedef struct _JabberIqCallbackData JabberIqCallbackData;

/**

* A JabberIqHandler is called to process an incoming IQ stanza.

@@ -96,6 +97,7 @@

void jabber_iq_parse(JabberStream *js, xmlnode *packet);

+void jabber_iq_callbackdata_free(JabberIqCallbackData *jcd);

void jabber_iq_remove_callback_by_id(JabberStream *js, const char *id);

void jabber_iq_set_callback(JabberIq *iq, JabberIqCallback *cb, gpointer data);

void jabber_iq_set_id(JabberIq *iq, const char *id);

~~--- a/libpurple/protocols/jabber/jabber.c Sun Jan 12 19:29:36 2014 -0800~~

+++ b/libpurple/protocols/jabber/jabber.c Sun Jan 12 22:51:33 2014 -0800

@@ -988,7 +988,7 @@

js->user_jb->subscription |= JABBER_SUB_BOTH;

js->iq_callbacks = g_hash_table_new_full(g_str_hash, g_str_equal,

~~- g_free, g_free);~~

+ g_free, (GDestroyNotify)jabber_iq_callbackdata_free);

js->chats = g_hash_table_new_full(g_str_hash, g_str_equal,

g_free, (GDestroyNotify)jabber_chat_free);

js->next_id = g_random_int();

~~--- a/libpurple/protocols/jabber/jutil.c Sun Jan 12 19:29:36 2014 -0800~~

+++ b/libpurple/protocols/jabber/jutil.c Sun Jan 12 22:51:33 2014 -0800

@@ -508,6 +508,34 @@

}

+gboolean

+jabber_id_equal(JabberStream *js, const JabberID *jid1, const JabberID *jid2)

+ const JabberID *j1, *j2;

+ JabberID *bare_user_jid;

+ gboolean equal;

+ /* If an outgoing stanza has no 'to', or an incoming has no 'from',

+ * then those are "the server acting as my account". This function will

+ * handle that correctly.

+ */

+ if (!jid1 && !jid2)

+ return TRUE;

+ bare_user_jid = jabber_id_to_bare_jid(js->user);

+ j1 = jid1 ? jid1 : bare_user_jid;

+ j2 = jid2 ? jid2 : bare_user_jid;

+ equal = purple_strequal(j1->node, j2->node) &&

+ purple_strequal(j1->domain, j2->domain) &&

+ purple_strequal(j1->resource, j2->resource);

+ jabber_id_free(bare_user_jid);

+ return equal;

char *jabber_get_domain(const char *in)

{

JabberID *jid = jabber_id_new(in);

@@ -536,6 +564,17 @@

return out;

}

+JabberID *

+jabber_id_to_bare_jid(const JabberID *jid)

+ JabberID *result = g_new0(JabberID, 1);

+ result->node = g_strdup(jid->node);

+ result->domain = g_strdup(jid->domain);

+ return result;

char *

jabber_get_bare_jid(const char *in)

{

@@ -561,6 +600,19 @@

NULL);

}

+char *

+jabber_id_get_full_jid(const JabberID *jid)

+ g_return_val_if_fail(jid != NULL, NULL);

+ return g_strconcat(jid->node ? jid->node : "",

+ jid->node ? "@" : "",

+ jid->domain,

+ jid->resource ? "/" : "",

+ jid->resource ? jid->resource : "",

+ NULL);

gboolean

jabber_jid_is_domain(const char *jid)

{

~~--- a/libpurple/protocols/jabber/jutil.h Sun Jan 12 19:29:36 2014 -0800~~

+++ b/libpurple/protocols/jabber/jutil.h Sun Jan 12 22:51:33 2014 -0800

@@ -44,12 +44,23 @@

#include "jabber.h"

JabberID* jabber_id_new(const char *str);

+/**

+ * Compare two JIDs for equality.

+ *

+ * Warning: If either JID is NULL then this function uses the user's

+ * bare JID, instead!

+ */

+gboolean jabber_id_equal(JabberStream *js, const JabberID *jid1, const JabberID *jid2);

void jabber_id_free(JabberID *jid);

char *jabber_get_domain(const char *jid);

char *jabber_get_resource(const char *jid);

char *jabber_get_bare_jid(const char *jid);

char *jabber_id_get_bare_jid(const JabberID *jid);

+char *jabber_id_get_full_jid(const JabberID *jid);

+JabberID *jabber_id_to_bare_jid(const JabberID *jid);

gboolean jabber_jid_is_domain(const char *jid);