ParseRoster does not check the sender of the roster and for pending roster queries
Description
Environment
Activity
Florian Schmaus September 11, 2014 at 7:43 AM
No, this is a bug in the server implementation: RFC 6121 2.1.6 clearly states that roster pushes either have no from or have a from set to the bare JID of the user. "A receiving client MUST ignore the stanza unless it has no 'from' attribute (i.e., implicitly from the bare JID of the user's account) or it has a 'from' attribute whose value matches the user's bare JID <user@domainpart>."
Please report the issue to the server developers.
And, in future, please don't use the Smack bug tracker to make such inquiries, use the forum instead!
Emmet McPoland September 11, 2014 at 1:14 AM
It would appear there is an issue with the following.
jid is the bareAddress.
however from isnt
So im currently getting an issue where
W/Roster﹕ Ignoring roster push with a non matching 'from' ourJid=e@xmpp.com from=e@xmpp.com/e8009b64c15b6b694fb3b1c6eb357279
I dont believe this is correct behaviour. The server is ejabberd.
https://www.igniterealtime.org/builds/smack/docs/latest/javadoc/src-html/org/jivesoftware/smack/Roster.html
1031 String version = rosterPacket.getVersion();
1032
1033 // Roster push (RFC 6121, 2.1.6)
1034 // A roster push with a non-empty from not matching our address MUST be ignored
1035 String jid = StringUtils.parseBareAddress(connection.getUser());
1036 if (rosterPacket.getFrom() != null &&
1037 !rosterPacket.getFrom().equals(jid)) {
1038 LOGGER.warning("Ignoring roster push with a non matching 'from' ourJid=" + jid
1039 + " from=" + rosterPacket.getFrom());
1040 return;
1041 }
1042
Lars Noschinski February 23, 2014 at 4:52 PM
Hm. No, the roster versioning patches (c06b0a7720ee) only check the from for roster pushes, not roster results. We also don't check whether a roster request is pending – but this should not matter much, once we correctly check the 'from'.
Lars Noschinski February 23, 2014 at 9:26 AM
So this refers to the necessity of checking the 'from' in roster pushes (as mandated by XEP-0237 resp. RFC-6121, 2.1.6)? I believe this is fixed in my pending patches to implement roster versioning.
Thijs Alkemade February 3, 2014 at 4:39 PM
Sorry for replying in the wrong place.
I've read XEP-0237 more carefully, and I've realized these are roster pushes, not roster replies, so my comment about pending requests is invalid. Checking the from should still be done, of course.
Reported by Thijs Alkemade: parseRoster @ PacketParserUtils.java:415 does not check the sender of the roster at all but also does not care whether or not a roster query was pending. This means that anybody can add new roster contacts, with a chosen alias and chosen groups. I've verified that this works in Yaxim.