From d38ac78b6d28cb7f16439f30e8cf06815fd139fc Mon Sep 17 00:00:00 2001 From: Quentin Zervaas Date: Thu, 9 Apr 2026 21:32:57 +0930 Subject: [PATCH] Fix packet validation, bounds checking, and mock server state handling - Validate CRC16 on received packets, reject mismatched checksums - Reject truncated packets instead of accepting with zeroed CRC - Fix off-by-one in RepeatingData normalBytes boundary check - Stop RepeatingData loop from advancing index past buffer bounds - Add bounds-safe loop conditions in GroupNameResponse and UnitAbilitiesResponse - Use clamping conversion for temperature UInt8 encoding - Remove orphaned duplicate connection in demo client - Add group state to mock server, handle group control and status properly - Fix test filename typo (ReponseTests -> ResponseTests) - Add malformed input test suite (18 tests) --- .gitignore | 2 + .../Classes/AirTouch2PlusClient.swift | 25 -- .../AirTouch2PlusMockServer.swift | 21 +- .../AirTouchMockServer/SystemState.swift | 14 + .../Messages/GroupNameResponseMessage.swift | 8 +- .../UnitAbilitiesResponseMessage.swift | 12 +- .../AirTouch2Plus/Packet/Packet+Codable.swift | 27 +- .../Packet/Packet+RepeatingData.swift | 17 +- .../MalformedInputTests.swift | 269 ++++++++++++++++++ ...ReponseTests.swift => ResponseTests.swift} | 0 10 files changed, 342 insertions(+), 53 deletions(-) create mode 100644 Tests/AirTouch2PlusTests/MalformedInputTests.swift rename Tests/AirTouch2PlusTests/{ReponseTests.swift => ResponseTests.swift} (100%) diff --git a/.gitignore b/.gitignore index e43b0f9..28d3de1 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,3 @@ .DS_Store +CLAUDE.md +.claude/ diff --git a/AirTouchDemo/AirTouchDemo/Classes/AirTouch2PlusClient.swift b/AirTouchDemo/AirTouchDemo/Classes/AirTouch2PlusClient.swift index 5451d4d..7433bd8 100644 --- a/AirTouchDemo/AirTouchDemo/Classes/AirTouch2PlusClient.swift +++ b/AirTouchDemo/AirTouchDemo/Classes/AirTouch2PlusClient.swift @@ -19,31 +19,6 @@ public class AirTouch2PlusClient { public func connect(endpoint: NWEndpoint) { self.connection?.disconnect() - let connec = AirTouch2PlusConnection() { connection, state in - - } packetReceivedHandler: { connection, packet in - switch packet.airTouch2PlusPacket { - case let message as GroupStatusMessage: - // Handle message here - break - - case let message as UnitStatusMessage: - // Handle message here - break - - default: - break - } - } - - // Request uni status - let message = UnitStatusMessage(units: []) - let packet = Packet.request(messageID: nil, message) - - connec.send(packet: packet) { error in - - } - let connection = AirTouch2PlusConnection(stateUpdateHandler: stateUpdateHandler, packetReceivedHandler: packetReceivedHandler) connection.connect(endpoint: endpoint) self.connection = connection diff --git a/AirTouchDemo/AirTouchMockServer/AirTouch2PlusMockServer.swift b/AirTouchDemo/AirTouchMockServer/AirTouch2PlusMockServer.swift index 958e20e..fc4a0b9 100644 --- a/AirTouchDemo/AirTouchMockServer/AirTouch2PlusMockServer.swift +++ b/AirTouchDemo/AirTouchMockServer/AirTouch2PlusMockServer.swift @@ -70,7 +70,24 @@ public class AirTouch2PlusMockServer { } case let m as GroupControlMessage: - message = GroupStatusMessage(groups: []) + for group in m.groups { + if var existing = systemState.groupStatuses[group.groupID] { + if let power = group.power { + switch power { + case .on: + existing = .init(groupID: existing.groupID, power: .on, openPercentage: existing.openPercentage, turboIsSupported: existing.turboIsSupported, spillIsActive: existing.spillIsActive) + case .off: + existing = .init(groupID: existing.groupID, power: .off, openPercentage: existing.openPercentage, turboIsSupported: existing.turboIsSupported, spillIsActive: existing.spillIsActive) + case .turbo: + existing = .init(groupID: existing.groupID, power: .turbo, openPercentage: existing.openPercentage, turboIsSupported: existing.turboIsSupported, spillIsActive: existing.spillIsActive) + case .next: + break + } + } + systemState.groupStatuses[group.groupID] = existing + } + } + message = GroupStatusMessage(groups: systemState.sortedGroupStatuses) case let m as GroupNameRequestMessage: var groups: [GroupNameResponseMessage.Group] = [] @@ -91,7 +108,7 @@ public class AirTouch2PlusMockServer { message = GroupNameResponseMessage(groups: groups) case let m as GroupStatusMessage: - message = GroupStatusMessage(groups: []) + message = GroupStatusMessage(groups: systemState.sortedGroupStatuses) case let m as UnitAbilitiesRequestMessage: switch m { diff --git a/AirTouchDemo/AirTouchMockServer/SystemState.swift b/AirTouchDemo/AirTouchMockServer/SystemState.swift index 41e3c3b..2608c00 100644 --- a/AirTouchDemo/AirTouchMockServer/SystemState.swift +++ b/AirTouchDemo/AirTouchMockServer/SystemState.swift @@ -11,8 +11,16 @@ class SystemState { var unitStatuses: [UnitID: UnitStatusMessage.Unit] var unitAbilities: [UnitID: UnitAbilitiesResponseMessage.Unit] + var groupStatuses: [GroupID: GroupStatusMessage.Group] public init() { + groupStatuses = [ + 0: .init(groupID: 0, power: .on, openPercentage: 80, turboIsSupported: true, spillIsActive: false), + 1: .init(groupID: 1, power: .on, openPercentage: 60, turboIsSupported: true, spillIsActive: false), + 2: .init(groupID: 2, power: .off, openPercentage: 0, turboIsSupported: false, spillIsActive: false), + 3: .init(groupID: 3, power: .off, openPercentage: 0, turboIsSupported: false, spillIsActive: false), + ] + unitStatuses = [ 0: .init( unitID: 0, @@ -47,6 +55,12 @@ class SystemState { String(format: "Group \(id)") } + var sortedGroupStatuses: [GroupStatusMessage.Group] { + groupStatuses.map { $0.value }.sorted { a, b in + a.groupID < b.groupID + } + } + var sortedUnitStatuses: [UnitStatusMessage.Unit] { unitStatuses.map { $0.value }.sorted { a, b in a.unitID < b.unitID diff --git a/Sources/AirTouch2Plus/Messages/GroupNameResponseMessage.swift b/Sources/AirTouch2Plus/Messages/GroupNameResponseMessage.swift index ef1abc9..cf66fe3 100644 --- a/Sources/AirTouch2Plus/Messages/GroupNameResponseMessage.swift +++ b/Sources/AirTouch2Plus/Messages/GroupNameResponseMessage.swift @@ -83,11 +83,7 @@ extension GroupNameResponseMessage: ByteCodable { var groups: [Group] = [] - while idx < bytes.count { - defer { - idx += repeatingLength - } - + while idx + repeatingLength <= bytes.count { let bytes = bytes[idx ..< idx + repeatingLength] let group = Group(bytes: Array(bytes)) @@ -95,6 +91,8 @@ extension GroupNameResponseMessage: ByteCodable { if let group { groups.append(group) } + + idx += repeatingLength } self.groups = groups diff --git a/Sources/AirTouch2Plus/Messages/UnitAbilitiesResponseMessage.swift b/Sources/AirTouch2Plus/Messages/UnitAbilitiesResponseMessage.swift index 1c8921e..338f958 100644 --- a/Sources/AirTouch2Plus/Messages/UnitAbilitiesResponseMessage.swift +++ b/Sources/AirTouch2Plus/Messages/UnitAbilitiesResponseMessage.swift @@ -108,8 +108,8 @@ extension UnitAbilitiesResponseMessage.Unit: ByteCodable { numGroups, modesByte, fanSpeedsByte, - UInt8(minSetPoint.value), - UInt8(maxSetPoint.value) + UInt8(clamping: Int(minSetPoint.value.rounded())), + UInt8(clamping: Int(maxSetPoint.value.rounded())) ] } @@ -205,11 +205,7 @@ extension UnitAbilitiesResponseMessage: ByteCodable { var units: [Unit] = [] - while idx < bytes.count { - defer { - idx += repeatingLength - } - + while idx + repeatingLength <= bytes.count { let bytes = bytes[idx ..< idx + repeatingLength] let unit = Unit(bytes: Array(bytes)) @@ -217,6 +213,8 @@ extension UnitAbilitiesResponseMessage: ByteCodable { if let unit { units.append(unit) } + + idx += repeatingLength } self.units = units diff --git a/Sources/AirTouch2Plus/Packet/Packet+Codable.swift b/Sources/AirTouch2Plus/Packet/Packet+Codable.swift index 70f950f..b2567f6 100644 --- a/Sources/AirTouch2Plus/Packet/Packet+Codable.swift +++ b/Sources/AirTouch2Plus/Packet/Packet+Codable.swift @@ -32,23 +32,36 @@ extension Packet: ByteDecodable { idx = maxIdx let dataLength = header.body.length - maxIdx = min(idx + Int(dataLength), bytes.count) + maxIdx = idx + Int(dataLength) + + guard maxIdx <= bytes.count else { + return nil + } guard maxIdx > idx else { - self.data = [] - self.crc16 = 0 - return + return nil } self.data = Array(bytes[idx ..< maxIdx]) idx = maxIdx + guard idx + 2 <= bytes.count else { + return nil + } + let crc16Bytes: [UInt8] = [ - bytes[safe: idx] ?? 0x0, - bytes[safe: idx + 1] ?? 0x0 + bytes[idx], + bytes[idx + 1] ] idx += 2 - self.crc16 = crc16Bytes.int16 + let crc16 = crc16Bytes.int16 + let expectedCrc16 = (header.body.bytes + data).crc16Modbus + + guard crc16 == expectedCrc16 else { + return nil + } + + self.crc16 = crc16 } } diff --git a/Sources/AirTouch2Plus/Packet/Packet+RepeatingData.swift b/Sources/AirTouch2Plus/Packet/Packet+RepeatingData.swift index b7d5551..caa01f9 100644 --- a/Sources/AirTouch2Plus/Packet/Packet+RepeatingData.swift +++ b/Sources/AirTouch2Plus/Packet/Packet+RepeatingData.swift @@ -79,10 +79,11 @@ extension Packet.RepeatingData { if normalBytesLength > 0 { let maxIdx = idx + Int(normalBytesLength) - if maxIdx < bytes.count { - normalBytes = Array(bytes[idx ..< maxIdx]) + guard maxIdx <= bytes.count else { + return nil } + normalBytes = Array(bytes[idx ..< maxIdx]) idx = maxIdx } @@ -91,12 +92,14 @@ extension Packet.RepeatingData { for _ in 0 ..< repeatsCount { let maxIdx = idx + Int(repeatsLength) - if maxIdx <= bytes.count { - let repeatBytes = Array(bytes[idx ..< maxIdx]) + guard maxIdx <= bytes.count else { + break + } + + let repeatBytes = Array(bytes[idx ..< maxIdx]) - if let rep = T(bytes: repeatBytes) { - repeats.append(rep) - } + if let rep = T(bytes: repeatBytes) { + repeats.append(rep) } idx = maxIdx diff --git a/Tests/AirTouch2PlusTests/MalformedInputTests.swift b/Tests/AirTouch2PlusTests/MalformedInputTests.swift new file mode 100644 index 0000000..f7d005e --- /dev/null +++ b/Tests/AirTouch2PlusTests/MalformedInputTests.swift @@ -0,0 +1,269 @@ +// +// MalformedInputTests.swift +// + +import XCTest +@testable import AirTouch2Plus + +final class MalformedInputTests: XCTestCase { + + // MARK: - CRC Validation + + func testPacketWithCorrectCRC() throws { + let packet = Packet( + address: .init(bytes: [0x80, 0xb0]), + messageID: 0x01, + messageType: 0xc0, + data: [0x21, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00] + ) + + let decoded = Packet(bytes: packet.bytes) + XCTAssertNotNil(decoded) + XCTAssertEqual(decoded, packet) + } + + func testPacketWithIncorrectCRC() throws { + var bytes: [UInt8] = [ + 0x55, 0x55, + 0x80, 0xb0, 0x01, 0xc0, 0x00, 0x08, + 0x21, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0xa4, 0x31 + ] + + // Corrupt the CRC + bytes[bytes.count - 1] = 0x00 + + XCTAssertNil(Packet(bytes: bytes)) + } + + func testPacketWithCorruptedData() throws { + var bytes: [UInt8] = [ + 0x55, 0x55, + 0x80, 0xb0, 0x01, 0xc0, 0x00, 0x08, + 0x21, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0xa4, 0x31 + ] + + // Corrupt a data byte (CRC no longer matches) + bytes[8] = 0xFF + + XCTAssertNil(Packet(bytes: bytes)) + } + + // MARK: - Truncated Packets + + func testPacketTruncatedBeforeData() throws { + // Header only, no data or CRC + let bytes: [UInt8] = [ + 0x55, 0x55, + 0x80, 0xb0, 0x01, 0xc0, 0x00, 0x08 + ] + + XCTAssertNil(Packet(bytes: bytes)) + } + + func testPacketTruncatedData() throws { + // Header says length=8 but only 4 data bytes present + let bytes: [UInt8] = [ + 0x55, 0x55, + 0x80, 0xb0, 0x01, 0xc0, 0x00, 0x08, + 0x21, 0x00, 0x00, 0x00 + ] + + XCTAssertNil(Packet(bytes: bytes)) + } + + func testPacketMissingCRC() throws { + // Header + full data but no CRC bytes + let bytes: [UInt8] = [ + 0x55, 0x55, + 0x80, 0xb0, 0x01, 0xc0, 0x00, 0x08, + 0x21, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 + ] + + XCTAssertNil(Packet(bytes: bytes)) + } + + func testPacketTooShortForHeader() throws { + let bytes: [UInt8] = [0x55, 0x55, 0x80] + XCTAssertNil(Packet(bytes: bytes)) + } + + func testEmptyBytes() throws { + XCTAssertNil(Packet(bytes: [])) + } + + // MARK: - RepeatingData Bounds + + func testRepeatingDataNormalBytesAtBoundary() throws { + // normalBytesLength=2, repeatsCount=0, repeatsLength=0, normalBytes=[0xAA, 0xBB] + let bytes: [UInt8] = [ + 0x00, 0x02, // normalBytesLength = 2 + 0x00, 0x00, // repeatsCount = 0 + 0x00, 0x00, // repeatsLength = 0 + 0xAA, 0xBB // normalBytes (exactly at boundary) + ] + + let result = Packet.RepeatingData( + type: GroupStatusMessage.Group.self, + bytes: bytes + ) + + XCTAssertNotNil(result) + XCTAssertEqual(result?.normalBytes, [0xAA, 0xBB]) + } + + func testRepeatingDataNormalBytesPastBoundary() throws { + // Claims 4 normal bytes but only 2 available + let bytes: [UInt8] = [ + 0x00, 0x04, // normalBytesLength = 4 + 0x00, 0x00, // repeatsCount = 0 + 0x00, 0x00, // repeatsLength = 0 + 0xAA, 0xBB // only 2 bytes available + ] + + let result = Packet.RepeatingData( + type: GroupStatusMessage.Group.self, + bytes: bytes + ) + + XCTAssertNil(result) + } + + func testRepeatingDataTruncatedRepeats() throws { + // Claims 2 repeats of 8 bytes each, but only provides 1 + let bytes: [UInt8] = [ + 0x00, 0x00, // normalBytesLength = 0 + 0x00, 0x02, // repeatsCount = 2 + 0x00, 0x08, // repeatsLength = 8 + // Only 8 bytes (1 repeat) instead of 16 (2 repeats) + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x80, 0x00 + ] + + let result = Packet.RepeatingData( + type: GroupStatusMessage.Group.self, + bytes: bytes + ) + + XCTAssertNotNil(result) + // Should parse the one valid repeat and stop + XCTAssertEqual(result?.repeats.count, 1) + } + + func testRepeatingDataZeroRepeats() throws { + let bytes: [UInt8] = [ + 0x00, 0x00, // normalBytesLength = 0 + 0x00, 0x00, // repeatsCount = 0 + 0x00, 0x08, // repeatsLength = 8 + ] + + let result = Packet.RepeatingData( + type: GroupStatusMessage.Group.self, + bytes: bytes + ) + + XCTAssertNotNil(result) + XCTAssertEqual(result?.repeats.count, 0) + } + + // MARK: - Truncated Message Parsing + + func testGroupNameResponseTruncatedLastGroup() throws { + // Valid command + one full group (9 bytes) + partial second group + let commandBytes: [UInt8] = [0xff, 0x12] + let fullGroup: [UInt8] = [0x00, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x31, 0x00, 0x00] + let partialGroup: [UInt8] = [0x01, 0x4b, 0x69] // only 3 of 9 bytes + + let bytes = commandBytes + fullGroup + partialGroup + + // Should parse the first group and stop without crashing + let result = GroupNameResponseMessage(bytes: bytes) + + // The modular guard (bytes.count - 2) % 9 != 0 rejects this + XCTAssertNil(result) + } + + func testGroupNameResponseValidSingleGroup() throws { + let bytes: [UInt8] = [ + 0xff, 0x12, + 0x00, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x31, 0x00, 0x00 + ] + + let result = GroupNameResponseMessage(bytes: bytes) + XCTAssertNotNil(result) + XCTAssertEqual(result?.groups.count, 1) + XCTAssertEqual(result?.groups.first?.groupID, 0) + XCTAssertEqual(result?.groups.first?.name, "Group1") + } + + func testUnitAbilitiesResponseTruncatedLastUnit() throws { + // Valid command + partial unit data (less than 24 bytes) + let commandBytes: [UInt8] = [0xff, 0x11] + let partialUnit: [UInt8] = [0x00, 0x16, 0x55, 0x4e, 0x49, 0x54] // only 6 of 24 bytes + + let bytes = commandBytes + partialUnit + + // The modular guard (bytes.count - 2) % 24 != 0 rejects this + let result = UnitAbilitiesResponseMessage(bytes: bytes) + XCTAssertNil(result) + } + + func testUnitAbilitiesResponseValidSingleUnit() throws { + let bytes: [UInt8] = [ + 0xff, 0x11, 0x00, 0x16, + 0x55, 0x4e, 0x49, 0x54, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x04, 0x1b, 0x1d, 0x11, 0x1f + ] + + let result = UnitAbilitiesResponseMessage(bytes: bytes) + XCTAssertNotNil(result) + XCTAssertEqual(result?.units.count, 1) + XCTAssertEqual(result?.units.first?.unitName, "UNIT") + } + + // MARK: - Temperature Clamping + + func testTemperatureClampingInUnitAbilities() throws { + let unit = UnitAbilitiesResponseMessage.Unit( + unitID: 0, + unitName: "TEST", + startGroupNumber: 0, + numGroups: 1, + supportedModes: [.cool], + supportedFanSpeeds: [.auto], + minSetPoint: .init(0), + maxSetPoint: .init(255) + ) + + let bytes = unit.bytes + + // Verify min/max encode without crashing + XCTAssertEqual(bytes[bytes.count - 2], 0) + XCTAssertEqual(bytes[bytes.count - 1], 255) + + // Round-trip + let decoded = UnitAbilitiesResponseMessage.Unit(bytes: bytes) + XCTAssertNotNil(decoded) + XCTAssertEqual(decoded?.minSetPoint.value, 0) + XCTAssertEqual(decoded?.maxSetPoint.value, 255) + } + + func testTemperatureClampingOutOfRange() throws { + let unit = UnitAbilitiesResponseMessage.Unit( + unitID: 0, + unitName: "TEST", + startGroupNumber: 0, + numGroups: 1, + supportedModes: [.cool], + supportedFanSpeeds: [.auto], + minSetPoint: .init(-5), + maxSetPoint: .init(300) + ) + + // Should not crash — values get clamped + let bytes = unit.bytes + XCTAssertEqual(bytes[bytes.count - 2], 0) // -5 clamped to 0 + XCTAssertEqual(bytes[bytes.count - 1], 255) // 300 clamped to 255 + } +} diff --git a/Tests/AirTouch2PlusTests/ReponseTests.swift b/Tests/AirTouch2PlusTests/ResponseTests.swift similarity index 100% rename from Tests/AirTouch2PlusTests/ReponseTests.swift rename to Tests/AirTouch2PlusTests/ResponseTests.swift