diff --git a/wacore/appstate/src/hash.rs b/wacore/appstate/src/hash.rs index c82718d8a..cf6e7b75f 100644 --- a/wacore/appstate/src/hash.rs +++ b/wacore/appstate/src/hash.rs @@ -58,15 +58,20 @@ impl HashState { .and_then(|idx| idx.blob.as_deref()) } let index_mode = mutations.iter().all(|m| index_mac_of(m).is_some()); - let mut removed_in_patch: std::collections::HashSet<&[u8]> = - std::collections::HashSet::new(); + // Membership set over REMOVE index_macs, which are HMAC outputs (uniformly + // random). A linear-scan Vec beats a SipHash HashSet at the patch sizes seen + // in practice — the same trade-off as detect_duplicate_index_in_patch and + // collect_unique_index_macs (#856). Only `.contains()` is queried below, so an + // unconditional push is membership-equivalent to the set (a malformed duplicate + // REMOVE is rejected by the duplicate-index guard regardless). + let mut removed_in_patch: Vec<&[u8]> = Vec::new(); if index_mode { for mutation in mutations { if mutation.operation.unwrap_or_default() == wa::syncd_mutation::SyncdOperation::Remove as i32 && let Some(index_mac) = index_mac_of(mutation) { - removed_in_patch.insert(index_mac); + removed_in_patch.push(index_mac); } } } @@ -88,7 +93,7 @@ impl HashState { added.push(&blob[blob.len() - 32..]); } if let Some(index_mac) = index_mac_of(mutation) { - if is_set && removed_in_patch.contains(index_mac) { + if is_set && removed_in_patch.contains(&index_mac) { continue; } match get_prev_set_value_mac(index_mac, i) { diff --git a/wacore/appstate/src/processor.rs b/wacore/appstate/src/processor.rs index 82591bdac..54af2b5f7 100644 --- a/wacore/appstate/src/processor.rs +++ b/wacore/appstate/src/processor.rs @@ -10,7 +10,7 @@ use crate::hash::{HashState, generate_patch_mac}; use crate::keys::ExpandedAppStateKeys; use log::{debug, trace}; use serde::{Deserialize, Serialize}; -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; use std::sync::Arc; use waproto::whatsapp as wa; @@ -311,8 +311,13 @@ where /// WA Web keys on the decrypted index; the raw index_mac blob is a deterministic /// function of that index, so keying on it is equivalent for detection. fn detect_duplicate_index_in_patch(mutations: &[wa::SyncdMutation]) -> Result<(), AppStateError> { - let mut seen_set: HashSet<&[u8]> = HashSet::new(); - let mut seen_remove: HashSet<&[u8]> = HashSet::new(); + // index_macs are HMAC outputs (uniformly random), so a HashSet only buys + // SipHash setup plus an allocation for no distribution benefit. A linear scan + // wins at the patch sizes seen in practice — the same trade-off measured for + // collect_unique_index_macs (#856). Set and Remove are deduped independently: + // a Set and a Remove may legitimately carry the same index within one patch. + let mut seen_set: Vec<&[u8]> = Vec::new(); + let mut seen_remove: Vec<&[u8]> = Vec::new(); for m in mutations { let Some(rec) = &m.record else { continue }; let Some(index_mac) = rec.index.as_ref().and_then(|i| i.blob.as_deref()) else { @@ -324,9 +329,10 @@ fn detect_duplicate_index_in_patch(mutations: &[wa::SyncdMutation]) -> Result<() wa::syncd_mutation::SyncdOperation::Set => &mut seen_set, wa::syncd_mutation::SyncdOperation::Remove => &mut seen_remove, }; - if !seen.insert(index_mac) { + if seen.contains(&index_mac) { return Err(AppStateError::DuplicateIndexInPatch); } + seen.push(index_mac); } Ok(()) }