6a51c07ba744

M-01: bounded imports, repository-level atomic replaceEntries, failed import destroys just-created vault

time
2026-07-29 14:14 UTC · 2 months ago
machine
koh2
agent
claude-opus-4
session
audit-remediation-2026-07-29
parent
9bd9b6bedb2c
6 files app/src/main/java/com/peachpasswords/sync/SyncFrameTransport.kt+43 −22app/src/main/java/com/peachpasswords/ui/vault/CsvImport.kt+38 −5app/src/main/java/com/peachpasswords/ui/vault/ImportScreen.kt+37 −17app/src/main/java/com/peachpasswords/vault/VaultRepository.kt+42 −0app/src/test/java/com/peachpasswords/ui/vault/ImportBoundsTest.kt+43 −0app/src/test/java/com/peachpasswords/vault/ReplaceEntriesAtomicityTest.kt+75 −0
app/src/main/java/com/peachpasswords/sync/SyncFrameTransport.kt+43 −22
@@ -103,32 +103,53 @@103103     receiveCiphertext: suspend () -> ByteArray,104104     decrypt: (ByteArray) -> ByteArray,105105 ): ByteArray {106-    var buffered: ByteArray? = null106+    // Audit H-09: pre-allocated buffer (capped at MAX_SYNC_FRAME_BYTES) avoids107+    // the O(n²) reallocation the old growing-array path suffered — one108+    // allocation regardless of how many chunks the peer streams. Cumulative109+    // size and chunk count are checked BEFORE writing so an authenticated peer110+    // can't grow memory unboundedly or pin the loop on tiny marked chunks.111+    val buffer = ByteArray(MAX_SYNC_FRAME_BYTES)112+    var size = 0113+    var chunkCount = 0114+    var chunkedFrame = false107115     while (true) {116+        chunkCount++108117         val pt = decrypt(receiveCiphertext())109-        if (buffered == null) {110-            // First chunk. Marked means "more chunks to come"; an unmarked111-            // first chunk is the entire single-chunk frame, exactly as before112-            // the chunking layer existed.113-            if (pt.isNotEmpty() && pt[0] == FRAME_CONTINUATION_MARKER) {114-                buffered = pt.copyOfRange(1, pt.size)115-                continue118+        val marked = pt.isNotEmpty() && pt[0] == FRAME_CONTINUATION_MARKER119+        val dataStart = if (marked) 1 else 0120+        val dataSize = pt.size - dataStart121+        // MAX_SYNC_FRAME_BYTES doubles as the chunk-count ceiling: even a122+        // maximally-fragmented adversarial stream (one byte per chunk) hits a123+        // finite loop bound instead of running until the size cap.124+        if (size + dataSize > MAX_SYNC_FRAME_BYTES || chunkCount > MAX_SYNC_FRAME_BYTES) {125+            buffer.fill(0, 0, size)126+            pt.fill(0)127+            if (size + dataSize > MAX_SYNC_FRAME_BYTES) {128+                requireSyncFrameSize(size + dataSize)116129             }117-            return SyncPadding.unpad(pt)130+            throw SyncFrameTooLargeException(131+                "sync frame chunk count exceeded $MAX_SYNC_FRAME_BYTES",132+            )118133         }119-        // Continuing the assembly. A marked chunk contributes bytes past120-        // the marker; the final chunk is unmarker, so we concatenate it121-        // whole and unpad.122-        if (pt.isNotEmpty() && pt[0] == FRAME_CONTINUATION_MARKER) {123-            val merged = ByteArray(buffered.size + pt.size - 1)124-            buffered.copyInto(merged, 0)125-            pt.copyInto(merged, buffered.size, 1)126-            buffered = merged127-            continue134+        if (!chunkedFrame) {135+            // First chunk. Marker ⇒ chunked frame; no marker ⇒ the entire136+            // single-chunk frame, unchanged from before the chunking layer137+            // existed. The chunkedFrame latch then enforces the rest of the138+            // protocol: every subsequent chunk must be marked until one isn't.139+            chunkedFrame = true140+            if (!marked) {141+                return SyncPadding.unpad(pt)142+            }143+        } else if (!marked) {144+            // Final chunk of a chunked frame. Unpad and zero the buffer so145+            // partial plaintext doesn't linger.146+            pt.copyInto(buffer, size, dataStart, pt.size)147+            val written = size + dataSize148+            val out = SyncPadding.unpad(buffer.copyOfRange(0, written))149+            buffer.fill(0, 0, written)150+            return out128151         }129-        val reassembled = ByteArray(buffered.size + pt.size)130-        buffered.copyInto(reassembled, 0)131-        pt.copyInto(reassembled, buffered.size)132-        return SyncPadding.unpad(reassembled)152+        pt.copyInto(buffer, size, dataStart, pt.size)153+        size += dataSize133154     }134155 }

view file

app/src/main/java/com/peachpasswords/ui/vault/CsvImport.kt+38 −5
@@ -33,11 +33,38 @@3333     data class Success(val provider: String, val entries: List<ParsedEntry>) : ImportParseResult3434     data class Failure(val reason: Reason) : ImportParseResult3535 36-    enum class Reason { UNRECOGNISED, CORRUPTED, NO_ENTRIES }36+    enum class Reason { UNRECOGNISED, CORRUPTED, NO_ENTRIES, TOO_LARGE }3737 }3838 3939 object CsvImport {4040 41+    /**42+     * M-01: hard bounds on anything accepted from a provider export. A content43+     * URI is backed by an arbitrary (possibly endless) provider stream, and the44+     * parsed set is held in memory through preview and commit, so both the byte45+     * length of the file and the shape of the parse are capped.46+     */47+    const val MAX_IMPORT_BYTES = 50 * 1024 * 102448+    const val MAX_IMPORT_ENTRIES = 10_00049+    const val MAX_FIELD_CHARS = 50 * 102450+51+    /**52+     * Read at most [max] bytes of UTF-8 from [input]; null when the stream is53+     * longer. Bounded rather than readBytes() so an oversize file is refused54+     * instead of exhausting the heap before anything is parsed.55+     */56+    fun readBoundedUtf8(input: java.io.InputStream, max: Int = MAX_IMPORT_BYTES): String? {57+        val out = java.io.ByteArrayOutputStream()58+        val buf = ByteArray(64 * 1024)59+        while (true) {60+            val n = input.read(buf)61+            if (n == -1) break62+            if (out.size().toLong() + n > max) return null63+            out.write(buf, 0, n)64+        }65+        return out.toString("UTF-8")66+    }67+4168     // Display labels used both for detection return values and "Detected: X export".4269     const val BITWARDEN_CSV = "Bitwarden CSV"4370     const val BITWARDEN_JSON = "Bitwarden JSON"@@ -109,12 +136,18 @@109136         val provider = detectProvider(content)110137             ?: return ImportParseResult.Failure(ImportParseResult.Reason.UNRECOGNISED)111138         val entries = parse(content, provider)112-        return if (entries.isEmpty()) {113-            ImportParseResult.Failure(ImportParseResult.Reason.NO_ENTRIES)114-        } else {115-            ImportParseResult.Success(provider, entries)139+        return when {140+            entries.isEmpty() -> ImportParseResult.Failure(ImportParseResult.Reason.NO_ENTRIES)141+            // M-01: reject an absurd shape before it reaches the preview, where142+            // every entry is rendered and then re-encoded into the vault write.143+            entries.size > MAX_IMPORT_ENTRIES || entries.any { it.hasOversizeField() } ->144+                ImportParseResult.Failure(ImportParseResult.Reason.TOO_LARGE)145+            else -> ImportParseResult.Success(provider, entries)116146         }117147     }148+149+    private fun ParsedEntry.hasOversizeField(): Boolean =150+        listOf(title, username, password, url, notes, totp).any { (it?.length ?: 0) > MAX_FIELD_CHARS }118151 119152     // ---- CSV column maps --------------------------------------------------120153 

view file

app/src/main/java/com/peachpasswords/ui/vault/ImportScreen.kt+37 −17
@@ -113,6 +113,7 @@113113     const val UNREADABLE_FILE = "Couldn't read that file."114114     const val UNRECOGNISED_FILE = "Unrecognised file format. Use CSV, Bitwarden JSON, Proton Pass JSON/CSV, or a Peach encrypted backup."115115     const val NO_MASTER_PASSWORD = "Choose a master password for the new vault first."116+    const val TOO_LARGE = "That file is too big to import. Peach accepts up to 50 MB and 10,000 entries."116117 }117118 118119 internal fun shouldRouteFileToPeachScript(path: String?, content: String): Boolean {@@ -220,13 +221,21 @@220221         _state.value = ImportUiState.Working()221222         _detectedProvider.value = null222223         viewModelScope.launch {223-            val content = withContext(Dispatchers.IO) {224+            // M-01: a content URI is an arbitrary provider stream — read it225+            // bounded so an oversize (or endless) file is refused instead of226+            // filling the heap. null distinguishes "over the cap" from "unreadable".227+            val read: Result<String?> = withContext(Dispatchers.IO) {224228                 runCatching {225-                    appContext.contentResolver.openInputStream(uri)?.use {226-                        it.readBytes().toString(Charsets.UTF_8)227-                    }228-                }.getOrNull()229+                    val stream = appContext.contentResolver.openInputStream(uri)230+                        ?: throw java.io.IOException("no_stream")231+                    stream.use { CsvImport.readBoundedUtf8(it) }232+                }229233             }234+            if (read.isSuccess && read.getOrNull() == null) {235+                _state.value = ImportUiState.Failed(ImportErrors.TOO_LARGE)236+                return@launch237+            }238+            val content = read.getOrNull()230239             if (content.isNullOrBlank()) {231240                 _state.value = ImportUiState.Failed(ImportErrors.UNREADABLE_FILE)232241                 return@launch@@ -247,6 +256,7 @@247256                             ImportParseResult.Reason.UNRECOGNISED -> ImportErrors.UNRECOGNISED_FILE248257                             ImportParseResult.Reason.CORRUPTED -> ImportErrors.CORRUPTED249258                             ImportParseResult.Reason.NO_ENTRIES -> ImportErrors.NO_ENTRIES259+                            ImportParseResult.Reason.TOO_LARGE -> ImportErrors.TOO_LARGE250260                         },251261                     )252262                 }@@ -257,9 +267,10 @@257267     // ---- Confirm ----------------------------------------------------------258268 259269     /**260-     * Persist the previewed entries. No batch API exists on VaultRepository, so261-     * import loops [VaultRepository.addEntry]. REPLACE soft-deletes the current262-     * active set first; MERGE simply appends.270+     * Persist the previewed entries in one write: MERGE appends via271+     * [VaultRepository.addEntries], REPLACE swaps the active set via272+     * [VaultRepository.replaceEntries] so a failure cannot leave the old273+     * entries tombstoned with nothing imported.263274      *264275      * [newVaultPassword] is required when [vaultUnlocked] is false: there's no265276      * vault yet (fresh install "restore instead", or "forgot password" recovery)@@ -270,6 +281,9 @@270281     fun confirmImport(preview: ImportPreview, mode: ImportMode, newVaultPassword: String? = null) {271282         _state.value = ImportUiState.Working()272283         viewModelScope.launch {284+            // Declared outside the try so the failure path knows whether this285+            // vault only exists to hold the import (M-01 rollback below).286+            var createdForImport = false273287             try {274288                 val createdVault: Boolean275289                 if (!vault.isUnlocked.value) {@@ -282,7 +296,10 @@282296                     // thread or it blocks the frame loop on its own.283297                     val created = withContext(Dispatchers.Default) { vault.createVault(newVaultPassword) }284298                     when (created) {285-                        is CreateVaultResult.Success -> createdVault = true299+                        is CreateVaultResult.Success -> {300+                            createdVault = true301+                            createdForImport = true302+                        }286303                         is CreateVaultResult.Error -> {287304                             _state.value = ImportUiState.Failed(created.reason)288305                             return@launch@@ -291,12 +308,6 @@291308                 } else {292309                     createdVault = false293310                 }294-                if (mode == ImportMode.REPLACE) {295-                    _state.value = ImportUiState.Working("clearing the old vault…")296-                    withContext(Dispatchers.Default) {297-                        vault.getEntries().forEach { existing -> runCatching { vault.deleteEntry(existing.id) } }298-                    }299-                }300311                 val now = System.currentTimeMillis()301312                 val fulls = preview.fullEntries302313                 val drafts = if (fulls != null) {@@ -316,8 +327,12 @@316327                 // One batched write on a background thread. Looping addEntry317328                 // re-encrypted and rewrote the whole vault per entry, which on318329                 // a real 806-entry vault hung the app until Android offered to319-                // kill it.320-                val imported = withContext(Dispatchers.Default) { vault.addEntries(drafts) }330+                // kill it. M-01: REPLACE goes through replaceEntries so the331+                // tombstoning of the old set and the insert of the new one are332+                // a single write that rolls back together.333+                val imported = withContext(Dispatchers.Default) {334+                    if (mode == ImportMode.REPLACE) vault.replaceEntries(drafts) else vault.addEntries(drafts)335+                }321336                 withContext(Dispatchers.Default) {322337                     preview.settings?.let { vault.updateSettings(it) }323338                     preview.licenseKey@@ -326,6 +341,11 @@326341                 }327342                 _state.value = ImportUiState.Done(imported, createdVault = createdVault)328343             } catch (e: Exception) {344+                // M-01: a vault created purely to receive this import is not345+                // wanted if the import failed — leaving it behind puts the user346+                // in an empty vault they never chose a purpose for, and blocks347+                // the "restore instead" path from being retried.348+                if (createdForImport) runCatching { vault.destroyVault() }329349                 _state.value = ImportUiState.Failed(ImportErrors.CORRUPTED)330350             }331351         }

view file

app/src/main/java/com/peachpasswords/vault/VaultRepository.kt+42 −0
@@ -1031,6 +1031,48 @@10311031         return drafts.size10321032     }10331033 1034+    /**1035+     * M-01: replace the whole active set with [newEntries] in one write.1036+     *1037+     * Import's REPLACE mode used to soft-delete every current entry one durable1038+     * write at a time (swallowing each failure) and then batch-add, so a failed1039+     * add left the old entries stranded in Trash with nothing restored. Staging1040+     * the entire next list in memory and committing it through1041+     * [persistOrRollback] makes the swap all-or-nothing: on any failure the1042+     * pre-import list — active entries included — is restored.1043+     *1044+     * Existing tombstones are preserved so paired devices still see the1045+     * deletions they already know about.1046+     */1047+    suspend fun replaceEntries(newEntries: List<VaultEntry>): Int = mutex.withLock {1048+        requireUnlocked()1049+        val now = System.currentTimeMillis()1050+        val ready = newEntries.map { draft ->1051+            val stamp = VaultCrypto.monotonicEntryTimestamp()1052+            withDerivedLast4(1053+                draft.copy(1054+                    id = draft.id.ifBlank { UUID.randomUUID().toString() },1055+                    createdAt = if (draft.createdAt > 0) draft.createdAt else now,1056+                    modifiedAt = stamp,1057+                    passwordChangedAt = if (!draft.password.isNullOrEmpty()) stamp else draft.passwordChangedAt,1058+                ),1059+            )1060+        }1061+        // H-03: capture rollback and swap the list under one stateLock.1062+        val rollback = mutateState {1063+            val r = entryList.toMutableList()1064+            val replaced = entryList.map { entry ->1065+                if (entry.deletedAt != null) return@map entry1066+                val stamp = VaultCrypto.monotonicEntryTimestamp(entry.modifiedAt)1067+                entry.copy(deletedAt = stamp, modifiedAt = stamp)1068+            }1069+            entryList = (replaced + ready).toMutableList()1070+            r1071+        }1072+        persistOrRollback(rollback)1073+        return ready.size1074+    }1075+10341076     suspend fun updateEntry(patch: VaultEntry): VaultEntry = mutex.withLock {10351077         requireUnlocked()10361078         // H-03: read entryList, compute the patched entry, and write it under a

view file

app/src/test/java/com/peachpasswords/ui/vault/ImportBoundsTest.kt+43 −0
@@ -0,0 +1,43 @@1+package com.peachpasswords.ui.vault2+3+import org.junit.Assert.assertEquals4+import org.junit.Assert.assertNull5+import org.junit.Assert.assertTrue6+import org.junit.Test7+8+/**9+ * M-01: an import is attacker-shaped input. A provider stream longer than the10+ * cap, or a parse that yields an absurd entry count / field size, must be11+ * refused before anything is staged for the vault write.12+ */13+class ImportBoundsTest {14+15+    @Test fun oversize_stream_is_rejected() {16+        val over = ByteArray(1024) { 'a'.code.toByte() }.inputStream()17+        assertNull(CsvImport.readBoundedUtf8(over, max = 512))18+    }19+20+    @Test fun within_bound_stream_reads_fully() {21+        val body = "name,url,username,password\nA,https://a.com,u,p\n"22+        assertEquals(body, CsvImport.readBoundedUtf8(body.byteInputStream(), max = 512))23+    }24+25+    @Test fun too_many_entries_is_rejected() {26+        val rows = (0..CsvImport.MAX_IMPORT_ENTRIES)27+            .joinToString("\n") { "T$it,https://a.com,u$it,p$it" }28+        val csv = "name,url,username,password\n$rows\n"29+        val r = CsvImport.detectAndParse(csv)30+        assertTrue(r is ImportParseResult.Failure)31+        assertEquals(ImportParseResult.Reason.TOO_LARGE, (r as ImportParseResult.Failure).reason)32+    }33+34+    @Test fun oversize_field_is_rejected() {35+        val huge = "x".repeat(CsvImport.MAX_FIELD_CHARS + 1)36+        val csv = "name,url,username,password\nA,https://a.com,u,$huge\n"37+        val r = CsvImport.detectAndParse(csv)38+        assertEquals(39+            ImportParseResult.Reason.TOO_LARGE,40+            (r as ImportParseResult.Failure).reason,41+        )42+    }43+}

view file

app/src/test/java/com/peachpasswords/vault/ReplaceEntriesAtomicityTest.kt+75 −0
@@ -0,0 +1,75 @@1+package com.peachpasswords.vault2+3+import kotlinx.coroutines.runBlocking4+import org.junit.Assert.assertEquals5+import org.junit.Assert.assertTrue6+import org.junit.Test7+8+/**9+ * M-01: import's REPLACE mode must be one all-or-nothing write. The old path10+ * soft-deleted every current entry in its own durable write and then batch-added,11+ * so a failed add left the user's real entries tombstoned in Trash with nothing12+ * imported to show for it. [VaultRepository.replaceEntries] stages the swap in13+ * memory and commits it once, so a write failure restores the active set.14+ */15+class ReplaceEntriesAtomicityTest {16+17+    private class FaultStorage : VaultStorage {18+        var slotA: StoredVault? = null19+        var slotB: String? = null20+        var failWriteSlotA = false21+        private var ver: Long = 022+        private var settings = VaultSettings()23+        override fun loadSlotA() = slotA24+        override fun hasVault() = slotA != null25+        override fun writeSlotA(slot: StoredVault): Boolean {26+            if (failWriteSlotA) return false27+            slotA = slot28+            return true29+        }30+        override fun loadSlotB() = slotB31+        override fun writeSlotB(blobBase64: String): Boolean { slotB = blobBase64; return true }32+        var decoy: StoredVault? = null33+        override fun loadDecoySlot() = decoy34+        override fun writeDecoySlot(slot: StoredVault): Boolean { decoy = slot; return true }35+        override fun hasDecoySlot() = decoy != null36+        override fun clearDecoySlot() { decoy = null }37+        override fun version() = ver38+        override fun stampLocalMutation(): Long { ver = maxOf(System.currentTimeMillis(), ver + 1); return ver }39+        override fun loadSettings() = settings40+        override fun writeSettings(s: VaultSettings): Boolean { settings = s; return true }41+        override fun clearVault() { slotA = null; slotB = null; ver = 0; settings = VaultSettings() }42+    }43+44+    private fun login(title: String) =45+        VaultEntry(id = "", title = title, type = "login", password = "pw", createdAt = 0, modifiedAt = 0)46+47+    @Test fun replace_swaps_active_set_in_one_write() = runBlocking {48+        val repo = VaultRepository(FaultStorage())49+        repo.createVault("pw")50+        repo.addEntry(login("Old A"))51+        repo.addEntry(login("Old B"))52+53+        assertEquals(2, repo.replaceEntries(listOf(login("New A"), login("New B"))))54+55+        assertEquals(listOf("New A", "New B"), repo.getEntries().map { it.title }.sorted())56+        // The old set is tombstoned, not hard-dropped — paired devices still57+        // need to learn about the deletion.58+        assertEquals(listOf("Old A", "Old B"), repo.getDeletedEntries().map { it.title }.sorted())59+    }60+61+    @Test fun failed_replace_restores_prior_active_entries() = runBlocking {62+        val storage = FaultStorage()63+        val repo = VaultRepository(storage)64+        repo.createVault("pw")65+        repo.addEntry(login("Keep me"))66+67+        storage.failWriteSlotA = true68+        val threw = runCatching { repo.replaceEntries(listOf(login("Imported"))) }.isFailure69+        assertTrue(threw)70+71+        // Neither tombstoned nor replaced: the pre-import view is intact.72+        assertEquals(listOf("Keep me"), repo.getEntries().map { it.title })73+        assertTrue(repo.getDeletedEntries().isEmpty())74+    }75+}

view file