Ignore MAM `chat` messages not sent from yourself

CHANGES.md

@@ -12,11 +12,11 @@
   different path, you'll need to set `publicPath` in `webpack.config.js` to
   your preferred path and then rebuild all assets (e.g. `make dist`).
 - Use `listenTo` to avoid memory leaks when views get removed.
+- SECURITY FIX: Ignore MAM `chat` messages not sent from yourself
 - #1692 Bugfix: `TypeError: oldest_message is undefined`
 - #1704 SECURITY FIX: Impersonation by misusage of groupchat carbons
 - #1705 Bugfix: `this.roomspanel` is `undefined` after hibernating
 
-
 ## 5.0.1 (2019-08-14)
 
 - Add a new GUI for moderator actions. You can trigger it by entering `/modtools` in a MUC.

spec/mam.js

@@ -210,6 +210,62 @@
 
             describe("when recieved", function () {
 
+                it("is discarded if it doesn't come from the right sender",
+                    mock.initConverse(
+                        null, ['discoInitialized'], {},
+                        async function (done, _converse) {
+
+                        await test_utils.waitForRoster(_converse, 'current', 1);
+                        const contact_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@montague.lit';
+                        await test_utils.openChatBoxFor(_converse, contact_jid);
+                        await test_utils.waitUntilDiscoConfirmed(_converse, _converse.bare_jid, null, [Strophe.NS.MAM]);
+                        const sent_IQs = _converse.connection.IQ_stanzas;
+                        const stanza = await u.waitUntil(() => sent_IQs.filter(iq => iq.querySelector(`iq[type="set"] query[xmlns="${Strophe.NS.MAM}"]`)).pop());
+                        const queryid = stanza.querySelector('query').getAttribute('queryid');
+                        let msg = $msg({'id': _converse.connection.getUniqueId(), 'from': 'impersonator@capulet.lit', 'to': _converse.bare_jid})
+                                    .c('result',  {'xmlns': 'urn:xmpp:mam:2', 'queryid':queryid, 'id': _converse.connection.getUniqueId()})
+                                        .c('forwarded', {'xmlns':'urn:xmpp:forward:0'})
+                                            .c('delay', {'xmlns':'urn:xmpp:delay', 'stamp':'2010-07-10T23:08:25Z'}).up()
+                                            .c('message', {
+                                                'xmlns':'jabber:client',
+                                                'to': _converse.bare_jid,
+                                                'id': _converse.connection.getUniqueId(),
+                                                'from': contact_jid,
+                                                'type':'chat'
+                                            }).c('body').t("Meet me at the dance");
+                        spyOn(_converse, 'log');
+                        _converse.connection._dataRecv(test_utils.createRequest(msg));
+                        expect(_converse.log).toHaveBeenCalledWith(`Ignoring alleged MAM message from ${msg.nodeTree.getAttribute('from')}`, Strophe.LogLevel.WARN);
+
+                        msg = $msg({'id': _converse.connection.getUniqueId(), 'to': _converse.bare_jid})
+                                    .c('result',  {'xmlns': 'urn:xmpp:mam:2', 'queryid':queryid, 'id': _converse.connection.getUniqueId()})
+                                        .c('forwarded', {'xmlns':'urn:xmpp:forward:0'})
+                                            .c('delay', {'xmlns':'urn:xmpp:delay', 'stamp':'2010-07-10T23:08:25Z'}).up()
+                                            .c('message', {
+                                                'xmlns':'jabber:client',
+                                                'to': _converse.bare_jid,
+                                                'id': _converse.connection.getUniqueId(),
+                                                'from': contact_jid,
+                                                'type':'chat'
+                                            }).c('body').t("Thrice the brinded cat hath mew'd.");
+                        _converse.connection._dataRecv(test_utils.createRequest(msg));
+
+                        const iq_result = $iq({'type': 'result', 'id': stanza.getAttribute('id')})
+                            .c('fin', {'xmlns': 'urn:xmpp:mam:2'})
+                                .c('set',  {'xmlns': 'http://jabber.org/protocol/rsm'})
+                                    .c('first', {'index': '0'}).t('23452-4534-1').up()
+                                    .c('last').t('09af3-cc343-b409f').up()
+                                    .c('count').t('16');
+                        _converse.connection._dataRecv(test_utils.createRequest(iq_result));
+
+                        const view = _converse.chatboxviews.get(contact_jid);
+                        await new Promise((resolve, reject) => view.once('messageInserted', resolve));
+                        expect(view.model.messages.length).toBe(1);
+                        expect(view.model.messages.at(0).get('message')).toBe("Thrice the brinded cat hath mew'd.");
+                        done();
+                }));
+
+
                 it("updates the is_archived value of an already cached version",
                     mock.initConverse(
                         null, ['discoInitialized'], {},

spec/messages.js

@@ -310,7 +310,6 @@
             let message, msg;
             const sender_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@montague.lit';
             await u.waitUntil(() => _converse.rosterview.el.querySelectorAll('.roster-group').length)
-            spyOn(_converse, 'log');
             spyOn(_converse.chatboxes, 'getChatBox').and.callThrough();
             _converse.filter_by_resource = true;
 
@@ -531,7 +530,6 @@
             test_utils.openControlBox();
 
             // Send a message from a different resource
-            spyOn(_converse, 'log');
             const msgtext = 'This is a carbon message';
             const sender_jid = mock.cur_names[1].replace(/ /g,'.').toLowerCase() + '@montague.lit';
             const msg = $msg({
@@ -584,7 +582,6 @@
             test_utils.openControlBox();
 
             // Send a message from a different resource
-            spyOn(_converse, 'log');
             const msgtext = 'This is a sent carbon message';
             const recipient_jid = mock.cur_names[5].replace(/ /g,'.').toLowerCase() + '@montague.lit';
             const msg = $msg({
@@ -622,9 +619,9 @@
         }));
 
         it("will be discarded if it's a malicious message meant to look like a carbon copy",
-        mock.initConverse(
-            null, ['rosterGroupsFetched'], {},
-            async function (done, _converse) {
+            mock.initConverse(
+                null, ['rosterGroupsFetched'], {},
+                async function (done, _converse) {
 
             await test_utils.waitForRoster(_converse, 'current');
             test_utils.openControlBox();
@@ -638,7 +635,6 @@
              *    </received>
              * </message>
              */
-            spyOn(_converse, 'log');
             const msgtext = 'Please come to Creepy Valley tonight, alone!';
             const sender_jid = mock.cur_names[1].replace(/ /g,'.').toLowerCase() + '@montague.lit';
             const impersonated_jid = mock.cur_names[2].replace(/ /g,'.').toLowerCase() + '@montague.lit';

src/headless/converse-chatboxes.js

@@ -1062,6 +1062,13 @@ converse.plugins.add('converse-chatboxes', {
 
             registerMessageHandler () {
                 _converse.connection.addHandler(stanza => {
+                    if (sizzle(`message > result[xmlns="${Strophe.NS.MAM}"]`, stanza).pop()) {
+                        // MAM messages are handled in converse-mam.
+                        // We shouldn't get MAM messages here because
+                        // they shouldn't have a `type` attribute.
+                        _converse.log(`Received a MAM message with type "chat".`, Strophe.LogLevel.WARN);
+                        return true;
+                    }
                     this.onMessage(stanza);
                     return true;
                 }, null, 'message', 'chat');

src/headless/converse-mam.js

@@ -500,14 +500,22 @@ converse.plugins.add('converse-mam', {
                     }
 
                     const messages = [];
-                    const message_handler = _converse.connection.addHandler(message => {
-                        if (options.groupchat && message.getAttribute('from') !== options['with']) {
+                    const message_handler = _converse.connection.addHandler(stanza => {
+                        const result = sizzle(`message > result[xmlns="${Strophe.NS.MAM}"]`, stanza).pop();
+                        if (result === undefined || result.getAttribute('queryid') !== queryid) {
                             return true;
                         }
-                        const result = message.querySelector('result');
-                        if (result !== null && result.getAttribute('queryid') === queryid) {
-                            messages.push(message);
+                        const from = stanza.getAttribute('from') || _converse.bare_jid;
+                        if (options.groupchat) {
+                            if (from !== options['with']) {
+                                _converse.log(`Ignoring alleged groupchat MAM message from ${stanza.getAttribute('from')}`, Strophe.LogLevel.WARN);
+                                return true;
+                            }
+                        } else if (from !== _converse.bare_jid) {
+                            _converse.log(`Ignoring alleged MAM message from ${stanza.getAttribute('from')}`, Strophe.LogLevel.WARN);
+                            return true;
                         }
+                        messages.push(stanza);
                         return true;
                     }, Strophe.NS.MAM);