From f261b7900f879ad991de42073094f8cb4443b8d2 Mon Sep 17 00:00:00 2001 From: SW van Heerden Date: Wed, 30 Aug 2023 16:44:18 +0200 Subject: [PATCH] fix: potential u64 overflow panic (#5688) Description --- fork_hash_index comes from remote peer, if the remote peer sets this to u64::Max, the local node will panic. This data is sanity checked later on, but here a panic is possible --- .../sync/header_sync/synchronizer.rs | 40 +++++++++---------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/base_layer/core/src/base_node/sync/header_sync/synchronizer.rs b/base_layer/core/src/base_node/sync/header_sync/synchronizer.rs index 90a27ffa15..61570410d1 100644 --- a/base_layer/core/src/base_node/sync/header_sync/synchronizer.rs +++ b/base_layer/core/src/base_node/sync/header_sync/synchronizer.rs @@ -420,8 +420,25 @@ impl<'a, B: BlockchainBackend + 'static> HeaderSynchronizer<'a, B> { return Err(err.into()); }, }; + if resp.headers.len() > NUM_INITIAL_HEADERS_TO_REQUEST { + self.ban_peer_long(peer, BanReason::PeerSentTooManyHeaders(resp.headers.len())) + .await?; + return Err(BlockHeaderSyncError::NotInSync); + } + if resp.fork_hash_index >= block_hashes.len() as u64 { + let _result = self + .ban_peer_long(peer, BanReason::SplitHashGreaterThanHashes { + fork_hash_index: resp.fork_hash_index, + num_block_hashes: block_hashes.len(), + }) + .await; + return Err(BlockHeaderSyncError::FoundHashIndexOutOfRange( + block_hashes.len() as u64, + resp.fork_hash_index, + )); + } - let steps_back = resp.fork_hash_index + offset as u64; + let steps_back = resp.fork_hash_index.saturating_add(offset as u64); return Ok((resp, block_hashes, steps_back)); } } @@ -440,14 +457,6 @@ impl<'a, B: BlockchainBackend + 'static> HeaderSynchronizer<'a, B> { let (resp, block_hashes, steps_back) = self .find_chain_split(sync_peer.node_id(), client, NUM_INITIAL_HEADERS_TO_REQUEST as u64) .await?; - if resp.headers.len() > NUM_INITIAL_HEADERS_TO_REQUEST { - self.ban_peer_long( - sync_peer.node_id(), - BanReason::PeerSentTooManyHeaders(resp.headers.len()), - ) - .await?; - return Err(BlockHeaderSyncError::NotInSync); - } let proto::FindChainSplitResponse { headers, fork_hash_index, @@ -464,19 +473,6 @@ impl<'a, B: BlockchainBackend + 'static> HeaderSynchronizer<'a, B> { ); } - if fork_hash_index >= block_hashes.len() as u64 { - let _result = self - .ban_peer_long(sync_peer.node_id(), BanReason::SplitHashGreaterThanHashes { - fork_hash_index, - num_block_hashes: block_hashes.len(), - }) - .await; - return Err(BlockHeaderSyncError::FoundHashIndexOutOfRange( - block_hashes.len() as u64, - fork_hash_index, - )); - } - // If the peer returned no new headers, this means header sync is done. if headers.is_empty() { if fork_hash_index > 0 {