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.
This commit is contained in:
jack
2025-10-14 22:26:03 +02:00
parent 367addf138
commit 747551f35a
5 changed files with 26 additions and 15 deletions
+16 -5
View File
@@ -52,7 +52,7 @@
/// ## Flag Bits /// ## Flag Bits
/// - Bit 0: Has recipient ID (directed message) /// - Bit 0: Has recipient ID (directed message)
/// - Bit 1: Has signature (authenticated 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 /// - Bits 3-7: Reserved for future use
/// ///
/// ## Size Constraints /// ## Size Constraints
@@ -111,11 +111,20 @@ struct BinaryProtocol {
static let recipientIDSize = 8 static let recipientIDSize = 8
static let signatureSize = 64 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 { switch version {
case 1: return v1HeaderSize case 1: return v1HeaderSize
case 2: return v2HeaderSize 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 == 1 && payloadDataSize > Int(UInt16.max) { return nil }
if version == 2 && payloadDataSize > Int(UInt32.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 estimatedPayload = payloadDataSize
let estimatedSignature = (packet.signature == nil ? 0 : signatureSize) let estimatedSignature = (packet.signature == nil ? 0 : signatureSize)
var data = Data() var data = Data()
@@ -274,7 +284,8 @@ struct BinaryProtocol {
guard let version = read8(), version == 1 || version == 2 else { return nil } guard let version = read8(), version == 1 || version == 2 else { return nil }
let lengthFieldBytes = lengthFieldSize(for: version) 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 raw.count >= minimumRequired else { return nil }
guard let type = read8(), let ttl = read8() else { return nil } guard let type = read8(), let ttl = read8() else { return nil }
+3 -5
View File
@@ -45,18 +45,16 @@ struct NotificationStreamAssembler {
continue continue
} }
let headerSize = BinaryProtocol.headerSize(for: version) guard let headerSize = BinaryProtocol.headerSize(for: version) else {
let framePrefix = headerSize + BinaryProtocol.senderIDSize
guard headerSize > 0 else {
dropped.append(buffer.removeFirst()) dropped.append(buffer.removeFirst())
pendingFrameStartedAt = nil pendingFrameStartedAt = nil
pendingFrameExpectedLength = 0 pendingFrameExpectedLength = 0
continue continue
} }
let framePrefix = headerSize + BinaryProtocol.senderIDSize
guard buffer.count >= framePrefix else { break } guard buffer.count >= framePrefix else { break }
let flagsOffset = 1 + 1 + 1 + 8 // version + type + ttl + timestamp let flagsIndex = buffer.startIndex + BinaryProtocol.Offsets.flags
let flagsIndex = buffer.startIndex + flagsOffset
guard flagsIndex < buffer.endIndex else { break } guard flagsIndex < buffer.endIndex else { break }
let flags = buffer[flagsIndex] let flags = buffer[flagsIndex]
let hasRecipient = (flags & BinaryProtocol.Flags.hasRecipient) != 0 let hasRecipient = (flags & BinaryProtocol.Flags.hasRecipient) != 0
+1 -1
View File
@@ -38,7 +38,7 @@ struct FileAttachmentView: View {
.font(.bitchatSystem(size: 14, weight: .medium)) .font(.bitchatSystem(size: 14, weight: .medium))
.foregroundColor(.primary) .foregroundColor(.primary)
.lineLimit(2) .lineLimit(2)
Text(url.path) Text(url.lastPathComponent)
.font(.bitchatSystem(size: 11, design: .monospaced)) .font(.bitchatSystem(size: 11, design: .monospaced))
.foregroundColor(.secondary) .foregroundColor(.secondary)
.lineLimit(1) .lineLimit(1)
@@ -140,9 +140,8 @@ final class NotificationStreamAssemblerTests: XCTestCase {
return XCTFail("Failed to encode packet frame") return XCTFail("Failed to encode packet frame")
} }
let flagsOffset = 1 + 1 + 1 + 8 XCTAssertLessThan(BinaryProtocol.Offsets.flags, frame.count)
XCTAssertLessThan(flagsOffset, frame.count) let flags = frame[frame.startIndex + BinaryProtocol.Offsets.flags]
let flags = frame[frame.startIndex + flagsOffset]
XCTAssertNotEqual(flags & BinaryProtocol.Flags.isCompressed, 0, "Frame should be compressed for large payloads") XCTAssertNotEqual(flags & BinaryProtocol.Flags.isCompressed, 0, "Frame should be compressed for large payloads")
let splitIndex = min(4096, frame.count / 2) let splitIndex = min(4096, frame.count / 2)
@@ -90,7 +90,10 @@ final class BinaryProtocolTests: XCTestCase {
} }
// The encoded size should be smaller than uncompressed due to compression // 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 let uncompressedSize = headerSize + BinaryProtocol.senderIDSize + largePayload.count
XCTAssertLessThan(encodedData.count, uncompressedSize) XCTAssertLessThan(encodedData.count, uncompressedSize)