Skip to content

Commit

Permalink
fix: potential u64 overflow panic (tari-project#5688)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
SWvheerden authored Aug 30, 2023
1 parent 304e40f commit f261b79
Showing 1 changed file with 18 additions and 22 deletions.
40 changes: 18 additions & 22 deletions base_layer/core/src/base_node/sync/header_sync/synchronizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
Expand All @@ -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,
Expand All @@ -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 {
Expand Down

0 comments on commit f261b79

Please sign in to comment.