From ed49212f5a527b2b0084faf97cb5c383221772e2 Mon Sep 17 00:00:00 2001 From: "Alexander \"PapaTutuWawa" Date: Fri, 13 Jan 2023 15:17:21 +0100 Subject: [PATCH] fix: Enabling carbons crashes --- packages/moxxmpp/lib/src/jid.dart | 20 ++++++- packages/moxxmpp/lib/src/xeps/xep_0280.dart | 30 +++++++++-- packages/moxxmpp/test/jid_test.dart | 12 ++++- packages/moxxmpp/test/xeps/xep_0280_test.dart | 54 +++++++++---------- 4 files changed, 83 insertions(+), 33 deletions(-) diff --git a/packages/moxxmpp/lib/src/jid.dart b/packages/moxxmpp/lib/src/jid.dart index 6930629..e7584b7 100644 --- a/packages/moxxmpp/lib/src/jid.dart +++ b/packages/moxxmpp/lib/src/jid.dart @@ -1,10 +1,11 @@ import 'package:meta/meta.dart'; +/// Represents a Jabber ID in parsed form. @immutable class JID { - const JID(this.local, this.domain, this.resource); + /// Parses the string [jid] into a JID instance. factory JID.fromString(String jid) { // Algorithm taken from here: https://blog.samwhited.com/2021/02/xmpp-addresses/ var localPart = ''; @@ -43,12 +44,29 @@ class JID { final String domain; final String resource; + /// Returns true if the JID is bare. bool isBare() => resource.isEmpty; + + /// Returns true if the JID is full. bool isFull() => resource.isNotEmpty; + /// Converts the JID into a bare JID. JID toBare() => JID(local, domain, ''); + + /// Converts the JID into one with a resource part of [resource]. JID withResource(String resource) => JID(local, domain, resource); + + /// Compares the JID with [other]. This function assumes that JID and [other] + /// are bare, i.e. only the domain- and localparts are compared. If [ensureBare] + /// is optionally set to true, then [other] MUST be bare. Otherwise, false is returned. + bool bareCompare(JID other, { bool ensureBare = false }) { + if (ensureBare && !other.isBare()) return false; + + return local == other.local && domain == other.domain; + } + /// Converts to JID instance into its string representation of + /// localpart@domainpart/resource. @override String toString() { var result = ''; diff --git a/packages/moxxmpp/lib/src/xeps/xep_0280.dart b/packages/moxxmpp/lib/src/xeps/xep_0280.dart index 317116b..b55e423 100644 --- a/packages/moxxmpp/lib/src/xeps/xep_0280.dart +++ b/packages/moxxmpp/lib/src/xeps/xep_0280.dart @@ -12,10 +12,17 @@ import 'package:moxxmpp/src/stringxml.dart'; import 'package:moxxmpp/src/xeps/xep_0030/xep_0030.dart'; import 'package:moxxmpp/src/xeps/xep_0297.dart'; +/// This manager class implements support for XEP-0280. class CarbonsManager extends XmppManagerBase { CarbonsManager() : super(); + + /// Indicates that message carbons are enabled. bool _isEnabled = false; + + /// Indicates that the server supports message carbons. bool _supported = false; + + /// Indicates that we know that [CarbonsManager._supported] is accurate. bool _gotSupported = false; @override @@ -101,10 +108,15 @@ class CarbonsManager extends XmppManagerBase { stanza: carbon, ); } - + + /// Send a request to the server, asking it to enable Message Carbons. + /// + /// Returns true if carbons were enabled. False, if not. Future enableCarbons() async { - final result = await getAttributes().sendStanza( + final attrs = getAttributes(); + final result = await attrs.sendStanza( Stanza.iq( + to: attrs.getFullJID().toBare().toString(), type: 'set', children: [ XMLNode.xmlns( @@ -129,6 +141,9 @@ class CarbonsManager extends XmppManagerBase { return true; } + /// Send a request to the server, asking it to disable Message Carbons. + /// + /// Returns true if carbons were disabled. False, if not. Future disableCarbons() async { final result = await getAttributes().sendStanza( Stanza.iq( @@ -163,8 +178,15 @@ class CarbonsManager extends XmppManagerBase { void forceEnable() { _isEnabled = true; } - + + /// Checks if a carbon sent by [senderJid] is valid to prevent vulnerabilities like + /// the ones listed at https://xmpp.org/extensions/xep-0280.html#security. + /// + /// Returns true if the carbon is valid. Returns false if not. bool isCarbonValid(JID senderJid) { - return _isEnabled && senderJid == getAttributes().getConnectionSettings().jid.toBare(); + return _isEnabled && getAttributes().getFullJID().bareCompare( + senderJid, + ensureBare: true, + ); } } diff --git a/packages/moxxmpp/test/jid_test.dart b/packages/moxxmpp/test/jid_test.dart index 2fa30b5..03358b6 100644 --- a/packages/moxxmpp/test/jid_test.dart +++ b/packages/moxxmpp/test/jid_test.dart @@ -1,6 +1,5 @@ import 'package:moxxmpp/moxxmpp.dart'; import 'package:test/test.dart'; - void main() { test('Parse a full JID', () { final jid = JID.fromString('test@server/abc'); @@ -42,4 +41,15 @@ void main() { test('Parse resource with a slash', () { expect(JID.fromString('hallo@welt.example./test/welt') == JID('hallo', 'welt.example', 'test/welt'), true); }); + + test('bareCompare', () { + final jid1 = JID('hallo', 'welt', 'lol'); + final jid2 = JID('hallo', 'welt', ''); + final jid3 = JID('hallo', 'earth', 'true'); + + expect(jid1.bareCompare(jid2), true); + expect(jid2.bareCompare(jid1), true); + expect(jid2.bareCompare(jid1, ensureBare: true), false); + expect(jid2.bareCompare(jid3), false); + }); } diff --git a/packages/moxxmpp/test/xeps/xep_0280_test.dart b/packages/moxxmpp/test/xeps/xep_0280_test.dart index c021db3..06727c6 100644 --- a/packages/moxxmpp/test/xeps/xep_0280_test.dart +++ b/packages/moxxmpp/test/xeps/xep_0280_test.dart @@ -4,33 +4,33 @@ import '../helpers/xmpp.dart'; void main() { test("Test if we're vulnerable against CVE-2020-26547 style vulnerabilities", () async { - final attributes = XmppManagerAttributes( - sendStanza: (stanza, { StanzaFromType addFrom = StanzaFromType.full, bool addId = true, bool retransmitted = false, bool awaitable = true, bool encrypted = false }) async { - // ignore: avoid_print - print('==> ${stanza.toXml()}'); - return XMLNode(tag: 'iq', attributes: { 'type': 'result' }); - }, - sendNonza: (nonza) {}, - sendEvent: (event) {}, - getManagerById: getManagerNullStub, - getConnectionSettings: () => ConnectionSettings( - jid: JID.fromString('bob@xmpp.example'), - password: 'password', - useDirectTLS: true, - allowPlainAuth: false, - ), - isFeatureSupported: (_) => false, - getFullJID: () => JID.fromString('bob@xmpp.example/uwu'), - getSocket: () => StubTCPSocket(play: []), - getConnection: () => XmppConnection(TestingReconnectionPolicy(), StubTCPSocket(play: [])), - getNegotiatorById: getNegotiatorNullStub, - ); - final manager = CarbonsManager(); - manager.register(attributes); - await manager.enableCarbons(); + final attributes = XmppManagerAttributes( + sendStanza: (stanza, { StanzaFromType addFrom = StanzaFromType.full, bool addId = true, bool retransmitted = false, bool awaitable = true, bool encrypted = false }) async { + // ignore: avoid_print + print('==> ${stanza.toXml()}'); + return XMLNode(tag: 'iq', attributes: { 'type': 'result' }); + }, + sendNonza: (nonza) {}, + sendEvent: (event) {}, + getManagerById: getManagerNullStub, + getConnectionSettings: () => ConnectionSettings( + jid: JID.fromString('bob@xmpp.example'), + password: 'password', + useDirectTLS: true, + allowPlainAuth: false, + ), + isFeatureSupported: (_) => false, + getFullJID: () => JID.fromString('bob@xmpp.example/uwu'), + getSocket: () => StubTCPSocket(play: []), + getConnection: () => XmppConnection(TestingReconnectionPolicy(), StubTCPSocket(play: [])), + getNegotiatorById: getNegotiatorNullStub, + ); + final manager = CarbonsManager(); + manager.register(attributes); + await manager.enableCarbons(); - expect(manager.isCarbonValid(JID.fromString('mallory@evil.example')), false); - expect(manager.isCarbonValid(JID.fromString('bob@xmpp.example')), true); - expect(manager.isCarbonValid(JID.fromString('bob@xmpp.example/abc')), false); + expect(manager.isCarbonValid(JID.fromString('mallory@evil.example')), false); + expect(manager.isCarbonValid(JID.fromString('bob@xmpp.example')), true); + expect(manager.isCarbonValid(JID.fromString('bob@xmpp.example/abc')), false); }); }