QA-01: No Deduplication of Peers in ConfirmGroup Allows Traffic Amplification

monad-qa-01-no-deduplication-of-peers

Summary

ConfirmGroup.peers is accepted as long as it contains the receiver and does not exceed max_group_size, but the peer list is not deduplicated before group construction.

Details

The secondary client forwards the peer vector into group construction, where the implementation removes only self and sorts the remaining peers. It does not enforce uniqueness. A malicious validator can therefore repeat the same NodeId multiple times and force redundant sends to the same destination.

1
if confirm_msg.peers.len() > confirm_msg.prepare.max_group_size {
2
    return;
3
}
4

5
if !confirm_msg.peers.contains(&self.client_node_id) {
6
    return;
7
}
8

9
let group = GroupAsClient::new_fullnode_group(
10
    confirm_msg.peers,
11
    &self.client_node_id,
12
    confirm_msg.prepare.validator_id,
13
    round_span,
14
);

1
pub fn new_fullnode_group(
2
    all_peers: Vec<NodeId<CertificateSignaturePubKey<ST>>>,
3
    self_id: &NodeId<CertificateSignaturePubKey<ST>>,
4
    validator_id: NodeId<CertificateSignaturePubKey<ST>>,
5
    round_span: RoundSpan,
6
) -> Self {
7
    let mut sorted_other_peers = all_peers;
8
    if self_id != &validator_id {
9
        let self_index = sorted_other_peers
10
            .iter()
11
            .position(|peer| peer == self_id)
12
            .expect("Could not find own node id");
13
        sorted_other_peers.swap_remove(self_index);
14
    }
15
    sorted_other_peers.sort();

Impact

The result is avoidable traffic amplification and extra processing work. The blast radius is capped by max_group_size, but uniqueness should still be enforced on the path.

Recommendation

Deduplicate ConfirmGroup.peers before creating the group object.