We're updating the issue view to help you get more done. 

ParseRoster does not check the sender of the roster and for pending roster queries

Description

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.

Environment

None

Acceptance Test - Entry

None

Activity

Show:
Thijs Alkemade
February 3, 2014, 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.

Lars Noschinski
February 23, 2014, 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.

Lars Noschinski
February 23, 2014, 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'.

Emmet McPoland
September 11, 2014, 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

Florian Schmaus
September 11, 2014, 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!

Assignee

Lars Noschinski

Reporter

Florian Schmaus

Labels

None

Expected Effort

None

Ignite Forum URL

None

Components

Fix versions

Affects versions

Priority

Critical
Configure