kv-sync: saturate seq increments instead of overflowing at u64::MAX
next_version() and the two merge paths in the engine computed max(seq) + 1 on seq values read from decrypted plaintext. A version carrying seq = u64::MAX panicked in debug builds and wrapped to 0 in release, so the next save looked like a replay to every other device. Found while checking fuzz/item_replay; needs an authentic plaintext, so low severity. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
@@ -379,7 +379,7 @@ impl SyncContext<'_> {
|
|||||||
open_record(acct, vault, vk, &local.record)?.ok_or(SyncError::Protocol("empty item".into()))?;
|
open_record(acct, vault, vk, &local.record)?.ok_or(SyncError::Protocol("empty item".into()))?;
|
||||||
let tomb =
|
let tomb =
|
||||||
open_sealed(acct, vault, vk, remote)?.ok_or(SyncError::Protocol("empty tombstone".into()))?;
|
open_sealed(acct, vault, vk, remote)?.ok_or(SyncError::Protocol("empty tombstone".into()))?;
|
||||||
let seq = replay::seq_of(&mine).max(replay::seq_of(&tomb)) + 1;
|
let seq = replay::seq_of(&mine).max(replay::seq_of(&tomb)).saturating_add(1);
|
||||||
replay::stamp(&mut mine, seq);
|
replay::stamp(&mut mine, seq);
|
||||||
let sealed = seal_record(acct, vault, vk, id, &mine, self.device_id)?;
|
let sealed = seal_record(acct, vault, vk, id, &mine, self.device_id)?;
|
||||||
self.store.put(vault, &rebased(sealed))?;
|
self.store.put(vault, &rebased(sealed))?;
|
||||||
@@ -408,7 +408,7 @@ impl SyncContext<'_> {
|
|||||||
if replay::same_content(&out.merged, &theirs) {
|
if replay::same_content(&out.merged, &theirs) {
|
||||||
self.store.put_remote(vault, remote)?;
|
self.store.put_remote(vault, remote)?;
|
||||||
} else {
|
} else {
|
||||||
let seq = replay::seq_of(&mine).max(replay::seq_of(&theirs)) + 1;
|
let seq = replay::seq_of(&mine).max(replay::seq_of(&theirs)).saturating_add(1);
|
||||||
replay::stamp(&mut out.merged, seq);
|
replay::stamp(&mut out.merged, seq);
|
||||||
let sealed = seal_record(acct, vault, vk, id, &out.merged, self.device_id)?;
|
let sealed = seal_record(acct, vault, vk, id, &out.merged, self.device_id)?;
|
||||||
self.store.put(vault, &rebased(sealed))?;
|
self.store.put(vault, &rebased(sealed))?;
|
||||||
|
|||||||
@@ -113,7 +113,8 @@ pub fn next_version(local: Option<&Value>, base: Base<'_>) -> u64 {
|
|||||||
Base::Sealed(v) => seq_of(v),
|
Base::Sealed(v) => seq_of(v),
|
||||||
_ => 0,
|
_ => 0,
|
||||||
};
|
};
|
||||||
local.map_or(0, seq_of).max(held) + 1
|
// Saturates: `seq` comes from decrypted plaintext, and a version at u64::MAX must not panic or wrap to 0.
|
||||||
|
local.map_or(0, seq_of).max(held).saturating_add(1)
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
@@ -122,6 +123,14 @@ mod tests {
|
|||||||
|
|
||||||
use super::*;
|
use super::*;
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn next_version_saturates_at_u64_max() {
|
||||||
|
// A plaintext carrying seq = u64::MAX used to overflow (panic in debug, wrap to 0 in release).
|
||||||
|
let max = json!({"title": "a", "seq": u64::MAX});
|
||||||
|
assert_eq!(next_version(Some(&max), Base::None), u64::MAX);
|
||||||
|
assert_eq!(next_version(None, Base::Sealed(&max)), u64::MAX);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn stamp_and_same_content() {
|
fn stamp_and_same_content() {
|
||||||
let mut v = json!({"title": "a"});
|
let mut v = json!({"title": "a"});
|
||||||
|
|||||||
Reference in New Issue
Block a user