[RWF] sc-network-gossip: register_message_hashed duplicate check is ineffective

[xlc]

Summary

ConsensusGossip::register_message_hashed guards message insertion with if self.known_messages.insert(message_hash, ()), intending to only store a message when its hash is newly inserted. However, schnellru::LruMap::insert (0.2.3) does NOT distinguish between new insertions and value replacements — it returns !self.is_empty() at the end, which is true whenever the map is non-empty. When the ByLength limiter's on_replace returns true (which it always does), inserting an existing key replaces the value and returns true. As a result, the dedup guard in register_message_hashed always passes for a non-empty cache, and register_message / multicast will push duplicate MessageEntry records when called with the same message content. This causes duplicate entries visible via messages_for(), wasteful memory usage, and potential duplicate message processing. The on_incoming path is unaffected because it performs a prior known_messages.get() check before calling register_message_hashed.

Severity / Sensitivity

Low severity, non-sensitive public issue. This is reported from a confirmed Runtime Whitebox Fuzzer finding against public Polkadot SDK code.

Source Evidence

• substrate/client/network-gossip/src/state_machine.rs lines 215-229: register_message_hashed uses if self.known_messages.insert(message_hash, ()) as a dedup guard before pushing a MessageEntry. Because schnellru::LruMap::insert returns true for existing keys (it returns !self.is_empty()), this guard is always true for a non-empty cache and never prevents duplicates.
• substrate/client/network-gossip/src/state_machine.rs lines 236-239: register_message (public API) computes the hash and calls register_message_hashed directly without any prior known_messages.get() check, so the broken dedup guard is the only protection — and it fails.
• substrate/client/network-gossip/src/state_machine.rs lines 365-382: on_incoming performs dedup via self.known_messages.get(&message_hash).is_some() BEFORE calling register_message_hashed, confirming the developers intended dedup. This correct approach is missing from register_message and multicast, highlighting the inconsistency.

Full Relevant PoC Test

Paste the snippet below into an in-crate unit test module for the affected package. It is self-contained apart from helpers and types already available in that crate's existing test context.

use super::*;

use crate::state_machine::ConsensusGossip;
use sc_network_types::PeerId;
use sp_runtime::testing::{Block as RawBlock, MockCallU64, TestXt};
use sp_runtime::traits::Block as BlockT;
use std::sync::Arc;

type TestBlock = RawBlock<TestXt<MockCallU64, ()>>;
type TestHash = <TestBlock as BlockT>::Hash;

const TEST_PROTOCOL: &str = "/rwf/test/gossip/1";

fn keep_topic() -> TestHash {
	[1u8; 32].into()
}

struct KeepTopicValidator {
	topic: TestHash,
}

impl Validator<TestBlock> for KeepTopicValidator {
	fn validate(
		&self,
		_context: &mut dyn ValidatorContext<TestBlock>,
		_sender: &PeerId,
		_data: &[u8],
	) -> ValidationResult<TestHash> {
		ValidationResult::ProcessAndKeep(self.topic)
	}
}

fn build_gossip() -> ConsensusGossip<TestBlock> {
	let validator: Arc<dyn Validator<TestBlock>> =
		Arc::new(KeepTopicValidator { topic: keep_topic() });
	ConsensusGossip::new(validator, TEST_PROTOCOL.into(), None)
}

// RWF-INVARIANTS: inv_7yh6n8y8
#[test]
fn test_inv_7yh6n8y8_duplicate_register_not_doubly_stored() {
	let topic = keep_topic();
	let mut consensus = build_gossip();
	let message = vec![1, 2, 3, 4];

	consensus.register_message(topic, message.clone());
	consensus.register_message(topic, message.clone());

	let count = consensus
		.messages_for(topic)
		.filter(|notification| notification.message == message)
		.count();
	assert_eq!(count, 1, "registering the same message twice must not store it twice");
}

Reproduction Command

cargo test -p sc-network-gossip test_inv_7yh6n8y8_duplicate_register_not_doubly_stored -- --nocapture

Observed Result

The PoC fails on the current implementation with:

assertion `left == right` failed: registering the same message twice must not store it twice
left: 2
right: 1

Expected Behavior

The target should preserve the invariant described above instead of producing the confirmed failure. In particular, the affected operation should reject malformed or inconsistent input, preserve durable state invariants, or return an error rather than silently corrupting state or panicking.

Validation Rationale

The public source shows register_message_hashed using self.known_messages.insert(message_hash, ()) as the only dedup guard before pushing a MessageEntry. For schnellru::LruMap, inserting an existing key can still return true, so the guard does not distinguish new messages from replacements. The PoC calls register_message twice with the same message bytes and counts stored messages for that topic; two entries are visible where only one should be stored.

RWF Metadata

• Found by Runtime Whitebox Fuzzer
• Finding: find_d73civrycw
• Report: freport_v3yfh9ayvb
• Target: sc-network-gossip
• Run: 20260628-212807-a53980
• Severity: low
• Category: logic-bug
• Invariants: inv_7yh6n8y8, inv_apvyiva2
• Risks: risk_wzhx7u7e