fix: Enabling carbons crashes

This commit is contained in:
PapaTutuWawa 2023-01-13 15:17:21 +01:00
parent ad1242c47d
commit ed49212f5a
4 changed files with 83 additions and 33 deletions

View File

@ -1,10 +1,11 @@
import 'package:meta/meta.dart'; import 'package:meta/meta.dart';
/// Represents a Jabber ID in parsed form.
@immutable @immutable
class JID { class JID {
const JID(this.local, this.domain, this.resource); const JID(this.local, this.domain, this.resource);
/// Parses the string [jid] into a JID instance.
factory JID.fromString(String jid) { factory JID.fromString(String jid) {
// Algorithm taken from here: https://blog.samwhited.com/2021/02/xmpp-addresses/ // Algorithm taken from here: https://blog.samwhited.com/2021/02/xmpp-addresses/
var localPart = ''; var localPart = '';
@ -43,12 +44,29 @@ class JID {
final String domain; final String domain;
final String resource; final String resource;
/// Returns true if the JID is bare.
bool isBare() => resource.isEmpty; bool isBare() => resource.isEmpty;
/// Returns true if the JID is full.
bool isFull() => resource.isNotEmpty; bool isFull() => resource.isNotEmpty;
/// Converts the JID into a bare JID.
JID toBare() => JID(local, domain, ''); JID toBare() => JID(local, domain, '');
/// Converts the JID into one with a resource part of [resource].
JID withResource(String resource) => JID(local, domain, 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 @override
String toString() { String toString() {
var result = ''; var result = '';

View File

@ -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_0030/xep_0030.dart';
import 'package:moxxmpp/src/xeps/xep_0297.dart'; import 'package:moxxmpp/src/xeps/xep_0297.dart';
/// This manager class implements support for XEP-0280.
class CarbonsManager extends XmppManagerBase { class CarbonsManager extends XmppManagerBase {
CarbonsManager() : super(); CarbonsManager() : super();
/// Indicates that message carbons are enabled.
bool _isEnabled = false; bool _isEnabled = false;
/// Indicates that the server supports message carbons.
bool _supported = false; bool _supported = false;
/// Indicates that we know that [CarbonsManager._supported] is accurate.
bool _gotSupported = false; bool _gotSupported = false;
@override @override
@ -101,10 +108,15 @@ class CarbonsManager extends XmppManagerBase {
stanza: carbon, stanza: carbon,
); );
} }
/// Send a request to the server, asking it to enable Message Carbons.
///
/// Returns true if carbons were enabled. False, if not.
Future<bool> enableCarbons() async { Future<bool> enableCarbons() async {
final result = await getAttributes().sendStanza( final attrs = getAttributes();
final result = await attrs.sendStanza(
Stanza.iq( Stanza.iq(
to: attrs.getFullJID().toBare().toString(),
type: 'set', type: 'set',
children: [ children: [
XMLNode.xmlns( XMLNode.xmlns(
@ -129,6 +141,9 @@ class CarbonsManager extends XmppManagerBase {
return true; return true;
} }
/// Send a request to the server, asking it to disable Message Carbons.
///
/// Returns true if carbons were disabled. False, if not.
Future<bool> disableCarbons() async { Future<bool> disableCarbons() async {
final result = await getAttributes().sendStanza( final result = await getAttributes().sendStanza(
Stanza.iq( Stanza.iq(
@ -163,8 +178,15 @@ class CarbonsManager extends XmppManagerBase {
void forceEnable() { void forceEnable() {
_isEnabled = true; _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) { bool isCarbonValid(JID senderJid) {
return _isEnabled && senderJid == getAttributes().getConnectionSettings().jid.toBare(); return _isEnabled && getAttributes().getFullJID().bareCompare(
senderJid,
ensureBare: true,
);
} }
} }

View File

@ -1,6 +1,5 @@
import 'package:moxxmpp/moxxmpp.dart'; import 'package:moxxmpp/moxxmpp.dart';
import 'package:test/test.dart'; import 'package:test/test.dart';
void main() { void main() {
test('Parse a full JID', () { test('Parse a full JID', () {
final jid = JID.fromString('test@server/abc'); final jid = JID.fromString('test@server/abc');
@ -42,4 +41,15 @@ void main() {
test('Parse resource with a slash', () { test('Parse resource with a slash', () {
expect(JID.fromString('hallo@welt.example./test/welt') == JID('hallo', 'welt.example', 'test/welt'), true); 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);
});
} }

View File

@ -4,33 +4,33 @@ import '../helpers/xmpp.dart';
void main() { void main() {
test("Test if we're vulnerable against CVE-2020-26547 style vulnerabilities", () async { test("Test if we're vulnerable against CVE-2020-26547 style vulnerabilities", () async {
final attributes = XmppManagerAttributes( final attributes = XmppManagerAttributes(
sendStanza: (stanza, { StanzaFromType addFrom = StanzaFromType.full, bool addId = true, bool retransmitted = false, bool awaitable = true, bool encrypted = false }) async { sendStanza: (stanza, { StanzaFromType addFrom = StanzaFromType.full, bool addId = true, bool retransmitted = false, bool awaitable = true, bool encrypted = false }) async {
// ignore: avoid_print // ignore: avoid_print
print('==> ${stanza.toXml()}'); print('==> ${stanza.toXml()}');
return XMLNode(tag: 'iq', attributes: { 'type': 'result' }); return XMLNode(tag: 'iq', attributes: { 'type': 'result' });
}, },
sendNonza: (nonza) {}, sendNonza: (nonza) {},
sendEvent: (event) {}, sendEvent: (event) {},
getManagerById: getManagerNullStub, getManagerById: getManagerNullStub,
getConnectionSettings: () => ConnectionSettings( getConnectionSettings: () => ConnectionSettings(
jid: JID.fromString('bob@xmpp.example'), jid: JID.fromString('bob@xmpp.example'),
password: 'password', password: 'password',
useDirectTLS: true, useDirectTLS: true,
allowPlainAuth: false, allowPlainAuth: false,
), ),
isFeatureSupported: (_) => false, isFeatureSupported: (_) => false,
getFullJID: () => JID.fromString('bob@xmpp.example/uwu'), getFullJID: () => JID.fromString('bob@xmpp.example/uwu'),
getSocket: () => StubTCPSocket(play: []), getSocket: () => StubTCPSocket(play: []),
getConnection: () => XmppConnection(TestingReconnectionPolicy(), StubTCPSocket(play: [])), getConnection: () => XmppConnection(TestingReconnectionPolicy(), StubTCPSocket(play: [])),
getNegotiatorById: getNegotiatorNullStub, getNegotiatorById: getNegotiatorNullStub,
); );
final manager = CarbonsManager(); final manager = CarbonsManager();
manager.register(attributes); manager.register(attributes);
await manager.enableCarbons(); await manager.enableCarbons();
expect(manager.isCarbonValid(JID.fromString('mallory@evil.example')), 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')), true);
expect(manager.isCarbonValid(JID.fromString('bob@xmpp.example/abc')), false); expect(manager.isCarbonValid(JID.fromString('bob@xmpp.example/abc')), false);
}); });
} }