fix BCH-01-002: prevent DoS via file storage exhaustion

Incoming files are now held in memory via PendingFileManager instead of being auto-saved to disk. Users must explicitly accept files before they are written. This prevents attackers from exhausting device storage. Key changes: - Add PendingFileManager with configurable limits (max 10 files, 5MB total) - Files auto-expire after 5 minutes if not accepted - LRU eviction when limits are exceeded - Pending files cleared during panic mode (emergencyDisconnectAll) - Add didReceivePendingFileTransfer delegate method - Add acceptPendingFile/declinePendingFile to Transport protocol Security audit reference: Cure53 BCH-01-002 (Medium severity) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-07-25 05:05:19 +00:00 · 2026-01-12 10:39:19 -10:00
parent b84c36c6fa
commit 4b3077169a
6 changed files with 683 additions and 57 deletions
@@ -134,18 +134,25 @@ struct FragmentationTests {
            }
        }

-        try await sleep(1.0)
+        // BCH-01-002: Files are now held as pending until user accepts (no auto-save to disk)
+        try await capture.waitForPendingFiles(count: 1, timeout: .seconds(2))

-        let message = try #require(capture.receivedMessages.first, "Expected file transfer message")
-        #expect(message.content.hasPrefix("[file]"))
+        let pending = try #require(capture.pendingFiles.first, "Expected pending file transfer")
+        #expect(pending.mimeType == "application/octet-stream")
+        #expect(pending.fileSize == FileTransferLimits.maxPayloadBytes)
+        #expect(pending.fileName == "limit.bin")

-        if let fileName = message.content.split(separator: " ").last {
-            let base = try FileManager.default.url(for: .applicationSupportDirectory, in: .userDomainMask, appropriateFor: nil, create: true)
-            let filesRoot = base.appendingPathComponent("files", isDirectory: true)
-            let incoming = filesRoot.appendingPathComponent("files/incoming", isDirectory: true)
-            let url = incoming.appendingPathComponent(String(fileName))
-            try? FileManager.default.removeItem(at: url)
+        // Verify file was NOT written to disk (the whole point of BCH-01-002 fix)
+        let base = try FileManager.default.url(for: .applicationSupportDirectory, in: .userDomainMask, appropriateFor: nil, create: true)
+        let filesRoot = base.appendingPathComponent("files", isDirectory: true)
+        let incoming = filesRoot.appendingPathComponent("files/incoming", isDirectory: true)
+        if FileManager.default.fileExists(atPath: incoming.path) {
+            let files = try FileManager.default.contentsOfDirectory(atPath: incoming.path)
+            #expect(files.isEmpty, "Files should NOT be auto-saved to disk before user acceptance")
        }
+
+        // Clean up pending files
+        PendingFileManager.shared.clearAll()
    }
    
    @Test("Invalid fragment header is ignored")
@@ -200,10 +207,13 @@ extension FragmentationTests {
        private let lock = NSLock()
        private var _publicMessages: [(peerID: PeerID, nickname: String, content: String)] = []
        private var _receivedMessages: [BitchatMessage] = []
+        private var _pendingFiles: [PendingFileTransfer] = []
        private var publicMessageContinuation: CheckedContinuation<Void, Never>?
        private var receivedMessageContinuation: CheckedContinuation<Void, Never>?
+        private var pendingFileContinuation: CheckedContinuation<Void, Never>?
        private var expectedPublicMessageCount: Int = 0
        private var expectedReceivedMessageCount: Int = 0
+        private var expectedPendingFileCount: Int = 0

        var publicMessages: [(peerID: PeerID, nickname: String, content: String)] {
            lock.lock()
@@ -217,6 +227,12 @@ extension FragmentationTests {
            return _receivedMessages
        }

+        var pendingFiles: [PendingFileTransfer] {
+            lock.lock()
+            defer { lock.unlock() }
+            return _pendingFiles
+        }
+
        func didReceiveMessage(_ message: BitchatMessage) {
            lock.lock()
            _receivedMessages.append(message)
@@ -233,6 +249,22 @@ extension FragmentationTests {
            }
        }

+        func didReceivePendingFileTransfer(_ pending: PendingFileTransfer) {
+            lock.lock()
+            _pendingFiles.append(pending)
+            let count = _pendingFiles.count
+            let expected = expectedPendingFileCount
+            let continuation = pendingFileContinuation
+            lock.unlock()
+
+            if count >= expected, let cont = continuation {
+                lock.lock()
+                pendingFileContinuation = nil
+                lock.unlock()
+                cont.resume()
+            }
+        }
+
        func didReceivePublicMessage(from peerID: PeerID, nickname: String, content: String, timestamp: Date, messageID: String?) {
            lock.lock()
            _publicMessages.append((peerID, nickname, content))
@@ -317,6 +349,38 @@ extension FragmentationTests {
            }
        }

+        /// Waits for the specified number of pending files to be received
+        func waitForPendingFiles(count: Int, timeout: Duration = .seconds(2)) async throws {
+            lock.lock()
+            if _pendingFiles.count >= count {
+                lock.unlock()
+                return
+            }
+            expectedPendingFileCount = count
+            lock.unlock()
+
+            try await withThrowingTaskGroup(of: Void.self) { group in
+                group.addTask {
+                    await withCheckedContinuation { continuation in
+                        self.lock.lock()
+                        if self._pendingFiles.count >= count {
+                            self.lock.unlock()
+                            continuation.resume()
+                            return
+                        }
+                        self.pendingFileContinuation = continuation
+                        self.lock.unlock()
+                    }
+                }
+                group.addTask {
+                    try await Task.sleep(for: timeout)
+                    throw CancellationError()
+                }
+                try await group.next()
+                group.cancelAll()
+            }
+        }
+
        func didConnectToPeer(_ peerID: PeerID) {}
        func didDisconnectFromPeer(_ peerID: PeerID) {}
        func didUpdatePeerList(_ peers: [PeerID]) {}
@@ -0,0 +1,288 @@
+//
+// PendingFileManagerTests.swift
+// bitchatTests
+//
+// This is free and unencumbered software released into the public domain.
+// For more information, see <https://unlicense.org>
+//
+
+import Testing
+import Foundation
+@testable import bitchat
+
+/// Tests for BCH-01-002 fix: PendingFileManager prevents DoS via storage exhaustion
+struct PendingFileManagerTests {
+
+    @Test("addPendingFile stores file in memory")
+    func addPendingFile_storesInMemory() {
+        let manager = PendingFileManager(config: .default)
+        defer { manager.clearAll() }
+
+        let content = Data(repeating: 0x42, count: 1024)
+        let pending = manager.addPendingFile(
+            senderPeerID: PeerID(str: "AABBCCDD11223344"),
+            senderNickname: "TestUser",
+            fileName: "test.bin",
+            mimeType: "application/octet-stream",
+            content: content,
+            isPrivate: false
+        )
+
+        #expect(pending != nil)
+        #expect(pending?.fileSize == 1024)
+        #expect(pending?.fileName == "test.bin")
+        #expect(manager.stats.count == 1)
+        #expect(manager.stats.totalBytes == 1024)
+    }
+
+    @Test("getPendingFile retrieves stored file")
+    func getPendingFile_retrievesStoredFile() {
+        let manager = PendingFileManager(config: .default)
+        defer { manager.clearAll() }
+
+        let content = Data(repeating: 0x42, count: 512)
+        let pending = manager.addPendingFile(
+            senderPeerID: PeerID(str: "AABBCCDD11223344"),
+            senderNickname: "TestUser",
+            fileName: "test.bin",
+            mimeType: "image/png",
+            content: content,
+            isPrivate: false
+        )
+
+        guard let id = pending?.id else {
+            Issue.record("Failed to add pending file")
+            return
+        }
+
+        let retrieved = manager.getPendingFile(id: id)
+        #expect(retrieved != nil)
+        #expect(retrieved?.content == content)
+    }
+
+    @Test("declineFile removes file from queue")
+    func declineFile_removesFromQueue() {
+        let manager = PendingFileManager(config: .default)
+        defer { manager.clearAll() }
+
+        let content = Data(repeating: 0x42, count: 256)
+        let pending = manager.addPendingFile(
+            senderPeerID: PeerID(str: "AABBCCDD11223344"),
+            senderNickname: "TestUser",
+            fileName: "test.bin",
+            mimeType: "application/octet-stream",
+            content: content,
+            isPrivate: false
+        )
+
+        guard let id = pending?.id else {
+            Issue.record("Failed to add pending file")
+            return
+        }
+
+        #expect(manager.stats.count == 1)
+        manager.declineFile(id: id)
+        #expect(manager.stats.count == 0)
+        #expect(manager.getPendingFile(id: id) == nil)
+    }
+
+    @Test("clearAll removes all pending files")
+    func clearAll_removesAllFiles() {
+        let manager = PendingFileManager(config: .default)
+
+        for i in 0..<5 {
+            _ = manager.addPendingFile(
+                senderPeerID: PeerID(str: "AABBCCDD1122334\(i)"),
+                senderNickname: "User\(i)",
+                fileName: "file\(i).bin",
+                mimeType: "application/octet-stream",
+                content: Data(repeating: UInt8(i), count: 100),
+                isPrivate: false
+            )
+        }
+
+        #expect(manager.stats.count == 5)
+        manager.clearAll()
+        #expect(manager.stats.count == 0)
+    }
+
+    @Test("count limit evicts oldest files")
+    func countLimit_evictsOldestFiles() {
+        let config = PendingFileManager.Config(
+            maxPendingCount: 3,
+            maxTotalBytes: 1_000_000,
+            expirationSeconds: 300
+        )
+        let manager = PendingFileManager(config: config)
+        defer { manager.clearAll() }
+
+        // Add 3 files (at limit)
+        var ids: [String] = []
+        for i in 0..<3 {
+            if let pending = manager.addPendingFile(
+                senderPeerID: PeerID(str: "AABBCCDD1122334\(i)"),
+                senderNickname: "User\(i)",
+                fileName: "file\(i).bin",
+                mimeType: "application/octet-stream",
+                content: Data(repeating: UInt8(i), count: 100),
+                isPrivate: false
+            ) {
+                ids.append(pending.id)
+            }
+        }
+
+        #expect(manager.stats.count == 3)
+
+        // Add 4th file - should evict oldest
+        let fourth = manager.addPendingFile(
+            senderPeerID: PeerID(str: "AABBCCDD11223349"),
+            senderNickname: "User9",
+            fileName: "file9.bin",
+            mimeType: "application/octet-stream",
+            content: Data(repeating: 0x99, count: 100),
+            isPrivate: false
+        )
+
+        #expect(fourth != nil)
+        #expect(manager.stats.count == 3) // Still at limit
+        #expect(manager.getPendingFile(id: ids[0]) == nil) // Oldest evicted
+        #expect(manager.getPendingFile(id: ids[1]) != nil) // Second still exists
+    }
+
+    @Test("size limit evicts files to make room")
+    func sizeLimit_evictsFilesToMakeRoom() {
+        let config = PendingFileManager.Config(
+            maxPendingCount: 100,
+            maxTotalBytes: 500, // Very small limit for testing
+            expirationSeconds: 300
+        )
+        let manager = PendingFileManager(config: config)
+        defer { manager.clearAll() }
+
+        // Add 2 files totaling 400 bytes (under limit)
+        let first = manager.addPendingFile(
+            senderPeerID: PeerID(str: "AABBCCDD11223340"),
+            senderNickname: "User0",
+            fileName: "file0.bin",
+            mimeType: "application/octet-stream",
+            content: Data(repeating: 0x00, count: 200),
+            isPrivate: false
+        )
+
+        _ = manager.addPendingFile(
+            senderPeerID: PeerID(str: "AABBCCDD11223341"),
+            senderNickname: "User1",
+            fileName: "file1.bin",
+            mimeType: "application/octet-stream",
+            content: Data(repeating: 0x01, count: 200),
+            isPrivate: false
+        )
+
+        #expect(manager.stats.count == 2)
+        #expect(manager.stats.totalBytes == 400)
+
+        // Add 300-byte file - needs to evict to fit
+        let third = manager.addPendingFile(
+            senderPeerID: PeerID(str: "AABBCCDD11223342"),
+            senderNickname: "User2",
+            fileName: "file2.bin",
+            mimeType: "application/octet-stream",
+            content: Data(repeating: 0x02, count: 300),
+            isPrivate: false
+        )
+
+        #expect(third != nil)
+        // First file should be evicted to make room
+        #expect(manager.getPendingFile(id: first!.id) == nil)
+        #expect(manager.stats.totalBytes <= 500)
+    }
+
+    @Test("acceptFile saves and removes from queue")
+    func acceptFile_savesAndRemovesFromQueue() {
+        let manager = PendingFileManager(config: .default)
+        defer { manager.clearAll() }
+
+        let content = Data(repeating: 0x42, count: 128)
+        let pending = manager.addPendingFile(
+            senderPeerID: PeerID(str: "AABBCCDD11223344"),
+            senderNickname: "TestUser",
+            fileName: "test.bin",
+            mimeType: "application/octet-stream",
+            content: content,
+            isPrivate: false
+        )
+
+        guard let id = pending?.id else {
+            Issue.record("Failed to add pending file")
+            return
+        }
+
+        #expect(manager.stats.count == 1)
+
+        var savedURL: URL?
+        let resultURL = manager.acceptFile(id: id) { pending in
+            // Simulate saving - just return a fake URL for testing
+            savedURL = URL(fileURLWithPath: "/tmp/test-\(pending.id).bin")
+            return savedURL
+        }
+
+        #expect(resultURL == savedURL)
+        #expect(manager.stats.count == 0) // Removed from queue
+        #expect(manager.getPendingFile(id: id) == nil)
+    }
+
+    @Test("getAllPendingFiles returns sorted by timestamp descending")
+    func getAllPendingFiles_sortedByTimestampDescending() async throws {
+        let manager = PendingFileManager(config: .default)
+        defer { manager.clearAll() }
+
+        // Add files with small delays to ensure different timestamps
+        for i in 0..<3 {
+            _ = manager.addPendingFile(
+                senderPeerID: PeerID(str: "AABBCCDD1122334\(i)"),
+                senderNickname: "User\(i)",
+                fileName: "file\(i).bin",
+                mimeType: "application/octet-stream",
+                content: Data(repeating: UInt8(i), count: 50),
+                isPrivate: false
+            )
+            try await Task.sleep(for: .milliseconds(10))
+        }
+
+        let all = manager.getAllPendingFiles()
+        #expect(all.count == 3)
+
+        // Should be sorted newest first
+        for i in 0..<(all.count - 1) {
+            #expect(all[i].timestamp >= all[i + 1].timestamp)
+        }
+    }
+
+    @Test("displayName returns fileName or generates default")
+    func displayName_returnsFileNameOrGeneratesDefault() {
+        let manager = PendingFileManager(config: .default)
+        defer { manager.clearAll() }
+
+        // With fileName
+        let withName = manager.addPendingFile(
+            senderPeerID: PeerID(str: "AABBCCDD11223344"),
+            senderNickname: "TestUser",
+            fileName: "custom.png",
+            mimeType: "image/png",
+            content: Data(repeating: 0x42, count: 100),
+            isPrivate: false
+        )
+        #expect(withName?.displayName == "custom.png")
+
+        // Without fileName - should use extension from MIME
+        let withoutName = manager.addPendingFile(
+            senderPeerID: PeerID(str: "AABBCCDD11223345"),
+            senderNickname: "TestUser2",
+            fileName: nil,
+            mimeType: "audio/mp3",
+            content: Data(repeating: 0x43, count: 100),
+            isPrivate: false
+        )
+        #expect(withoutName?.displayName == "file.mp3")
+    }
+}