Smack disconnects if <priority> is out of range

Smack Smack Dev
1

I’d like to report a potential issue. It looks like [SMACK-60] Invalid priority shouldn’t crash connection.

Smack does not seem to correctly check the value of the element of a received presence stanza.

The value of the element in a presence stanza MUST be an integer between -128 and +127.

If Smack receives a message with a value that is out of range the client gets disconnected (instead of setting it to 0).

See commit Assume Presence priority to be zero when priority is out of range. SM… · igniterealtime/Smack@7b62abf · GitHub

}
else if (elementName.equals("priority")) {
try {
int priority = Integer.parseInt(parser.nextText());
presence.setPriority(priority);
}
catch (NumberFormatException nfe) { }
catch (IllegalArgumentException iae) {
// Presence priority is out of range so assume priority to be zero
presence.setPriority(0);
}
}

There is no value check anymore in https://github.com/igniterealtime/Smack/blob/master/smack-core/src/main/java/org/jivesoftware/smack/util/PacketParserUtils.java

You can reproduce the issue as follows:

  • Smack client
  • Psi clients (set the priority to a value that is out of range)
  • Both clients join a chat room
  • Psi sends a message with wrong to the chat room
  • Smack client will get disconnected – user will no longer be allowed to join the room.

Can someone please confirm this?

2

I can confirm this issue. It occurred when communicating with a Psi client.

I submitted a pull request which adds a check in the parser on the value range. See

Presence Parser does only accept a valid priority number. by annovanvliet · Pull Request #130 · igniterealtime/Smack · G…

3

Thanks Anno for submitting the PR.

I’ve noticed that Spark has a similar issue (it doesn’t trust the presence packet received by Smack):

/core/src/main/java/org/jivesoftware/spark/ui/conferences/ConferenceServices.jav a

int priority = presence.getPriority();
//Sometimes priority is not set in the presence packet received. Make sure priority is in valid range
priority = (priority < -128 || priority > 128) ? 1 : priority;
final Presence p = new Presence(presence.getType(), presence.getStatus(), priority, presence.getMode())
4

The Proposed modification got rejected:

Works as intended. If you don’t want to trigger a reconnect, set an parsing exception handler. This of course, would mean that subsequent systems like Managers or the Roster will never see that particular presence stanza.
But this set me to a alternative solution by setting a ParsingExceptionCallback Function

myConnection.setParsingExceptionCallback(new ExceptionLoggingCallback());

Still the Presence message is ignored, but it does not result in a reconnect. It will still result in rooms where not all participants are visible.

5

Today, we’ve added Spark to our test environment, which doesn’t disconnect and seems to ignore the faulty Presence stanza that’s sent by Psi.

  1. Our Smack client
  2. Psi client
  3. Spark (Smack) client
  4. All three clients join the same chat room on Openfire
  5. Psi sends a message with wrong priority to the chat room
  6. Our Smack client will get disconnected – user will no longer be allowed to join the room.
  7. Spark and Psi will continue but both users have a different view:
  • Psi user will see 2 users in the chat room
  • Spark user will see only 1 user – but it can see messages sent from Psi.
    For Spark the Psi user appears as “Ghost” user.

For the Openfire the Psi user doesn’t appear as ghost user (XEP-0045: Multi-User Chat).

Does anyone have a suggestion?