Calculating a XEP-0115 hash involves sorting the fields of data forms in the contact's capabilities according to their field name. wocky_caps_hash_compute_from_lists() does this using this function: gint wocky_data_form_field_cmp (const WockyDataFormField *left, const WockyDataFormField *right) { return strcmp (left->var, right->var); } This will obviously crash if either ->var is NULL, but WockyDataForm validates forms it parses so this shouldn't happen, right? Unfortunately, http://xmpp.org/extensions/xep-0004.html#protocol-field says: > If the <field/> element type is anything other than "fixed" (see below), it > MUST possess a 'var' attribute that uniquely identifies the field in the > context of the form (if it is "fixed", it MAY possess a 'var' attribute). And WockyDataForm implements this correctly: var = wocky_node_get_attribute (node, "var"); if (var == NULL && type != WOCKY_DATA_FORM_FIELD_TYPE_FIXED) { DEBUG ("field node doesn't have a 'var' attribute; ignoring"); return FALSE; } So we have a remotely-triggered DoS: send Gabble a <presence> with a caps hash; include a form with an anonymous fixed field in the reply; boom. Since anyone can send presence to anyone else, and Gabble always looks up any caps it sees in any presences it receives. (Note that this is a presence leak, too; another bug, I think.) I have some fixes; patches to follow. I will also suggest that http://xmpp.org/extensions/xep-0115.html be updated to mention this.
Here's the fix in Wocky: http://cgit.collabora.com/git/user/wjt/wocky/log/?h=61433-caps-hash-crash Here's a test in Wocky: http://cgit.collabora.com/git/user/wjt/wocky/log/?h=61433-caps-hash-crash-test Here's the Gabble 0.16 branch which pulls in just the fixes to Wocky: http://cgit.collabora.com/git/user/wjt/telepathy-gabble/log/?h=61433-caps-hash-crash Here's a branch on top of that which adds a test to Gabble, too: http://cgit.collabora.com/git/user/wjt/telepathy-gabble/log/?h=61433-caps-hash-crash-test Ubuntu Oneiric has 0.13.5 to which the same patches should apply. Debian testing and Ubuntu's current LTS has 0.16.x so a release on that series will do. Maemo Harmattan has 0.12.6 which has the same bug, but the code in that version lives in Gabble, not Wocky, so dataforms_cmp and caps_hash_compute in src/caps-hash.c would need equivalent fixes.
Do we have a CVE ID for this, or a plan to get one? Kurt Seifried of the Red Hat security team <secalert@redhat.com> does most of the allocations on oss-security, and has offered to allocate CVEs before advisories go out. Patches all look good to me, r+. Someone will have to backport this to 0.9.x for Debian stable :-(
Created attachment 75495 [details] [review] [0.12 1/6] Turn off deprecation warnings, this is a stable branch Also do the same for our Wocky and Yell submodules, by applying the corresponding patches to them. --- Wocky and Yell patches not attached, but they're the same change.
Created attachment 75496 [details] [review] [0.12 2/6] Cherry-pick more Wocky commits to fix compilation with newer libsasl --- I'll attach them in a moment.
Created attachment 75497 [details] [review] [0.12 2a/6] tests: Fix compilation error with libsasl2 From: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
Created attachment 75498 [details] [review] [0.12 2b/6] sasl test: fix build failure on older libsasl2s From: Jonny Lamb <jonny.lamb@collabora.co.uk> A second try. Signed-off-by: Jonny Lamb <jonny.lamb@collabora.co.uk> Backported: Simon McVittie <simon.mcvittie@collabora.co.uk> --- I screwed up the attribution on the version referenced by Attachment #75496 [details], sorry. This one is correct.
Created attachment 75499 [details] [review] [0.12 2/6] Cherry-pick more Wocky commits to fix compilation with newer libsasl
Created attachment 75500 [details] [review] [0.12 3/6] jingle/outgoing-basics: disregard MembersChangedDetailed flag From: Will Thompson <will.thompson@collabora.co.uk> In 0.15, 65c8b8f makes this test always expect the Members_Changed_Detailed flag to be present in the channel's group flags. But 0.14 doesn't depend on a new enough telepathy-glib to be sure of that; so here we just mask that flag.
Created attachment 75501 [details] [review] [0.12 4/6] presence-cache: don't crash if computed_hash == NULL From: David Laban <david.laban@collabora.co.uk> [Necessary to fix #61433. -smcv]
Created attachment 75502 [details] [review] [0.12 5/6] fd.o #61433: caps hash: make all field comparisons NULL-safe This is less targeted than wjt's version in Wocky - some of these are probably guaranteed to be non-NULL anyway, but it's quicker to g_strcmp0 everywhere.
Created attachment 75503 [details] [review] [0.12 6/6] Test for #61433 and #39464 From: Will Thompson <will.thompson@collabora.co.uk> Backported: Simon McVittie <simon.mcvittie@collabora.co.uk>
(In reply to comment #9) > [0.12 4/6] presence-cache: don't crash if computed_hash == NULL > [0.12 5/6] fd.o #61433: caps hash: make all field comparisons NULL-safe These two also apply to Debian stable's 0.9.15.
(In reply to comment #10) > Created attachment 75502 [details] [review] [review] > [0.12 5/6] fd.o #61433: caps hash: make all field comparisons NULL-safe > > This is less targeted than wjt's version in Wocky - some of these are > probably guaranteed to be non-NULL anyway, but it's quicker to > g_strcmp0 everywhere. Inspired by this, I've found some other cases which crash in the Wocky code…
We have been assigned CVE-2013-1769 for this issue by Kurt Seifried of the Red Hat Security Response Team.
The new tip of http://cgit.collabora.com/git/user/wjt/wocky/log/?h=61433-caps-hash-crash fixes two crashes caused by FORM_TYPE fields without <value>s. http://cgit.collabora.com/git/user/wjt/wocky/log/?h=61433-caps-hash-crash-test has a test for that patch. It also has a test for the case where FORM_TYPE has 2 <value>s, which XEP-0115 says should make the algorithm fail. Wocky currently ignores everything after the first one because raw_value_contents is not actually the raw value contents, so that test fails. This separate bug doesn't have any security impact that I can see. If Wocky were to include the extra <value>s in the hash calculation, it might be possible to collide with the hash of another form. If this: <field name='FORM_TYPE' type='hidden'> <value>bar</value> <value>foo</value> </field> <field name='alpha' type='text-single'> <value>omega</value> </field> was erroneously encoded by Wocky as "…bar<foo<alpha<omega<…", this would collide with: <field name='FORM_TYPE' type='hidden'> <value>bar</value> </field> <field name='foo' type='text-single'> <value>alpha</value> <value>omega</value> </field> which legitimately is encoded as "…bar<foo<alpha<omega<…". But Wocky does not do this.
Simon: all your 0.12 patches look fine.
This is fixed in 0.16.5 and 0.17.3.
I believe that telepathy-gabble 0.8 is also affected. Simon's backported fixes for the 0.12 branch (specifically attachment 75501 [details] [review] and attachment 75502 [details] [review]) should apply pretty cleanly and fix the bug in the 0.8 branch too.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.