From 6533293f75ea3628930a7f2557975edf30477dc3 Mon Sep 17 00:00:00 2001 From: jack Date: Tue, 30 Sep 2025 13:49:21 +0200 Subject: [PATCH] Fix critical issues from PR #681 review Critical fixes: - BinaryProtocol: Return nil for unknown versions (prevents buffer underflows) - Add BinaryProtocol.Offsets struct to centralize magic numbers - Replace magic offset calculations with named constants Security/Privacy: - FileAttachmentView: Use url.lastPathComponent instead of url.path (prevents exposing full system paths) Documentation: - Fix compression algorithm documentation (zlib, not LZ4) All tests passing. --- bitchat/Protocols/BinaryProtocol.swift | 21 ++++++++++++++----- bitchat/Services/BLEService.swift | 8 +++---- bitchat/Views/Media/FileAttachmentView.swift | 2 +- .../NotificationStreamAssemblerTests.swift | 5 ++--- .../Protocol/BinaryProtocolTests.swift | 5 ++++- 5 files changed, 26 insertions(+), 15 deletions(-) diff --git a/bitchat/Protocols/BinaryProtocol.swift b/bitchat/Protocols/BinaryProtocol.swift index 78c80313..eb571da2 100644 --- a/bitchat/Protocols/BinaryProtocol.swift +++ b/bitchat/Protocols/BinaryProtocol.swift @@ -52,7 +52,7 @@ /// ## Flag Bits /// - Bit 0: Has recipient ID (directed message) /// - Bit 1: Has signature (authenticated message) -/// - Bit 2: Is compressed (LZ4 compression applied) +/// - Bit 2: Is compressed (zlib compression applied) /// - Bits 3-7: Reserved for future use /// /// ## Size Constraints @@ -111,11 +111,20 @@ struct BinaryProtocol { static let recipientIDSize = 8 static let signatureSize = 64 - static func headerSize(for version: UInt8) -> Int { + // Field offsets within packet header + struct Offsets { + static let version = 0 + static let type = 1 + static let ttl = 2 + static let timestamp = 3 + static let flags = 11 // After version(1) + type(1) + ttl(1) + timestamp(8) + } + + static func headerSize(for version: UInt8) -> Int? { switch version { case 1: return v1HeaderSize case 2: return v2HeaderSize - default: return 0 + default: return nil } } @@ -156,7 +165,8 @@ struct BinaryProtocol { if version == 1 && payloadDataSize > Int(UInt16.max) { return nil } if version == 2 && payloadDataSize > Int(UInt32.max) { return nil } - let estimatedHeader = headerSize(for: version) + senderIDSize + (packet.recipientID == nil ? 0 : recipientIDSize) + guard let headerSize = headerSize(for: version) else { return nil } + let estimatedHeader = headerSize + senderIDSize + (packet.recipientID == nil ? 0 : recipientIDSize) let estimatedPayload = payloadDataSize let estimatedSignature = (packet.signature == nil ? 0 : signatureSize) var data = Data() @@ -274,7 +284,8 @@ struct BinaryProtocol { guard let version = read8(), version == 1 || version == 2 else { return nil } let lengthFieldBytes = lengthFieldSize(for: version) - let minimumRequired = headerSize(for: version) + senderIDSize + guard let headerSize = headerSize(for: version) else { return nil } + let minimumRequired = headerSize + senderIDSize guard raw.count >= minimumRequired else { return nil } guard let type = read8(), let ttl = read8() else { return nil } diff --git a/bitchat/Services/BLEService.swift b/bitchat/Services/BLEService.swift index da250a00..9830e46a 100644 --- a/bitchat/Services/BLEService.swift +++ b/bitchat/Services/BLEService.swift @@ -45,18 +45,16 @@ struct NotificationStreamAssembler { continue } - let headerSize = BinaryProtocol.headerSize(for: version) - let framePrefix = headerSize + BinaryProtocol.senderIDSize - guard headerSize > 0 else { + guard let headerSize = BinaryProtocol.headerSize(for: version) else { dropped.append(buffer.removeFirst()) pendingFrameStartedAt = nil pendingFrameExpectedLength = 0 continue } + let framePrefix = headerSize + BinaryProtocol.senderIDSize guard buffer.count >= framePrefix else { break } - let flagsOffset = 1 + 1 + 1 + 8 // version + type + ttl + timestamp - let flagsIndex = buffer.startIndex + flagsOffset + let flagsIndex = buffer.startIndex + BinaryProtocol.Offsets.flags guard flagsIndex < buffer.endIndex else { break } let flags = buffer[flagsIndex] let hasRecipient = (flags & BinaryProtocol.Flags.hasRecipient) != 0 diff --git a/bitchat/Views/Media/FileAttachmentView.swift b/bitchat/Views/Media/FileAttachmentView.swift index 7540bcb9..0960a4b7 100644 --- a/bitchat/Views/Media/FileAttachmentView.swift +++ b/bitchat/Views/Media/FileAttachmentView.swift @@ -38,7 +38,7 @@ struct FileAttachmentView: View { .font(.bitchatSystem(size: 14, weight: .medium)) .foregroundColor(.primary) .lineLimit(2) - Text(url.path) + Text(url.lastPathComponent) .font(.bitchatSystem(size: 11, design: .monospaced)) .foregroundColor(.secondary) .lineLimit(1) diff --git a/bitchatTests/NotificationStreamAssemblerTests.swift b/bitchatTests/NotificationStreamAssemblerTests.swift index e366bb94..9aadefed 100644 --- a/bitchatTests/NotificationStreamAssemblerTests.swift +++ b/bitchatTests/NotificationStreamAssemblerTests.swift @@ -127,9 +127,8 @@ struct NotificationStreamAssemblerTests { return XCTFail("Failed to encode packet frame") } - let flagsOffset = 1 + 1 + 1 + 8 - XCTAssertLessThan(flagsOffset, frame.count) - let flags = frame[frame.startIndex + flagsOffset] + XCTAssertLessThan(BinaryProtocol.Offsets.flags, frame.count) + let flags = frame[frame.startIndex + BinaryProtocol.Offsets.flags] XCTAssertNotEqual(flags & BinaryProtocol.Flags.isCompressed, 0, "Frame should be compressed for large payloads") let splitIndex = min(4096, frame.count / 2) diff --git a/bitchatTests/Protocol/BinaryProtocolTests.swift b/bitchatTests/Protocol/BinaryProtocolTests.swift index bab28975..425722ec 100644 --- a/bitchatTests/Protocol/BinaryProtocolTests.swift +++ b/bitchatTests/Protocol/BinaryProtocolTests.swift @@ -68,7 +68,10 @@ struct BinaryProtocolTests { let encodedData = try #require(BinaryProtocol.encode(packet), "Failed to encode packet with large payload") // The encoded size should be smaller than uncompressed due to compression - let headerSize = BinaryProtocol.headerSize(for: packet.version) + guard let headerSize = BinaryProtocol.headerSize(for: packet.version) else { + XCTFail("Invalid version") + return + } let uncompressedSize = headerSize + BinaryProtocol.senderIDSize + largePayload.count #expect(encodedData.count < uncompressedSize)