Fixes #1704

Ignore carbon groupchat messages

Fixes #1704
Ignore carbon groupchat messages
0af9bc8f · JC Brand · a69ebcbd · 0af9bc8f · 0af9bc8f · 0af9bc8f
Commit 0af9bc8f authored Sep 11, 2019 by JC Brand
Hide whitespace changes
Inline Side-by-side

Showing with 79 additions and 1 deletion

CHANGES.md CHANGES.md +1 -0

spec/muc_messages.js spec/muc_messages.js +60 -0

src/headless/converse-muc.js src/headless/converse-muc.js +18 -1

No files found.
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -13,6 +13,7 @@
  your preferred path and then rebuild all assets (e.g. `make dist`).
 - Use `listenTo` to avoid memory leaks when views get removed.
 - #1692 Bugfix: `TypeError: oldest_message is undefined`
+- #1704 SECURITY FIX: Impersonation by misusage of groupchat carbons
 - #1705 Bugfix: `this.roomspanel` is `undefined` after hibernating

--- a/spec/muc_messages.js
+++ b/spec/muc_messages.js
@@ -112,6 +112,66 @@
            done();
        }));
+        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) {
+            await test_utils.waitForRoster(_converse, 'current');
+            test_utils.openControlBox();
+            const muc_jid = 'xsf@muc.xmpp.org';
+            const sender_jid = `${muc_jid}/romeo`;
+            const impersonated_jid = `${muc_jid}/i_am_groot`
+            await test_utils.openAndEnterChatRoom(_converse, muc_jid, 'romeo');
+            const stanza = $pres({
+                    to: 'romeo@montague.lit/_converse.js-29092160',
+                    from: sender_jid
+                })
+                .c('x', {xmlns: Strophe.NS.MUC_USER})
+                .c('item', {
+                    'affiliation': 'none',
+                    'jid': 'newguy@montague.lit/_converse.js-290929789',
+                    'role': 'participant'
+                }).tree();
+            _converse.connection._dataRecv(test_utils.createRequest(stanza));
+            /*
+             * <message to="romeo@montague.im/poezio" id="718d40df-3948-4798-a99b-35cc9f03cc4f-641" type="groupchat" from="xsf@muc.xmpp.org/romeo">
+             *     <received xmlns="urn:xmpp:carbons:2">
+             *         <forwarded xmlns="urn:xmpp:forward:0">
+             *         <message xmlns="jabber:client" to="xsf@muc.xmpp.org" type="groupchat" from="xsf@muc.xmpp.org/i_am_groot">
+             *             <body>I am groot.</body>
+             *         </message>
+             *         </forwarded>
+             *     </received>
+             * </message>
+             */
+            const msg = $msg({
+                    'from': sender_jid,
+                    'id': _converse.connection.getUniqueId(),
+                    'to': _converse.connection.jid,
+                    'type': 'groupchat',
+                    'xmlns': 'jabber:client'
+                }).c('received', {'xmlns': 'urn:xmpp:carbons:2'})
+                  .c('forwarded', {'xmlns': 'urn:xmpp:forward:0'})
+                  .c('message', {
+                        'xmlns': 'jabber:client',
+                        'from': impersonated_jid,
+                        'to': muc_jid,
+                        'type': 'groupchat'
+                }).c('body').t('I am groot').tree();
+            const view = _converse.api.chatviews.get(muc_jid);
+            spyOn(_converse, 'log');
+            await view.model.onMessage(msg);
+            expect(_converse.log).toHaveBeenCalledWith(
+                'onMessage: Ignoring XEP-0280 "groupchat" message carbon, '+
+                'according to the XEP groupchat messages SHOULD NOT be carbon copied',
+                Strophe.LogLevel.ERROR
+            );
+            expect(view.el.querySelectorAll('.chat-msg').length).toBe(0);
+            expect(view.model.messages.length).toBe(0);
+            done();
+        }));
        it("keeps track of the sender's role and affiliation",
            mock.initConverse(
                null, ['rosterGroupsFetched'], {},

--- a/src/headless/converse-muc.js
+++ b/src/headless/converse-muc.js
@@ -520,6 +520,13 @@ converse.plugins.add('converse-muc', {
                    {'ignoreNamespaceFragment': true, 'matchBareFromJid': true}
                );
                this.message_handler = _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', 'groupchat', null, room_jid,
@@ -1528,10 +1535,20 @@ converse.plugins.add('converse-muc', {
             * @param { XMLElement } stanza - The message stanza.
             */
            async onMessage (stanza) {
+                const original_stanza = stanza;
+                const is_carbon = sizzle(`received[xmlns="${Strophe.NS.CARBONS}"]`, original_stanza).length > 0;
+                if (is_carbon) {
+                    // XEP-280: groupchat messages SHOULD NOT be carbon copied, so we're discarding it.
+                    _converse.log(
+                        'onMessage: Ignoring XEP-0280 "groupchat" message carbon, '+
+                        'according to the XEP groupchat messages SHOULD NOT be carbon copied',
+                        Strophe.LogLevel.ERROR);
+                    return;
+                }
                this.createInfoMessages(stanza);
                this.fetchFeaturesIfConfigurationChanged(stanza);
-                const original_stanza = stanza;
                const forwarded = sizzle(`forwarded[xmlns="${Strophe.NS.FORWARD}"]`, stanza).pop();
                if (forwarded) {
                    stanza = forwarded.querySelector('message');
                }