diff options
| author | Linus Jahn <lnj@kaidan.im> | 2020-07-19 13:50:13 +0200 |
|---|---|---|
| committer | Linus Jahn <lnj@kaidan.im> | 2020-07-20 17:18:24 +0200 |
| commit | e73120d4ab5b1a93c6ad051ff6807af02cd8a039 (patch) | |
| tree | 2c4c49bf0ae6b2fc6b08bd18c1cee1202e1fb334 | |
| parent | 86a1888e39765dd57762c4422e6ede0c4a408935 (diff) | |
| download | qxmpp-e73120d4ab5b1a93c6ad051ff6807af02cd8a039.tar.gz | |
QXmppCarbonManager: Fix vulnerability: Add sender check
The XEP requires that only carbon messages from the client's bare JID
are accepted. This prevents that other entities can inject messages into
the client.
| -rw-r--r-- | src/client/QXmppCarbonManager.cpp | 6 | ||||
| -rw-r--r-- | tests/qxmppcarbonmanager/tst_qxmppcarbonmanager.cpp | 69 |
2 files changed, 71 insertions, 4 deletions
diff --git a/src/client/QXmppCarbonManager.cpp b/src/client/QXmppCarbonManager.cpp index d403e6de..e936ddf2 100644 --- a/src/client/QXmppCarbonManager.cpp +++ b/src/client/QXmppCarbonManager.cpp @@ -93,6 +93,12 @@ bool QXmppCarbonManager::handleStanza(const QDomElement &element) if (carbon.isNull() || carbon.namespaceURI() != ns_carbons) return false; // Neither sent nor received -> no carbon message + // 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 forwarded = carbon.firstChildElement("forwarded"); if (forwarded.isNull()) return false; 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); |