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
5 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 −0
VERSION+1 −1
@@ -1 +1 @@1-2.4.41+2.4.5

view file

app/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 {

view file

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+}

view file

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]

view file

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() }

view file