Denial of Service Vulnerabilities

[Daniel Atallah]

On Tue, Mar 12, 2013 at 8:21 PM, Thijs Alkemade <thijs at adium.im> wrote:
> Attached is a patch fixing xmpp issue (2) and the extra issues I've mentioned.

> The patch is pretty straightforward, it stores the JabberID* that an iq query
> went to in the JabberCallbackData, and checks this when a reply comes in. I've
> also added a couple of helper functions to jutil.c for handling JIDs.
>
> One detail I feel like I should mention is that there is no timeout for
> outstanding iq replies, but maybe it should: previously this meant that only 2
> pointers per iq were stored, but now this is 7 pointers + whatever data is
> stored in the JabberID (which can be up to 3 kB (!) in total).
>
> A malicious contact could try to provoke queries to them from Pidgin, but
> refuse to reply, thus causing it to increase memory usage and performing a DoS
> attack. Extensions like XEP-0115: Entity Capabilities (implemented in Pidgin)
> and XEP-0166: Jingle (implemented for voice/video) make it possible for other
> contacts to trigger Pidgin to send a iq query to a contact.
>
> However, this would probably require something like 10k-100k outstanding
> queries to have an impact. That's more likely to DoS someone's connection than
> their iq-handler table.

Yeah, we probably should have some sort of timeout with a cleanup for
this.  It seems like we'd also need some sort of throttling of
duplicate requests, which may be more complicated.

> Regards,
> Thijs
>

This appears to work as advertised - a couple notes:

iq.c:397 - shouldn't this send an error and return?
The behavior now will be to expose the iq to "jabber-watched-iq" and
then to send a "feature-not-implemented" error.

jabber_id_equal(JabberStream *js, JabberID *jid_in1, JabberID
*jid_in2) - jid_in1 and jid_in2 should be const
Behavior of this function is a bit weird - it doesn't seem appropriate
to assume that it should use the user's jid if one of those is NULL.
Wouldn't it be more appropriate to explicitly pass the user's jid when
it should be used for the comparison?

-D