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
↓ peach-android-6a51c07b.tar.gzatlas at this save
files changed6 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 −0app/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 }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 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 }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 aapp/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+}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+}