Fix bug in DNSSD IPv6 address string conversion. (#14)

* Fix bug in DNSSD IPv6 address string conversion.

The old implementation converted each byte to a hexadecimal number
separately, meaning that leading zeros in a byte would be dropped
even in the middle of a group.

For example, the address 2620:149:110b:470e:0:0:0:e1a was incorrectly
rendered as              2620:149:11b:47e:0:0:0:e1a, with zeros dropped
in the 3rd and 4th octets.

I figured the most robust fix would be to use inet_ntop to format the
address, which produces a correctly abbreviated address like
2620:149:110b:470e::e1a. This is the approach used in DNSResolver_c-ares.swift.

* Throw error when too-short address is returned

* Fix formatting issues

* Print an error in soundness.sh if swiftformat is not installed

---------

Co-authored-by: Torin Rudeen <[email protected]>

Author: torinmr

Package.swift

@@ -12,7 +12,7 @@ var caresExclude = [
 ]
 
 do {
-    if !(try FileManager.default.contentsOfDirectory(atPath: "./Sources/CAsyncDNSResolver/c-ares/CMakeFiles").isEmpty) {
+    if try !(FileManager.default.contentsOfDirectory(atPath: "./Sources/CAsyncDNSResolver/c-ares/CMakeFiles").isEmpty) {
         caresExclude.append("./c-ares/CMakeFiles/")
     }
 } catch {

Sources/AsyncDNSResolver/AsyncDNSResolver.swift

@@ -26,7 +26,7 @@ public struct AsyncDNSResolver {
         #if canImport(Darwin)
         self.init(DNSSDDNSResolver())
         #else
-        self.init(try CAresDNSResolver())
+        try self.init(CAresDNSResolver())
         #endif
     }
 
@@ -44,7 +44,7 @@ public struct AsyncDNSResolver {
     /// - Parameters:
     ///   - options: Options to create ``CAresDNSResolver`` with.
     public init(options: CAresDNSResolver.Options) throws {
-        self.init(try CAresDNSResolver(options: options))
+        try self.init(CAresDNSResolver(options: options))
     }
 
     /// See ``DNSResolver/queryA(name:)``.

Sources/AsyncDNSResolver/dnssd/DNSResolver_dnssd.swift

@@ -220,19 +220,14 @@ extension DNSSD {
                 throw AsyncDNSResolver.Error.noData()
             }
 
-            let bufferPtr = UnsafeBufferPointer(start: ptr, count: Int(length))
-            var buffer = ByteBuffer(bytes: bufferPtr)
-
-            guard let addressBytes = buffer.readInteger(as: UInt32.self) else {
-                throw AsyncDNSResolver.Error.badResponse("failed to read address")
-            }
-
-            let address = withUnsafeBytes(of: addressBytes) { buffer in
-                let buffer = buffer.bindMemory(to: UInt8.self)
-                return "\(buffer[3]).\(buffer[2]).\(buffer[1]).\(buffer[0])"
+            guard length >= MemoryLayout<in_addr>.size else {
+                throw AsyncDNSResolver.Error.badResponse()
             }
 
-            return ARecord(address: .IPv4(address), ttl: nil)
+            var parsedAddressBytes = [CChar](repeating: 0, count: Int(INET_ADDRSTRLEN))
+            inet_ntop(AF_INET, ptr, &parsedAddressBytes, socklen_t(INET_ADDRSTRLEN))
+            let parsedAddress = String(cString: parsedAddressBytes)
+            return ARecord(address: .IPv4(parsedAddress), ttl: nil)
         }
 
         func generateReply(records: [ARecord]) throws -> [ARecord] {
@@ -248,18 +243,14 @@ extension DNSSD {
                 throw AsyncDNSResolver.Error.noData()
             }
 
-            let bufferPtr = UnsafeBufferPointer(start: ptr, count: Int(length))
-            var buffer = ByteBuffer(bytes: bufferPtr)
-
-            guard let addressBytes = buffer.readBytes(length: 16) else {
-                throw AsyncDNSResolver.Error.badResponse("failed to read address")
+            guard length >= MemoryLayout<in6_addr>.size else {
+                throw AsyncDNSResolver.Error.badResponse()
             }
 
-            let address = stride(from: 0, to: addressBytes.endIndex, by: 2).map {
-                "\(String(addressBytes[$0], radix: 16))\(String(addressBytes[$0.advanced(by: 1)], radix: 16))"
-            }.joined(separator: ":")
-
-            return AAAARecord(address: .IPv6(address), ttl: nil)
+            var parsedAddressBytes = [CChar](repeating: 0, count: Int(INET6_ADDRSTRLEN))
+            inet_ntop(AF_INET6, ptr, &parsedAddressBytes, socklen_t(INET6_ADDRSTRLEN))
+            let parsedAddress = String(cString: parsedAddressBytes)
+            return AAAARecord(address: .IPv6(parsedAddress), ttl: nil)
         }
 
         func generateReply(records: [AAAARecord]) throws -> [AAAARecord] {

Tests/AsyncDNSResolverTests/dnssd/DNSSDDNSResolverTests.swift

@@ -104,6 +104,36 @@ final class DNSSDDNSResolverTests: XCTestCase {
         XCTAssertFalse(reply.isEmpty, "should have SRV record(s)")
     }
 
+    func test_parseA() throws {
+        let addrBytes: [UInt8] = [38, 32, 1, 73]
+        try addrBytes.withUnsafeBufferPointer {
+            let record = try DNSSD.AQueryReplyHandler.instance.parseRecord(data: $0.baseAddress, length: UInt16($0.count))
+            XCTAssertEqual(record, ARecord(address: .IPv4("38.32.1.73"), ttl: nil))
+        }
+    }
+
+    func test_parseATooShort() throws {
+        let addrBytes: [UInt8] = [38, 32, 1]
+        try addrBytes.withUnsafeBufferPointer {
+            XCTAssertThrowsError(
+                try DNSSD.AQueryReplyHandler.instance.parseRecord(
+                    data: $0.baseAddress, length: UInt16($0.count)
+                )
+            )
+        }
+    }
+
+    func test_parseAAAATooShort() throws {
+        let addrBytes: [UInt8] = [38, 32, 1, 73, 17, 11, 71, 14, 0, 0, 0, 0, 0, 0, 14]
+        try addrBytes.withUnsafeBufferPointer {
+            XCTAssertThrowsError(
+                try DNSSD.AAAAQueryReplyHandler.instance.parseRecord(
+                    data: $0.baseAddress, length: UInt16($0.count)
+                )
+            )
+        }
+    }
+
     func test_concurrency() async throws {
         func run(
             times: Int = 100,

scripts/soundness.sh

@@ -21,6 +21,12 @@ function replace_acceptable_years() {
     sed -e 's/202[012]-202[123]/YEARS/' -e 's/202[0123]/YEARS/'
 }
 
+if ! hash swiftformat &> /dev/null
+then
+  printf "\033[0;31mPlease install swiftformat (https://github.com/nicklockwood/SwiftFormat) and run again.\033[0m\n"
+  exit 1
+fi
+
 printf "=> Checking format... "
 FIRST_OUT="$(git status --porcelain)"
 swiftformat . > /dev/null 2>&1