Stream: Add IQ response sender check

Verifies that the sender of the response is correct, so no evil entity
can inject responses.

Fixes #510.

5 files changed, 49 insertions, 17 deletions

diff --git a/src/base/QXmppStream.cpp b/src/base/QXmppStream.cpp
index 6b5d9589..c650b8ac 100644
--- a/src/base/QXmppStream.cpp
+++ b/src/base/QXmppStream.cpp
@@ -33,7 +33,11 @@ using namespace QXmpp::Private;
 static bool randomSeeded = false;
 #endif
 
-using IqState = QFutureInterface<QXmppStream::IqResult>;
+struct IqState
+{
+    QFutureInterface<QXmppStream::IqResult> interface;
+    QString jid;
+};
 
 class QXmppStreamPrivate
 {
@@ -219,7 +223,7 @@ QFuture<QXmppStream::IqResult> QXmppStream::sendIq(QXmppIq &&iq)
         iq.setId(QXmppUtils::generateStanzaUuid());
     }
 
-    return sendIq(QXmppPacket(iq), iq.id());
+    return sendIq(QXmppPacket(iq), iq.id(), iq.to());
 }
 
 ///
@@ -229,7 +233,7 @@ QFuture<QXmppStream::IqResult> QXmppStream::sendIq(QXmppIq &&iq)
 ///
 /// \since QXmpp 1.5
 ///
-QFuture<QXmppStream::IqResult> QXmppStream::sendIq(QXmppPacket &&packet, const QString &id)
+QFuture<QXmppStream::IqResult> QXmppStream::sendIq(QXmppPacket &&packet, const QString &id, const QString &to)
 {
     using namespace QXmpp;
 
@@ -239,6 +243,12 @@ QFuture<QXmppStream::IqResult> QXmppStream::sendIq(QXmppPacket &&packet, const Q
             SendError::Disconnected });
     }
 
+    if (to.isEmpty()) {
+        return makeReadyFuture<IqResult>(SendError {
+            QStringLiteral("The 'to' address must be set so the stream can match the response."),
+            SendError::Disconnected });
+    }
+
     auto sendFuture = send(std::move(packet));
     if (sendFuture.isFinished()) {
         if (std::holds_alternative<SendError>(sendFuture.result())) {
@@ -249,8 +259,8 @@ QFuture<QXmppStream::IqResult> QXmppStream::sendIq(QXmppPacket &&packet, const Q
         awaitLast(sendFuture, this, [this, id](SendResult result) {
             if (std::holds_alternative<SendError>(result)) {
                 if (auto itr = d->runningIqs.find(id); itr != d->runningIqs.end()) {
-                    itr.value().reportResult(std::get<SendError>(result));
-                    itr.value().reportFinished();
+                    itr.value().interface.reportResult(std::get<SendError>(result));
+                    itr.value().interface.reportFinished();
 
                     d->runningIqs.erase(itr);
                 }
@@ -258,9 +268,13 @@ QFuture<QXmppStream::IqResult> QXmppStream::sendIq(QXmppPacket &&packet, const Q
         });
     }
 
-    IqState interface(IqState::Started);
-    d->runningIqs.insert(id, interface);
-    return interface.future();
+    IqState state {
+        QFutureInterface<IqResult>(QFutureInterfaceBase::Started),
+        to,
+    };
+    auto future = state.interface.future();
+    d->runningIqs.insert(id, std::move(state));
+    return future;
 }
 
 ///
@@ -271,10 +285,10 @@ QFuture<QXmppStream::IqResult> QXmppStream::sendIq(QXmppPacket &&packet, const Q
 void QXmppStream::cancelOngoingIqs()
 {
     for (auto &state : d->runningIqs) {
-        state.reportResult(QXmpp::SendError {
+        state.interface.reportResult(QXmpp::SendError {
             QStringLiteral("IQ has been cancelled."),
             QXmpp::SendError::Disconnected });
-        state.reportFinished();
+        state.interface.reportFinished();
     }
     d->runningIqs.clear();
 }
@@ -475,9 +489,13 @@ bool QXmppStream::handleIqResponse(const QDomElement &stanza)
 
     if (auto itr = d->runningIqs.find(stanza.attribute(QStringLiteral("id")));
         itr != d->runningIqs.end()) {
+        if (stanza.attribute("from") != itr.value().jid) {
+            warning(QStringLiteral("Received IQ response to one of our requests from wrong sender. Ignoring."));
+            return false;
+        }
 
-        itr.value().reportResult(stanza);
-        itr.value().reportFinished();
+        itr.value().interface.reportResult(stanza);
+        itr.value().interface.reportFinished();
 
         d->runningIqs.erase(itr);
         return true;
diff --git a/src/base/QXmppStream.h b/src/base/QXmppStream.h
index aba73785..f8c4e548 100644
--- a/src/base/QXmppStream.h
+++ b/src/base/QXmppStream.h
@@ -46,7 +46,7 @@ public:
 
     using IqResult = std::variant<QDomElement, QXmpp::SendError>;
     QFuture<IqResult> sendIq(QXmppIq &&);
-    QFuture<IqResult> sendIq(QXmppPacket &&, const QString &id);
+    QFuture<IqResult> sendIq(QXmppPacket &&, const QString &id, const QString &to);
     void cancelOngoingIqs();
     bool hasIqId(const QString &id) const;
 
diff --git a/src/client/QXmppClient.cpp b/src/client/QXmppClient.cpp
index 174c4219..a6d55fa8 100644
--- a/src/client/QXmppClient.cpp
+++ b/src/client/QXmppClient.cpp
@@ -517,12 +517,12 @@ QFuture<QXmppClient::IqResult> QXmppClient::sendIq(QXmppIq &&iq, const std::opti
 ///
 QFuture<QXmppClient::IqResult> QXmppClient::sendSensitiveIq(QXmppIq &&iq, const std::optional<QXmppSendStanzaParams> &params)
 {
-    const auto sendEncrypted = [this](QFuture<IqEncryptResult> &&future, const QString &id) {
+    const auto sendEncrypted = [this](QFuture<IqEncryptResult> &&future, const QString &id, const QString &to) {
         QFutureInterface<IqResult> interface(QFutureInterfaceBase::Started);
-        await(future, this, [this, interface, id](IqEncryptResult result) mutable {
+        await(future, this, [this, interface, id, to](IqEncryptResult result) mutable {
             if (const auto *xml = std::get_if<QByteArray>(&result)) {
                 // encrypted successfully
-                auto future = d->stream->sendIq(QXmppPacket(*xml, true), id);
+                auto future = d->stream->QXmppStream::sendIq(QXmppPacket(*xml, true), id, to);
                 await(future, this, [this, interface](QXmppStream::IqResult result) mutable {
                     if (const auto encryptedDom = std::get_if<QDomElement>(&result)) {
                         if (!isIqResponse(*encryptedDom)) {
@@ -571,10 +571,13 @@ QFuture<QXmppClient::IqResult> QXmppClient::sendSensitiveIq(QXmppIq &&iq, const
     if (iq.id().isEmpty() || d->stream->hasIqId(iq.id())) {
         iq.setId(QXmppUtils::generateStanzaUuid());
     }
+    if (iq.to().isEmpty()) {
+        iq.setTo(d->stream->configuration().domain());
+    }
 
     if (d->encryptionExtension) {
         const auto id = iq.id();
-        return sendEncrypted(d->encryptionExtension->encryptIq(std::move(iq), params), id);
+        return sendEncrypted(d->encryptionExtension->encryptIq(std::move(iq), params), id, iq.to());
     }
     return d->stream->sendIq(std::move(iq));
 }
diff --git a/src/client/QXmppOutgoingClient.cpp b/src/client/QXmppOutgoingClient.cpp
index 7d1ad6a3..9a8e51b8 100644
--- a/src/client/QXmppOutgoingClient.cpp
+++ b/src/client/QXmppOutgoingClient.cpp
@@ -19,6 +19,7 @@
 
 #include <QCryptographicHash>
 #include <QDnsLookup>
+#include <QFuture>
 #include <QNetworkProxy>
 #include <QSslConfiguration>
 #include <QSslSocket>
@@ -316,6 +317,15 @@ bool QXmppOutgoingClient::isStreamResumed() const
     return d->streamResumed;
 }
 
+QFuture<QXmppStream::IqResult> QXmppOutgoingClient::sendIq(QXmppIq &&iq)
+{
+    // always set a to address (the QXmppStream needs this for matching)
+    if (iq.to().isEmpty()) {
+        iq.setTo(d->config.domain());
+    }
+    return QXmppStream::sendIq(std::move(iq));
+}
+
 void QXmppOutgoingClient::_q_socketDisconnected()
 {
     debug("Socket disconnected");
diff --git a/src/client/QXmppOutgoingClient.h b/src/client/QXmppOutgoingClient.h
index 57e3477a..6c72d076 100644
--- a/src/client/QXmppOutgoingClient.h
+++ b/src/client/QXmppOutgoingClient.h
@@ -38,6 +38,7 @@ public:
     bool isClientStateIndicationEnabled() const;
     bool isStreamManagementEnabled() const;
     bool isStreamResumed() const;
+    QFuture<IqResult> sendIq(QXmppIq &&);
 
     /// Returns the used socket
     QSslSocket *socket() const { return QXmppStream::socket(); };