Denial of Service Vulnerabilities
Mark Doliner
mark at kingant.net
Mon Jan 13 02:37:49 EST 2014
Hi Fabian. Thanks again for reporting these problems to us, and I'm
sorry it is taking us forever to release fixes. I'm hoping to request
CVE numbers from our Red Hat contact in the next few days, and I'm
hoping to Pidgin 2.10.8 with fixes on Jan 23.
Can we credit you for discovering these problems? Who should we
credit? "Fabian Yamaguchi and Christian Wressnegger of the University
of Goettingen"?
I've attached our fixes for problems 1 through 4 to this email. Feel
free to review if you'd like, and please let us know if you see
further problems. These are the bugs we'll be requesting CVE numbers
for.
A note on #2 (jabber_vcard_parse): It should not have been possible to
spoof iq replies. Thijs Alkemade wrote a patch to fix this. I believe
this checking is enough to prevent the NULL-pointer dereference.
A note on #5 (mxit_add_buddy): We believe this requires action by the
local user to trigger the crash, and we think it's pretty unlikely
(malicious server or man-in-the-middle would need to send a malicious
response to a search, I think, and then the local user would need to
choose to add a buddy with a malformed name, and it'll only crash on
systems like Windows that don't guard against fprintf("%s", NULL) and
only with older versions of GLib). So we don't plan to request a CVE
number for it. We fixed this in our public release-2.x.y branch last
March: https://hg.pidgin.im/pidgin/main/rev/9eb08b587d95
-------------- next part --------------
diff -r 932b985540e9 -r 23cbfff68a0c libpurple/protocols/msn/msg.c
--- a/libpurple/protocols/msn/msg.c Sat Mar 16 14:05:51 2013 -0400
+++ b/libpurple/protocols/msn/msg.c Sat Mar 16 14:17:45 2013 -0400
@@ -178,6 +178,8 @@ msn_message_parse_payload(MsnMessage *ms
g_free(tmp_base);
g_return_if_reached();
}
+
+ /* NUL-terminate the end of the headers - it'll get skipped over below */
*end = '\0';
/* Split the headers and parse each one */
@@ -195,10 +197,12 @@ msn_message_parse_payload(MsnMessage *ms
/* The only one I care about is 'boundary' (which is folded from
the key 'Content-Type'), so only process that. */
- if (!strcmp(key, "boundary")) {
+ if (!strcmp(key, "boundary") && value) {
char *end = strchr(value, '\"');
- *end = '\0';
- msn_message_set_header(msg, key, value);
+ if (end) {
+ *end = '\0';
+ msn_message_set_header(msg, key, value);
+ }
}
g_strfreev(tokens);
@@ -210,18 +214,15 @@ msn_message_parse_payload(MsnMessage *ms
key = tokens[0];
value = tokens[1];
- /*if not MIME content ,then return*/
if (!strcmp(key, "MIME-Version"))
{
- g_strfreev(tokens);
- continue;
+ /* Ignore MIME-Version header */
}
-
- if (!strcmp(key, "Content-Type"))
+ else if (!strcmp(key, "Content-Type"))
{
char *charset, *c;
- if ((c = strchr(value, ';')) != NULL)
+ if (value && (c = strchr(value, ';')) != NULL)
{
if ((charset = strchr(c, '=')) != NULL)
{
-------------- next part --------------
diff -r ed1f9a0c0979 -r 93d4bff19574 ChangeLog
--- a/ChangeLog Sun Jan 12 19:29:36 2014 -0800
+++ b/ChangeLog Sun Jan 12 22:51:33 2014 -0800
@@ -69,6 +69,10 @@ version 2.10.8:
(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
diff -r ed1f9a0c0979 -r 93d4bff19574 libpurple/protocols/jabber/iq.c
--- 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 @@ JabberIq *jabber_iq_new_query(JabberStre
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_set_id(JabberIq *iq, cons
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_remove_callback_by_id(Jab
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 @@ void jabber_iq_parse(JabberStream *js, x
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 @@ void jabber_iq_parse(JabberStream *js, x
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 @@ void jabber_iq_parse(JabberStream *js, x
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 @@ void jabber_iq_parse(JabberStream *js, x
jabber_iq_send(iq);
}
+
+ jabber_id_free(from_id);
}
void jabber_iq_register_handler(const char *node, const char *xmlns, JabberIqHandler *handlerfunc)
diff -r ed1f9a0c0979 -r 93d4bff19574 libpurple/protocols/jabber/iq.h
--- 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 @@ typedef enum {
#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 @@ JabberIq *jabber_iq_new_query(JabberStre
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);
diff -r ed1f9a0c0979 -r 93d4bff19574 libpurple/protocols/jabber/jabber.c
--- 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 @@ jabber_stream_new(PurpleAccount *account
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();
diff -r ed1f9a0c0979 -r 93d4bff19574 libpurple/protocols/jabber/jutil.c
--- 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 @@ jabber_id_free(JabberID *jid)
}
}
+
+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 @@ char *jabber_get_resource(const char *in
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 @@ jabber_id_get_bare_jid(const JabberID *j
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)
{
diff -r ed1f9a0c0979 -r 93d4bff19574 libpurple/protocols/jabber/jutil.h
--- 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 @@ typedef enum {
#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);
-------------- next part --------------
diff -r 23cbfff68a0c -r ef836278304b libpurple/protocols/msn/oim.c
--- a/libpurple/protocols/msn/oim.c Sat Mar 16 14:17:45 2013 -0400
+++ b/libpurple/protocols/msn/oim.c Sat Mar 16 14:17:45 2013 -0400
@@ -824,10 +824,10 @@ msn_parse_oim_xml(MsnOim *oim, xmlnode *
char *unread = xmlnode_get_data(iu_node);
const char *passports[2] = { msn_user_get_passport(session->user) };
const char *urls[2] = { session->passport_info.mail_url };
- int count = atoi(unread);
+ int count;
/* XXX/khc: pretty sure this is wrong */
- if (count > 0)
+ if (unread && (count = atoi(unread)) > 0)
purple_notify_emails(session->account->gc, count, FALSE, NULL,
NULL, passports, urls, NULL, NULL);
g_free(unread);
-------------- next part --------------
diff -r ef836278304b -r 68d6df7dc69c libpurple/protocols/msn/oim.c
--- a/libpurple/protocols/msn/oim.c Sat Mar 16 14:17:45 2013 -0400
+++ b/libpurple/protocols/msn/oim.c Sat Mar 16 14:17:45 2013 -0400
@@ -362,11 +362,12 @@ msn_oim_send_read_cb(MsnSoapMessage *req
if (faultcode) {
char *faultcode_str = xmlnode_get_data(faultcode);
- if (g_str_equal(faultcode_str, "q0:AuthenticationFailed")) {
+ if (faultcode_str && g_str_equal(faultcode_str, "q0:AuthenticationFailed")) {
xmlnode *challengeNode = xmlnode_get_child(faultNode,
"detail/LockKeyChallenge");
+ char *challenge = NULL;
- if (challengeNode == NULL) {
+ if (challengeNode == NULL || (challenge = xmlnode_get_data(challengeNode)) == NULL) {
if (oim->challenge) {
g_free(oim->challenge);
oim->challenge = NULL;
@@ -384,7 +385,6 @@ msn_oim_send_read_cb(MsnSoapMessage *req
} else {
char buf[33];
- char *challenge = xmlnode_get_data(challengeNode);
msn_handle_chl(challenge, buf);
g_free(oim->challenge);
@@ -400,22 +400,23 @@ msn_oim_send_read_cb(MsnSoapMessage *req
}
} else {
/* Report the error */
- const char *str_reason;
+ const char *str_reason = NULL;
- if (g_str_equal(faultcode_str, "q0:SystemUnavailable")) {
- str_reason = _("Message was not sent because the system is "
- "unavailable. This normally happens when the "
- "user is blocked or does not exist.");
+ if (faultcode_str) {
+ if (g_str_equal(faultcode_str, "q0:SystemUnavailable")) {
+ str_reason = _("Message was not sent because the system is "
+ "unavailable. This normally happens when the "
+ "user is blocked or does not exist.");
+ } else if (g_str_equal(faultcode_str, "q0:SenderThrottleLimitExceeded")) {
+ str_reason = _("Message was not sent because messages "
+ "are being sent too quickly.");
+ } else if (g_str_equal(faultcode_str, "q0:InvalidContent")) {
+ str_reason = _("Message was not sent because an unknown "
+ "encoding error occurred.");
+ }
+ }
- } else if (g_str_equal(faultcode_str, "q0:SenderThrottleLimitExceeded")) {
- str_reason = _("Message was not sent because messages "
- "are being sent too quickly.");
-
- } else if (g_str_equal(faultcode_str, "q0:InvalidContent")) {
- str_reason = _("Message was not sent because an unknown "
- "encoding error occurred.");
-
- } else {
+ if (str_reason == NULL) {
str_reason = _("Message was not sent because an unknown "
"error occurred.");
}
diff -r ef836278304b -r 68d6df7dc69c libpurple/protocols/msn/soap.c
--- a/libpurple/protocols/msn/soap.c Sat Mar 16 14:17:45 2013 -0400
+++ b/libpurple/protocols/msn/soap.c Sat Mar 16 14:17:45 2013 -0400
@@ -304,21 +304,25 @@ msn_soap_handle_body(MsnSoapConnection *
if (faultcode != NULL) {
char *faultdata = xmlnode_get_data(faultcode);
- if (g_str_equal(faultdata, "psf:Redirect")) {
+ if (faultdata && g_str_equal(faultdata, "psf:Redirect")) {
xmlnode *url = xmlnode_get_child(fault, "redirectUrl");
if (url) {
char *urldata = xmlnode_get_data(url);
- msn_soap_handle_redirect(conn, urldata);
+ if (urldata)
+ msn_soap_handle_redirect(conn, urldata);
g_free(urldata);
}
g_free(faultdata);
msn_soap_message_destroy(response);
return TRUE;
- } else if (g_str_equal(faultdata, "wsse:FailedAuthentication")) {
+ } else if (faultdata && g_str_equal(faultdata, "wsse:FailedAuthentication")) {
xmlnode *reason = xmlnode_get_child(fault, "faultstring");
- char *reasondata = xmlnode_get_data(reason);
+ char *reasondata = NULL;
+
+ if (reason)
+ reasondata = xmlnode_get_data(reason);
msn_soap_connection_sanitize(conn, TRUE);
msn_session_set_error(conn->session, MSN_ERROR_AUTH,
More information about the security
mailing list