From 7afa1db54e07f024b2a4550d160b4d4a9f67f2c7 Mon Sep 17 00:00:00 2001 From: Andrew Kolos Date: Tue, 7 Jan 2025 17:15:54 -0500 Subject: [PATCH 1/8] change --- packages/multicast_dns/lib/src/packet.dart | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/packages/multicast_dns/lib/src/packet.dart b/packages/multicast_dns/lib/src/packet.dart index dcf1402a04f..e3c6e603666 100644 --- a/packages/multicast_dns/lib/src/packet.dart +++ b/packages/multicast_dns/lib/src/packet.dart @@ -4,6 +4,7 @@ import 'dart:convert'; import 'dart:io'; +import 'dart:math'; import 'dart:typed_data'; import 'constants.dart'; @@ -134,20 +135,23 @@ _FQDNReadResult _readFQDN( final List parts = []; final int prevOffset = offset; + final List offsetsToVisit = [offset]; + int highestOffsetRead = offset; + + while (offsetsToVisit.isNotEmpty) { + offset = offsetsToVisit.removeLast(); + while (true) { // At least one byte is required. checkLength(offset + 1); - // Check for compressed. if (data[offset] & 0xc0 == 0xc0) { // At least two bytes are required for a compressed FQDN. checkLength(offset + 2); // A compressed FQDN has a new offset in the lower 14 bits. - final _FQDNReadResult result = _readFQDN( - data, byteData, byteData.getUint16(offset) & ~0xc000, length); - parts.addAll(result.fqdnParts); - offset += 2; + offsetsToVisit.add(byteData.getUint16(offset) & ~0xc000); + highestOffsetRead = max(highestOffsetRead, offset + 2); break; } else { // A normal FQDN part has a length and a UTF-8 encoded name @@ -163,12 +167,15 @@ _FQDNReadResult _readFQDN( // we should continue decoding even if it isn't to avoid dropping the // rest of the data, which might still be useful. parts.add(utf8.decode(partBytes, allowMalformed: true)); + highestOffsetRead = max(highestOffsetRead, offset); } else { + highestOffsetRead = max(highestOffsetRead, offset); break; } } + } } - return _FQDNReadResult(parts, offset - prevOffset); + return _FQDNReadResult(parts, highestOffsetRead - prevOffset); } /// Decode an mDNS query packet. From fb65177692db8fe6ce20251d5c1e90bfa00cf4af Mon Sep 17 00:00:00 2001 From: Andrew Kolos Date: Tue, 7 Jan 2025 17:39:35 -0500 Subject: [PATCH 2/8] format --- packages/multicast_dns/lib/src/packet.dart | 64 +++++++++++----------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/packages/multicast_dns/lib/src/packet.dart b/packages/multicast_dns/lib/src/packet.dart index e3c6e603666..8f713bdda73 100644 --- a/packages/multicast_dns/lib/src/packet.dart +++ b/packages/multicast_dns/lib/src/packet.dart @@ -139,41 +139,41 @@ _FQDNReadResult _readFQDN( int highestOffsetRead = offset; while (offsetsToVisit.isNotEmpty) { - offset = offsetsToVisit.removeLast(); - - while (true) { - // At least one byte is required. - checkLength(offset + 1); - // Check for compressed. - if (data[offset] & 0xc0 == 0xc0) { - // At least two bytes are required for a compressed FQDN. - checkLength(offset + 2); - - // A compressed FQDN has a new offset in the lower 14 bits. - offsetsToVisit.add(byteData.getUint16(offset) & ~0xc000); - highestOffsetRead = max(highestOffsetRead, offset + 2); - break; - } else { - // A normal FQDN part has a length and a UTF-8 encoded name - // part. If the length is 0 this is the end of the FQDN. - final int partLength = data[offset]; - offset++; - if (partLength > 0) { - checkLength(offset + partLength); - final Uint8List partBytes = - Uint8List.view(data.buffer, offset, partLength); - offset += partLength; - // According to the RFC, this is supposed to be utf-8 encoded, but - // we should continue decoding even if it isn't to avoid dropping the - // rest of the data, which might still be useful. - parts.add(utf8.decode(partBytes, allowMalformed: true)); - highestOffsetRead = max(highestOffsetRead, offset); - } else { - highestOffsetRead = max(highestOffsetRead, offset); + offset = offsetsToVisit.removeLast(); + + while (true) { + // At least one byte is required. + checkLength(offset + 1); + // Check for compressed. + if (data[offset] & 0xc0 == 0xc0) { + // At least two bytes are required for a compressed FQDN. + checkLength(offset + 2); + + // A compressed FQDN has a new offset in the lower 14 bits. + offsetsToVisit.add(byteData.getUint16(offset) & ~0xc000); + highestOffsetRead = max(highestOffsetRead, offset + 2); break; + } else { + // A normal FQDN part has a length and a UTF-8 encoded name + // part. If the length is 0 this is the end of the FQDN. + final int partLength = data[offset]; + offset++; + if (partLength > 0) { + checkLength(offset + partLength); + final Uint8List partBytes = + Uint8List.view(data.buffer, offset, partLength); + offset += partLength; + // According to the RFC, this is supposed to be utf-8 encoded, but + // we should continue decoding even if it isn't to avoid dropping the + // rest of the data, which might still be useful. + parts.add(utf8.decode(partBytes, allowMalformed: true)); + highestOffsetRead = max(highestOffsetRead, offset); + } else { + highestOffsetRead = max(highestOffsetRead, offset); + break; + } } } - } } return _FQDNReadResult(parts, highestOffsetRead - prevOffset); } From d8716a7cc95e2e15079a3b494e77eab82bc40209 Mon Sep 17 00:00:00 2001 From: Andrew Kolos Date: Tue, 7 Jan 2025 17:41:27 -0500 Subject: [PATCH 3/8] bump package version --- packages/multicast_dns/pubspec.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/multicast_dns/pubspec.yaml b/packages/multicast_dns/pubspec.yaml index 6f0f3052cc0..3303a8fb944 100644 --- a/packages/multicast_dns/pubspec.yaml +++ b/packages/multicast_dns/pubspec.yaml @@ -2,7 +2,7 @@ name: multicast_dns description: Dart package for performing mDNS queries (e.g. Bonjour, Avahi). repository: https://github.com/flutter/packages/tree/main/packages/multicast_dns issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+multicast_dns%22 -version: 0.3.2+7 +version: 0.3.2+8 environment: sdk: ^3.4.0 From 1fe57f02a95dc9d4e29802942a3f295ecc333691 Mon Sep 17 00:00:00 2001 From: Andrew Kolos Date: Tue, 7 Jan 2025 17:46:01 -0500 Subject: [PATCH 4/8] Update CHANGELOG.md --- packages/multicast_dns/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/multicast_dns/CHANGELOG.md b/packages/multicast_dns/CHANGELOG.md index 4b23be31ce2..7a7d48a6936 100644 --- a/packages/multicast_dns/CHANGELOG.md +++ b/packages/multicast_dns/CHANGELOG.md @@ -1,5 +1,6 @@ -## NEXT +## 0.3.2+8 +* Fixes stack overflows ocurring during the parsing of domain names in MDNS messages. * Updates minimum supported SDK version to Flutter 3.22/Dart 3.4. ## 0.3.2+7 From 8a597b30077626e7f636ff84dd12b49542509f1d Mon Sep 17 00:00:00 2001 From: Andrew Kolos Date: Mon, 20 Jan 2025 23:42:41 -0800 Subject: [PATCH 5/8] detect cycles in name compression --- packages/multicast_dns/lib/src/packet.dart | 9 +++++ packages/multicast_dns/test/decode_test.dart | 40 ++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/packages/multicast_dns/lib/src/packet.dart b/packages/multicast_dns/lib/src/packet.dart index 8f713bdda73..adf63dc53d9 100644 --- a/packages/multicast_dns/lib/src/packet.dart +++ b/packages/multicast_dns/lib/src/packet.dart @@ -121,6 +121,10 @@ String readFQDN(List packet, [int offset = 0]) { return _readFQDN(data, byteData, offset, data.length).fqdn; } +// Read a FQDN at the given offset. Returns a pair with the FQDN +// parts and the number of bytes consumed. +// +// If decoding fails (e.g. due to an invalid packet) `null` is returned. // Read a FQDN at the given offset. Returns a pair with the FQDN // parts and the number of bytes consumed. // @@ -136,10 +140,15 @@ _FQDNReadResult _readFQDN( final List parts = []; final int prevOffset = offset; final List offsetsToVisit = [offset]; + final Set visitedOffsets = {}; int highestOffsetRead = offset; while (offsetsToVisit.isNotEmpty) { offset = offsetsToVisit.removeLast(); + if (visitedOffsets.contains(offset)) { + throw MDnsDecodeException(offset); + } + visitedOffsets.add(offset); while (true) { // At least one byte is required. diff --git a/packages/multicast_dns/test/decode_test.dart b/packages/multicast_dns/test/decode_test.dart index 32d7c2fd0c8..358f764eba9 100644 --- a/packages/multicast_dns/test/decode_test.dart +++ b/packages/multicast_dns/test/decode_test.dart @@ -192,6 +192,10 @@ void testBadPackages() { } } }); + + test('Detects cyclic pointers and returns null', () { + expect(decodeMDnsResponse(cycle), isNull); + }); } void testPTRRData() { @@ -584,6 +588,42 @@ const List package3 = [ 0x2c ]; +/// Contains compressed domain names where a there is a cycle amongst the +/// offset pointers. +const List cycle = [ + 0x00, + 0x00, + 0x84, + 0x00, + 0x00, + 0x00, + 0x00, + 0x01, + 0x00, + 0x00, + 0x00, + 0x00, + 0x07, 0x65, 0x78, 0x61, 0x6d, 0x70, 0x6c, 0x65, // "example" + 0xC0, 0x16, // Pointer to "com" + 0x03, 0x63, 0x6f, 0x6d, // "com" + 0xC0, 0x0c, // Pointer to "example" + 0x00, + 0x00, + 0x01, + 0x80, + 0x01, + 0x00, + 0x00, + 0x00, + 0x78, + 0x00, + 0x04, + 0xc0, + 0xa8, + 0x01, + 0xbf +]; + const List packagePtrResponse = [ 0x00, 0x00, From d97523303ea21bd88c81d8bc783dce1b07259fad Mon Sep 17 00:00:00 2001 From: Andrew Kolos Date: Tue, 21 Jan 2025 07:33:34 -0800 Subject: [PATCH 6/8] remove unreferenced `decodeMDnsQuery` function --- packages/multicast_dns/lib/src/packet.dart | 35 ---------------------- 1 file changed, 35 deletions(-) diff --git a/packages/multicast_dns/lib/src/packet.dart b/packages/multicast_dns/lib/src/packet.dart index adf63dc53d9..b064ef57750 100644 --- a/packages/multicast_dns/lib/src/packet.dart +++ b/packages/multicast_dns/lib/src/packet.dart @@ -187,41 +187,6 @@ _FQDNReadResult _readFQDN( return _FQDNReadResult(parts, highestOffsetRead - prevOffset); } -/// Decode an mDNS query packet. -/// -/// If decoding fails (e.g. due to an invalid packet), `null` is returned. -/// -/// See https://tools.ietf.org/html/rfc1035 for format. -ResourceRecordQuery? decodeMDnsQuery(List packet) { - final int length = packet.length; - if (length < _kHeaderSize) { - return null; - } - - final Uint8List data = - packet is Uint8List ? packet : Uint8List.fromList(packet); - final ByteData packetBytes = ByteData.view(data.buffer); - - // Check whether it's a query. - final int flags = packetBytes.getUint16(_kFlagsOffset); - if (flags != 0) { - return null; - } - final int questionCount = packetBytes.getUint16(_kQdcountOffset); - if (questionCount == 0) { - return null; - } - - final _FQDNReadResult fqdn = - _readFQDN(data, packetBytes, _kHeaderSize, data.length); - - int offset = _kHeaderSize + fqdn.bytesRead; - final int type = packetBytes.getUint16(offset); - offset += 2; - final int queryType = packetBytes.getUint16(offset) & 0x8000; - return ResourceRecordQuery(type, fqdn.fqdn, queryType); -} - /// Decode an mDNS response packet. /// /// If decoding fails (e.g. due to an invalid packet) `null` is returned. From 8a68221180a436be950ad1136caade88059ded29 Mon Sep 17 00:00:00 2001 From: Andrew Kolos Date: Tue, 21 Jan 2025 07:33:51 -0800 Subject: [PATCH 7/8] fix messed up merge --- packages/multicast_dns/lib/src/packet.dart | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/multicast_dns/lib/src/packet.dart b/packages/multicast_dns/lib/src/packet.dart index b064ef57750..80bc189f588 100644 --- a/packages/multicast_dns/lib/src/packet.dart +++ b/packages/multicast_dns/lib/src/packet.dart @@ -121,10 +121,6 @@ String readFQDN(List packet, [int offset = 0]) { return _readFQDN(data, byteData, offset, data.length).fqdn; } -// Read a FQDN at the given offset. Returns a pair with the FQDN -// parts and the number of bytes consumed. -// -// If decoding fails (e.g. due to an invalid packet) `null` is returned. // Read a FQDN at the given offset. Returns a pair with the FQDN // parts and the number of bytes consumed. // From 0efa536de595690056ef367303f1bcdc7731c90b Mon Sep 17 00:00:00 2001 From: Andrew Kolos Date: Sun, 26 Jan 2025 13:43:33 -0800 Subject: [PATCH 8/8] improve cycle prevention --- packages/multicast_dns/lib/src/packet.dart | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/multicast_dns/lib/src/packet.dart b/packages/multicast_dns/lib/src/packet.dart index 80bc189f588..330397deaa8 100644 --- a/packages/multicast_dns/lib/src/packet.dart +++ b/packages/multicast_dns/lib/src/packet.dart @@ -136,26 +136,29 @@ _FQDNReadResult _readFQDN( final List parts = []; final int prevOffset = offset; final List offsetsToVisit = [offset]; - final Set visitedOffsets = {}; + int upperLimitOffset = offset; int highestOffsetRead = offset; while (offsetsToVisit.isNotEmpty) { offset = offsetsToVisit.removeLast(); - if (visitedOffsets.contains(offset)) { - throw MDnsDecodeException(offset); - } - visitedOffsets.add(offset); while (true) { // At least one byte is required. checkLength(offset + 1); // Check for compressed. if (data[offset] & 0xc0 == 0xc0) { - // At least two bytes are required for a compressed FQDN. + // At least two bytes are required for a compressed FQDN (see RFC1035 section 4.1.4). checkLength(offset + 2); // A compressed FQDN has a new offset in the lower 14 bits. - offsetsToVisit.add(byteData.getUint16(offset) & ~0xc000); + final int pointerDest = byteData.getUint16(offset) & ~0xc000; + // Pointers can only point to prior occurances of some name. + // This check also guards against pointers that form loops. + if (pointerDest >= upperLimitOffset) { + throw MDnsDecodeException(offset); + } + upperLimitOffset = pointerDest; + offsetsToVisit.add(pointerDest); highestOffsetRead = max(highestOffsetRead, offset + 2); break; } else {