Fix crash in BinaryProtocol.decode due to out-of-bounds data access

- Add comprehensive bounds checking before all data access operations
- Validate payload lengths don't exceed available data
- Protect against integer overflow in size calculations
- Handle malformed compressed packets gracefully
- Add extensive test coverage for edge cases

This fixes crashes when receiving malformed Bluetooth packets that claim
payload sizes larger than the actual data available.
This commit is contained in:
jack
2025-08-01 16:52:21 +02:00
parent aa1ecf40fc
commit 69c8161cf8
2 changed files with 312 additions and 30 deletions
+52 -26
View File
@@ -212,24 +212,30 @@ struct BinaryProtocol {
// Remove padding first
let unpaddedData = MessagePadding.unpad(data)
// Minimum size check: header + senderID
guard unpaddedData.count >= headerSize + senderIDSize else {
return nil
}
var offset = 0
// Header
// Header parsing with bounds checks
guard offset + 1 <= unpaddedData.count else { return nil }
let version = unpaddedData[offset]; offset += 1
// Check if version is supported
guard ProtocolVersion.isSupported(version) else {
// Log unsupported version for debugging
return nil
}
guard offset + 1 <= unpaddedData.count else { return nil }
let type = unpaddedData[offset]; offset += 1
guard offset + 1 <= unpaddedData.count else { return nil }
let ttl = unpaddedData[offset]; offset += 1
// Timestamp
// Timestamp - need 8 bytes
guard offset + 8 <= unpaddedData.count else { return nil }
let timestampData = unpaddedData[offset..<offset+8]
let timestamp = timestampData.reduce(0) { result, byte in
(result << 8) | UInt64(byte)
@@ -237,73 +243,93 @@ struct BinaryProtocol {
offset += 8
// Flags
guard offset + 1 <= unpaddedData.count else { return nil }
let flags = unpaddedData[offset]; offset += 1
let hasRecipient = (flags & Flags.hasRecipient) != 0
let hasSignature = (flags & Flags.hasSignature) != 0
let isCompressed = (flags & Flags.isCompressed) != 0
// Payload length
// Payload length - need 2 bytes
guard offset + 2 <= unpaddedData.count else { return nil }
let payloadLengthData = unpaddedData[offset..<offset+2]
let payloadLength = payloadLengthData.reduce(0) { result, byte in
(result << 8) | UInt16(byte)
}
offset += 2
// Calculate expected total size
var expectedSize = headerSize + senderIDSize + Int(payloadLength)
if hasRecipient {
expectedSize += recipientIDSize
}
if hasSignature {
expectedSize += signatureSize
}
// Validate payloadLength is reasonable (prevent integer overflow)
guard payloadLength <= 65535 else { return nil }
guard unpaddedData.count >= expectedSize else {
return nil
}
// SenderID
// SenderID - need 8 bytes
guard offset + senderIDSize <= unpaddedData.count else { return nil }
let senderID = unpaddedData[offset..<offset+senderIDSize]
offset += senderIDSize
// RecipientID
// RecipientID if present
var recipientID: Data?
if hasRecipient {
guard offset + recipientIDSize <= unpaddedData.count else { return nil }
recipientID = unpaddedData[offset..<offset+recipientIDSize]
offset += recipientIDSize
}
// Payload
// Payload handling with comprehensive bounds checking
let payload: Data
if isCompressed {
// First 2 bytes are original size
// Compressed payload needs at least 2 bytes for original size
guard Int(payloadLength) >= 2 else { return nil }
// Check we have enough data for the original size prefix
guard offset + 2 <= unpaddedData.count else { return nil }
let originalSizeData = unpaddedData[offset..<offset+2]
let originalSize = Int(originalSizeData.reduce(0) { result, byte in
(result << 8) | UInt16(byte)
})
offset += 2
// Compressed payload
let compressedPayload = unpaddedData[offset..<offset+Int(payloadLength)-2]
offset += Int(payloadLength) - 2
// Validate original size is reasonable
guard originalSize >= 0 && originalSize <= 1048576 else { return nil } // Max 1MB
// Decompress
// Check we have enough data for the compressed payload
let compressedPayloadSize = Int(payloadLength) - 2
guard compressedPayloadSize >= 0 && offset + compressedPayloadSize <= unpaddedData.count else {
return nil
}
let compressedPayload = unpaddedData[offset..<offset+compressedPayloadSize]
offset += compressedPayloadSize
// Decompress with error handling
guard let decompressedPayload = CompressionUtil.decompress(compressedPayload, originalSize: originalSize) else {
return nil
}
// Verify decompressed size matches expected
guard decompressedPayload.count == originalSize else {
return nil
}
payload = decompressedPayload
} else {
// Uncompressed payload
guard Int(payloadLength) >= 0 && offset + Int(payloadLength) <= unpaddedData.count else {
return nil
}
payload = unpaddedData[offset..<offset+Int(payloadLength)]
offset += Int(payloadLength)
}
// Signature
// Signature if present
var signature: Data?
if hasSignature {
guard offset + signatureSize <= unpaddedData.count else { return nil }
signature = unpaddedData[offset..<offset+signatureSize]
offset += signatureSize
}
// Final validation: ensure we haven't gone past the end
guard offset <= unpaddedData.count else { return nil }
return BitchatPacket(
type: type,
senderID: senderID,
+260 -4
View File
@@ -121,8 +121,9 @@ final class BinaryProtocolTests: XCTestCase {
func testMessagePadding() throws {
let payloads = [
"Short",
"This is a medium length message for testing",
TestConstants.testLongMessage
String(repeating: "Medium length message content ", count: 10), // ~300 bytes
String(repeating: "Long message content that should exceed the 512 byte limit ", count: 20), // ~1200+ bytes
String(repeating: "Very long message content that should definitely exceed the 2048 byte limit for sure ", count: 30) // ~2700+ bytes
]
var encodedSizes = Set<Int>()
@@ -150,8 +151,8 @@ final class BinaryProtocolTests: XCTestCase {
XCTAssertEqual(String(data: decodedPacket.payload, encoding: .utf8), payload)
}
// Different payload sizes should result in different padded sizes
XCTAssertGreaterThan(encodedSizes.count, 1)
// Different payload sizes should result in at least 2 different padded sizes
XCTAssertGreaterThanOrEqual(encodedSizes.count, 2, "Expected at least 2 different padded sizes, got \(encodedSizes)")
}
// MARK: - Message Encoding/Decoding Tests
@@ -312,4 +313,259 @@ final class BinaryProtocolTests: XCTestCase {
// Should fail to decode
XCTAssertNil(BinaryProtocol.decode(encoded))
}
// MARK: - Bounds Checking Tests (Crash Prevention)
func testMalformedPacketWithInvalidPayloadLength() throws {
// Test the specific crash scenario: payloadLength = 193 (0xc1) but only 30 bytes available
var malformedData = Data()
// Valid header (13 bytes)
malformedData.append(1) // version
malformedData.append(1) // type
malformedData.append(10) // ttl
// Timestamp (8 bytes)
for _ in 0..<8 {
malformedData.append(0)
}
malformedData.append(0) // flags (no recipient, no signature, not compressed)
// Invalid payload length: 193 (0x00c1) but we'll only provide 8 bytes total data
malformedData.append(0x00) // high byte
malformedData.append(0xc1) // low byte (193)
// SenderID (8 bytes) - this brings us to 21 bytes total
for _ in 0..<8 {
malformedData.append(0x01)
}
// Only provide 8 more bytes instead of the claimed 193
for _ in 0..<8 {
malformedData.append(0x02)
}
// Total data is now 30 bytes, but payloadLength claims 193
XCTAssertEqual(malformedData.count, 30)
// This should not crash - should return nil gracefully
let result = BinaryProtocol.decode(malformedData)
XCTAssertNil(result, "Malformed packet with invalid payload length should return nil, not crash")
}
func testTruncatedPacketHandling() throws {
// Test various truncation scenarios
let packet = TestHelpers.createTestPacket()
guard let validEncoded = BinaryProtocol.encode(packet) else {
XCTFail("Failed to encode test packet")
return
}
// Test truncation at various points
let truncationPoints = [0, 5, 10, 15, 20, 25]
for point in truncationPoints {
let truncated = validEncoded.prefix(point)
let result = BinaryProtocol.decode(truncated)
XCTAssertNil(result, "Truncated packet at \(point) bytes should return nil, not crash")
}
}
func testMalformedCompressedPacket() throws {
// Test compressed packet with invalid original size
var malformedData = Data()
// Valid header
malformedData.append(1) // version
malformedData.append(1) // type
malformedData.append(10) // ttl
// Timestamp (8 bytes)
for _ in 0..<8 {
malformedData.append(0)
}
malformedData.append(0x04) // flags: isCompressed = true
// Small payload length that's insufficient for compression
malformedData.append(0x00) // high byte
malformedData.append(0x01) // low byte (1 byte - insufficient for 2-byte original size)
// SenderID (8 bytes)
for _ in 0..<8 {
malformedData.append(0x01)
}
// Only 1 byte of "compressed" data (should need at least 2 for original size)
malformedData.append(0x99)
// Should handle this gracefully
let result = BinaryProtocol.decode(malformedData)
XCTAssertNil(result, "Malformed compressed packet should return nil, not crash")
}
func testExcessivelyLargePayloadLength() throws {
// Test packet claiming extremely large payload
var malformedData = Data()
// Valid header
malformedData.append(1) // version
malformedData.append(1) // type
malformedData.append(10) // ttl
// Timestamp (8 bytes)
for _ in 0..<8 {
malformedData.append(0)
}
malformedData.append(0) // flags
// Maximum payload length (65535)
malformedData.append(0xFF) // high byte
malformedData.append(0xFF) // low byte
// SenderID (8 bytes)
for _ in 0..<8 {
malformedData.append(0x01)
}
// Provide only a tiny amount of actual data
malformedData.append(contentsOf: [0x01, 0x02, 0x03])
// Should handle this gracefully without trying to allocate massive amounts of memory
let result = BinaryProtocol.decode(malformedData)
XCTAssertNil(result, "Packet with excessive payload length should return nil, not crash")
}
func testCompressedPacketWithInvalidOriginalSize() throws {
// Test compressed packet with unreasonable original size
var malformedData = Data()
// Valid header
malformedData.append(1) // version
malformedData.append(1) // type
malformedData.append(10) // ttl
// Timestamp (8 bytes)
for _ in 0..<8 {
malformedData.append(0)
}
malformedData.append(0x04) // flags: isCompressed = true
// Reasonable payload length
malformedData.append(0x00) // high byte
malformedData.append(0x10) // low byte (16 bytes)
// SenderID (8 bytes)
for _ in 0..<8 {
malformedData.append(0x01)
}
// Original size claiming to be extremely large (2MB)
malformedData.append(0x20) // high byte of original size
malformedData.append(0x00) // low byte of original size (0x2000 = 8192, but let's make it larger with more bytes)
// Add more bytes to make it claim larger size - but this will be invalid
// because our validation should catch unreasonable sizes
malformedData.append(contentsOf: [0x01, 0x02, 0x03, 0x04]) // Some compressed data
// Pad to match payload length
while malformedData.count < 21 + 16 { // header + senderID + payload
malformedData.append(0x00)
}
let result = BinaryProtocol.decode(malformedData)
XCTAssertNil(result, "Compressed packet with invalid original size should return nil, not crash")
}
func testMaliciousPacketWithIntegerOverflow() throws {
// Test packet designed to cause integer overflow
var maliciousData = Data()
// Valid header
maliciousData.append(1) // version
maliciousData.append(1) // type
maliciousData.append(10) // ttl
// Timestamp (8 bytes)
for _ in 0..<8 {
maliciousData.append(0)
}
// Set flags to have recipient and signature (increase expected size)
maliciousData.append(0x03) // hasRecipient | hasSignature
// Very large payload length
maliciousData.append(0xFF) // high byte
maliciousData.append(0xFE) // low byte (65534)
// SenderID (8 bytes)
for _ in 0..<8 {
maliciousData.append(0x01)
}
// RecipientID (8 bytes - required due to flag)
for _ in 0..<8 {
maliciousData.append(0x02)
}
// Provide minimal payload data - should trigger bounds check failure
maliciousData.append(contentsOf: [0x01, 0x02])
// Should handle gracefully without integer overflow issues
let result = BinaryProtocol.decode(maliciousData)
XCTAssertNil(result, "Malicious packet designed for integer overflow should return nil, not crash")
}
func testPartialHeaderData() throws {
// Test packets with incomplete headers
let headerSizes = [0, 1, 5, 10, 12] // Various incomplete header sizes
for size in headerSizes {
let partialData = Data(repeating: 0x01, count: size)
let result = BinaryProtocol.decode(partialData)
XCTAssertNil(result, "Partial header data (\(size) bytes) should return nil, not crash")
}
}
func testBoundaryConditions() throws {
// Test exact boundary conditions
let packet = TestHelpers.createTestPacket()
guard let validEncoded = BinaryProtocol.encode(packet) else {
XCTFail("Failed to encode test packet")
return
}
// Remove several bytes to ensure it fails - should fail gracefully
let truncated = validEncoded.dropLast(10)
let result = BinaryProtocol.decode(truncated)
XCTAssertNil(result, "Truncated packet should return nil, not crash")
// Test minimum valid size - create a valid minimal packet
var minData = Data()
minData.append(1) // version
minData.append(1) // type
minData.append(10) // ttl
// Timestamp (8 bytes)
for _ in 0..<8 {
minData.append(0)
}
minData.append(0) // flags (no optional fields)
minData.append(0) // payload length high byte
minData.append(0) // payload length low byte (0 payload)
// SenderID (8 bytes)
for _ in 0..<8 {
minData.append(0x01)
}
// This should be exactly the minimum size and should decode without crashing
let minResult = BinaryProtocol.decode(minData)
// The important thing is no crash occurs - result might be nil or valid
// We don't assert the result, just that no crash happens
}
}