Smack IQ not correctly implemented with null to field

Smack Smack Dev
1

Hi Flow,

Not sure if this is an issue but smack’s iq with no to field or bare jid it doesn’t expect a from filter . Ejabberd when given an iq with no to returns a response with barejid: jabberd:iq:roster.

Smack’s fromfilter - https://github.com/igniterealtime/Smack/blob/b18c6dac62bf6d1da8c0f8ab159d426b257 e4754/smack-core/src/main/java/org/jivesoftware/smack/filter/FromMatchesFilter.j ava- The address to filter for. If null is given, the stanza(/packet) must not have a from address.

RFC6120 - https://tools.ietf.org/html/rfc6120#section-8.1.1
When the server generates a stanza from the server for delivery to the client on behalf of the account of the connected client (e.g., in the context of data storage services provided by the
server on behalf of the client), the stanza MUST either (a) not include a ‘from’ attribute or (b) include a ‘from’ attribute whose value is the account’s bare JID (localpart@domainpart).

Packet

In IQReplyFilter

fromFilter = new OrFilter();
fromFilter.addFilter(FromMatchesFilter.createFull(to));
if (to == null) {
fromFilter.addFilter(FromMatchesFilter.createBare(local));
fromFilter.addFilter(FromMatchesFilter.createFull(server));
}
else if (to.equals(local.asBareJid())) {
fromFilter.addFilter(FromMatchesFilter.createFull(null));
}

to is null and therefore fromfilter first condition is null. In abstrctjid it’s giving null pointer since from field is present in iq result. It looks wrong to me.

2

A fix which is working with ejabberd - https://github.com/bangarharshit/Smack/commit/4872e434bc687d9fec794d003aabc0f84b 9c03ca . Still need to fix unit test but it will be great if you can take a look at it. I will also test with openfire which doesn’t add a to field.

3

Could you show us the stacktrace of the NPE?

4

I have changed code to print stack trace

Oct 28, 2015 10:17:12 PM org.jivesoftware.smack.filter.IQReplyFilter accept

SEVERE: java.lang.NullPointerException

java.lang.NullPointerException

at org.jxmpp.jid.impl.AbstractJid.equals(AbstractJid.java:145)

at org.jivesoftware.smack.filter.FromMatchesFilter.accept(FromMatchesFilter.java:1 04)

at org.jivesoftware.smack.filter.OrFilter.accept(OrFilter.java:50)

at org.jivesoftware.smack.filter.IQReplyFilter.accept(IQReplyFilter.java:119)

at org.jivesoftware.smack.AbstractXMPPConnection$ListenerWrapper.filterMatches(Abs tractXMPPConnection.java:1269)

at org.jivesoftware.smack.AbstractXMPPConnection.invokePacketCollectorsAndNotifyRe cvListeners(AbstractXMPPConnection.java:1109)

at org.jivesoftware.smack.AbstractXMPPConnection$2.run(AbstractXMPPConnection.java :1013)

at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)

at java.util.concurrent.FutureTask.run(FutureTask.java:262)

at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)

at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)

at java.lang.Thread.run(Thread.java:745)

// Second, check if the from attributes are correct and log potential IQ spoofing attempts
try {
if (fromFilter.accept(packet)) {
return true;
} else {
String msg = String.format("Rejected potentially spoofed reply to IQ-packet. Filter settings: "

  • “packetId=%s, to=%s, local=%s, server=%s. Received packet with from=%s”,
    packetId, to, local, server, packet.getFrom());
    LOGGER.log(Level.WARNING, msg , packet);
    return false;
    }
    }
    catch (Exception e) {
    LOGGER.log(Level.SEVERE, e.toString(), e);
    }
    return false;
    }
5

null.tostring is called when from field is present in iq result when iq input doesn’t have to. I just have change the order to prevent that NPE - Fixed code for iq · bangarharshit/Smack@6ca36e3 · GitHub Sorry for late reply.

6

I think the correct fix is https://github.com/Flowdalic/jxmpp/commit/c553a64eb83b295a11fa498858dd439e8334ff 58

7

Thanks. That will fix it. I will patch it locally. Once new smack is released I will upgrade it. BTW are you planning release for smack (alpha or non alpha) soon I will then pick up from there itself.

8

Thanks to you for reporting

I think there will be another alpha of 4.2 within the next 4-8 weeks.