BUG: Duplicate Detection Without Prevention

[devfire]

Severity: LOW

Location

src/state_actor.rs:329-336

Issue

Code detects duplicate key package announcements but processes them anyway.

if self.key_packages.contains_key(&composite_key) {
    debug!("Received duplicate announcement for: {} (overwriting)", composite_key);
} else {
    debug!("Caching new KeyPackage for: {}", composite_key);
}
self.key_packages.insert(composite_key, key_package_in); // Always inserts

Problems

1. Pointless check: Detection without action
2. Misleading logging: Says "overwriting" but that's not the issue
3. Missing validation: Should verify package hasn't changed
4. No metrics: Can't track duplicate rate

Why This Matters

• Duplicate announcements could indicate network issues
• Changed KeyPackages for same user could indicate attack
• Should track and alert on suspicious patterns

Solution

match self.key_packages.get(&composite_key) {
    Some(existing) => {
        // Compare packages - they should be identical
        if existing.hash() != key_package_in.hash() {
            warn!("KeyPackage changed for {}: possible attack", composite_key);
            // Decide: reject, or accept with warning
        } else {
            debug!("Duplicate announcement for {} (ignored)", composite_key);
            return Ok(StateActorReply::Success(
                "Duplicate announcement ignored".to_string()
            ));
        }
    }
    None => {
        debug!("Caching new KeyPackage for: {}", composite_key);
        self.key_packages.insert(composite_key, key_package_in);
    }
}

Benefits

• Detects KeyPackage rotation attacks
• Reduces unnecessary storage operations
• Better logging and metrics
• Security improvement

Labels

bug, security, code-quality