2ce28477fa2c
clamp remote entry and tombstone timestamps before merging, closing the HV-1 pinning gap the browser had already fixed (v2.4.5)
- time
- 2026-07-26 08:32 UTC · 3 months ago
- machine
- koh2
- agent
- claude-opus-5
- session
- sync campaign remaining items
- parent
- f4ade7ac52cc
↓ peach-android-2ce28477.tar.gzatlas at this save
files changed5 files
VERSION+1 −1app/build.gradle+2 −2app/src/main/java/com/peachpasswords/sync/SyncLimits.kt+25 −0app/src/main/java/com/peachpasswords/vault/VaultRepository.kt+19 −2app/src/test/java/com/peachpasswords/sync/ChirpSyncSuiteTest.kt+27 −0app/build.gradle+2 −2
@@ -16,8 +16,8 @@1616 applicationId "com.peachpasswords"1717 minSdk 261818 targetSdk 3519- versionCode 24000420- versionName "2.4.4"19+ versionCode 24000520+ versionName "2.4.5"2121 2222 testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner"2323 vectorDrawables {app/src/main/java/com/peachpasswords/sync/SyncLimits.kt+25 −0
@@ -17,3 +17,28 @@1717 throw SyncFrameTooLargeException("sync frame exceeds $MAX_SYNC_FRAME_BYTES bytes")1818 }1919 }20+21+/**22+ * Timestamp bounds, ported from peach-browser `src/shared/sync-limits.ts`.23+ * Both sides must agree, so change them together.24+ */25+object SyncLimits {26+ /**27+ * HV-1: maximum tolerated forward clock skew on any timestamp received from28+ * a sync peer. A peer whose clock is slightly ahead still merges cleanly,29+ * but a deliberately far-future `modifiedAt`/`deletedAt` can no longer30+ * permanently pin a malicious value against honest local edits.31+ */32+ const val MAX_CLOCK_SKEW_MS = 5 * 60 * 1000L33+34+ /**35+ * Clamp a peer-supplied timestamp to `now + MAX_CLOCK_SKEW_MS`. Applied to36+ * every remote `modifiedAt` and `deletedAt` BEFORE it is compared against37+ * any local timestamp.38+ *39+ * See the HOLE 2 coverage audit in the browser's `sync-limits.ts` for the40+ * full list of peer-supplied timestamps and what bounds each one.41+ */42+ fun clampSyncTimestamp(ts: Long, now: Long = System.currentTimeMillis()): Long =43+ minOf(ts, now + MAX_CLOCK_SKEW_MS)44+}app/src/main/java/com/peachpasswords/vault/VaultRepository.kt+19 −2
@@ -10,6 +10,7 @@1010 import kotlinx.serialization.json.Json1111 import android.util.Log1212 import com.peachpasswords.BuildConfig13+import com.peachpasswords.sync.SyncLimits1314 import kotlinx.serialization.json.JsonElement1415 import kotlinx.serialization.json.JsonNull1516 import kotlinx.serialization.json.JsonObject@@ -913,11 +914,27 @@913914 remoteTombstones: List<Tombstone>,914915 ): Int = mutex.withLock {915916 requireUnlocked()916- val tombMap = remoteTombstones.associate { it.id to it.deletedAt }917+ // HV-1: clamp every peer-supplied timestamp to now + MAX_CLOCK_SKEW_MS918+ // BEFORE any comparison against a local one. Without this a peer can919+ // send a far-future modifiedAt and permanently pin its version of an920+ // entry against every honest local edit. The browser clamps in the921+ // matching place (vault.ts applyMergedRemoteEntries).922+ val mergeNow = System.currentTimeMillis()923+ val clamped = remoteEntries.map { entry ->924+ entry.copy(925+ modifiedAt = SyncLimits.clampSyncTimestamp(entry.modifiedAt, mergeNow),926+ deletedAt = entry.deletedAt?.let {927+ SyncLimits.clampSyncTimestamp(it, mergeNow)928+ },929+ )930+ }931+ val tombMap = remoteTombstones.associate {932+ it.id to SyncLimits.clampSyncTimestamp(it.deletedAt, mergeNow)933+ }917934 var applied = 0918935 val byId = entryList.associateBy { it.id }.toMutableMap()919936 920- for (remote in remoteEntries) {937+ for (remote in clamped) {921938 val tomb = tombMap[remote.id]922939 if (tomb != null && tomb >= remote.modifiedAt) continue923940 val local = byId[remote.id]app/src/test/java/com/peachpasswords/sync/ChirpSyncSuiteTest.kt+27 −0
@@ -117,6 +117,33 @@117117 assertTrue(threw)118118 }119119 120+ @Test fun `HV-1 a far-future remote modifiedAt is clamped into the skew window`() = runBlocking {121+ val (a, _, _) = pairedDevices()122+ val local = a.addEntry(123+ VaultEntry(id = "", title = "Local", type = "login", password = "local-pw", createdAt = 0, modifiedAt = 0),124+ )125+126+ // A hostile peer claims an edit ten years in the future, trying to pin127+ // its version against every honest local edit for a decade.128+ val farFuture = System.currentTimeMillis() + 10L * 365 * 24 * 60 * 60 * 1000129+ a.applyMergedRemoteEntries(130+ listOf(local.copy(title = "Injected", password = "attacker-pw", modifiedAt = farFuture)),131+ emptyList(),132+ )133+134+ // The edit still lands (it is genuinely newer) but only within the skew135+ // window, so the monotonic local entry clock can always advance past it.136+ // The overtake itself is not exercised here: updateEntry would push137+ // VaultCrypto's process-global monotonic counter forward and skew every138+ // later test in the same JVM.139+ val pinned = a.getEntry(local.id)!!140+ assertTrue(141+ "remote modifiedAt must be clamped into the skew window",142+ pinned.modifiedAt <= System.currentTimeMillis() + SyncLimits.MAX_CLOCK_SKEW_MS,143+ )144+ assertTrue("clamped value must be far below the claim", pinned.modifiedAt < farFuture)145+ }146+120147 @Test fun sas_gate_never_completes_without_explicit_user_confirmation() = runBlocking {121148 val session = PairingSession()122149 val waiter = async { session.await() }