Merge branch 'stable'

4 files changed, 110 insertions, 20 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0fd08f65..7aaf7f24 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,18 @@ QXmpp 1.4.0 (UNRELEASED)
 
 *under development*
 
+QXmpp 1.3.1 (Jul 20, 2020)
+--------------------------
+
+The most important change of this release is the fix of CVE-2017-5603. QXmpp is
+not vulnerable to roster push attacks (CVE-2016-9928).
+
+Fixes:
+ - QXmppRosterIq: Set subscriptionType to NotSet correctly (#293, @melvo)
+ - Fix `QXMPP_EXPORT` define when linking statically (#299, @leobasilio)
+ - QXmppMessageReceiptManager: Ignore all error messages (#300, @lnjX)
+ - QXmppCarbonManager: Fix CVE-2017-5603 (missing sender check) (#304, @lnjX)
+
 QXmpp 1.3.0 (Apr 06, 2020)
 --------------------------
 
diff --git a/src/client/QXmppCarbonManager.cpp b/src/client/QXmppCarbonManager.cpp
index d403e6de..45dd96da 100644
--- a/src/client/QXmppCarbonManager.cpp
+++ b/src/client/QXmppCarbonManager.cpp
@@ -40,21 +40,24 @@ QXmppCarbonManager::~QXmppCarbonManager()
 {
 }
 
+///
 /// Returns whether message carbons are currently enabled
-
+///
 bool QXmppCarbonManager::carbonsEnabled() const
 {
     return m_carbonsEnabled;
 }
 
-/// Enable or disable message carbons.
+///
+/// Enables or disables message carbons for this connection.
+///
 /// This function does not check whether the server supports
 /// message carbons, but just sends the corresponding stanza
 /// to the server, so one must check in advance by using the
 /// discovery manager.
 ///
 /// By default, carbon copies are disabled.
-
+///
 void QXmppCarbonManager::setCarbonsEnabled(bool enabled)
 {
     if (m_carbonsEnabled == enabled)
@@ -73,6 +76,7 @@ void QXmppCarbonManager::setCarbonsEnabled(bool enabled)
     }
 }
 
+/// \cond
 QStringList QXmppCarbonManager::discoveryFeatures() const
 {
     return QStringList() << ns_carbons;
@@ -91,18 +95,21 @@ bool QXmppCarbonManager::handleStanza(const QDomElement &element)
     }
 
     if (carbon.isNull() || carbon.namespaceURI() != ns_carbons)
-        return false;  // Neither sent nor received -> no carbon message
+        return false;
 
-    QDomElement forwarded = carbon.firstChildElement("forwarded");
-    if (forwarded.isNull())
+    // carbon copies must always come from our bare JID
+    if (element.attribute("from") != client()->configuration().jidBare()) {
+        info("Received carbon copy from possible attacker trying to use CVE-2017-5603.");
         return false;
+    }
 
-    QDomElement messageelement = forwarded.firstChildElement("message");
-    if (messageelement.isNull())
+    auto forwarded = carbon.firstChildElement("forwarded");
+    auto messageElement = forwarded.firstChildElement("message");
+    if (messageElement.isNull())
         return false;
 
     QXmppMessage message;
-    message.parse(messageelement);
+    message.parse(messageElement);
 
     if (sent)
         emit messageSent(message);
@@ -111,3 +118,20 @@ bool QXmppCarbonManager::handleStanza(const QDomElement &element)
 
     return true;
 }
+/// \endcond
+
+///
+/// \fn QXmppCarbonManager::messageReceived()
+///
+/// Emitted when a message was received from someone else and directed to
+/// another resource.
+///
+/// If you connect this signal to the QXmppClient::messageReceived signal, they
+/// will appear as normal messages.
+///
+
+///
+/// \fn QXmppCarbonManager::messageSent()
+///
+/// Emitted when another resource sent a message to someone else.
+///
diff --git a/src/client/QXmppCarbonManager.h b/src/client/QXmppCarbonManager.h
index d1e113be..33a39627 100644
--- a/src/client/QXmppCarbonManager.h
+++ b/src/client/QXmppCarbonManager.h
@@ -56,14 +56,7 @@ public:
     /// \endcond
 
 Q_SIGNALS:
-    /// \brief Emitted when a message was received from someone else
-    /// and directed to another resource.
-    /// If you connect this signal to the \s QXmppClient::messageReceived
-    /// signal, they will appear as normal messages.
     void messageReceived(const QXmppMessage &);
-
-    /// \brief Emitted when another resource sent a message to
-    /// someone else
     void messageSent(const QXmppMessage &);
 
 private:
diff --git a/tests/qxmppcarbonmanager/tst_qxmppcarbonmanager.cpp b/tests/qxmppcarbonmanager/tst_qxmppcarbonmanager.cpp
index 6166c6f1..684c7470 100644
--- a/tests/qxmppcarbonmanager/tst_qxmppcarbonmanager.cpp
+++ b/tests/qxmppcarbonmanager/tst_qxmppcarbonmanager.cpp
@@ -23,6 +23,7 @@
  */
 
 #include "QXmppCarbonManager.h"
+#include "QXmppClient.h"
 #include "QXmppMessage.h"
 
 #include "util.h"
@@ -56,16 +57,24 @@ private slots:
 
 private:
     QXmppCarbonTestHelper m_helper;
-    QXmppCarbonManager m_manager;
+    QXmppCarbonManager *m_manager;
+    QXmppClient client;
 };
 
 void tst_QXmppCarbonManager::initTestCase()
 {
-    connect(&m_manager, &QXmppCarbonManager::messageSent,
+    m_manager = new QXmppCarbonManager();
+
+    connect(m_manager, &QXmppCarbonManager::messageSent,
             &m_helper, &QXmppCarbonTestHelper::messageSent);
 
-    connect(&m_manager, &QXmppCarbonManager::messageReceived,
+    connect(m_manager, &QXmppCarbonManager::messageReceived,
             &m_helper, &QXmppCarbonTestHelper::messageReceived);
+
+    client.connectToServer("romeo@montague.example", "a");
+    client.disconnectFromServer();
+
+    client.addExtension(m_manager);
 }
 
 void tst_QXmppCarbonManager::testHandleStanza_data()
@@ -127,6 +136,58 @@ void tst_QXmppCarbonManager::testHandleStanza_data()
                       "<thread>0e3141cd80894871a68e6fe6b1ec56fa</thread>"
                       "</message>");
 
+    QTest::newRow("received-wrong-from")
+        << QByteArray("<message xmlns='jabber:client'"
+                      "from='not-romeo@montague.example'"
+                      "to='romeo@montague.example/home'"
+                      "type='chat'>"
+                      "<received xmlns='urn:xmpp:carbons:2'>"
+                      "<forwarded xmlns='urn:xmpp:forward:0'>"
+                      "<message xmlns='jabber:client'"
+                      "from='juliet@capulet.example/balcony'"
+                      "to='romeo@montague.example/garden'"
+                      "type='chat'>"
+                      "<body>What man art thou that, thus bescreen'd in night, so stumblest on my counsel?</body>"
+                      "<thread>0e3141cd80894871a68e6fe6b1ec56fa</thread>"
+                      "</message>"
+                      "</forwarded>"
+                      "</received>"
+                      "</message>")
+        << false << false
+        << QByteArray("<message xmlns='jabber:client'"
+                      "from='juliet@capulet.example/balcony'"
+                      "to='romeo@montague.example/garden'"
+                      "type='chat'>"
+                      "<body>What man art thou that, thus bescreen'd in night, so stumblest on my counsel?</body>"
+                      "<thread>0e3141cd80894871a68e6fe6b1ec56fa</thread>"
+                      "</message>");
+
+    QTest::newRow("sent-wrong-from")
+        << QByteArray("<message xmlns='jabber:client'"
+                      "from='not-romeo@montague.example'"
+                      "to='romeo@montague.example/garden'"
+                      "type='chat'>"
+                      "<sent xmlns='urn:xmpp:carbons:2'>"
+                      "<forwarded xmlns='urn:xmpp:forward:0'>"
+                      "<message xmlns='jabber:client'"
+                      "to='juliet@capulet.example/balcony'"
+                      "from='romeo@montague.example/home'"
+                      "type='chat'>"
+                      "<body>Neither, fair saint, if either thee dislike.</body>"
+                      "<thread>0e3141cd80894871a68e6fe6b1ec56fa</thread>"
+                      "</message>"
+                      "</forwarded>"
+                      "</sent>"
+                      "</message>")
+        << false << true
+        << QByteArray("<message xmlns='jabber:client'"
+                      "to='juliet@capulet.example/balcony'"
+                      "from='romeo@montague.example/home'"
+                      "type='chat'>"
+                      "<body>Neither, fair saint, if either thee dislike.</body>"
+                      "<thread>0e3141cd80894871a68e6fe6b1ec56fa</thread>"
+                      "</message>");
+
     QTest::newRow("forwarded_normal")
         << QByteArray("<message to='mercutio@verona.lit' from='romeo@montague.lit/orchard' type='chat' id='28gs'>"
                       "<body>A most courteous exposition!</body>"
@@ -167,7 +228,7 @@ void tst_QXmppCarbonManager::testHandleStanza()
     QCOMPARE(doc.setContent(xml, true), true);
     QDomElement element = doc.documentElement();
 
-    bool accepted = m_manager.handleStanza(element);
+    bool accepted = m_manager->handleStanza(element);
 
     QCOMPARE(accepted, accept);
     QCOMPARE(m_helper.m_signalTriggered, accept);