cryptonote_protocol: avoid overlapping reserved spans
What the developer wrote
Authored by selsta
cryptonote_protocol: avoid overlapping reserved spans
What changed, and why it matters
This commit fixes a bug in how Monero nodes request blocks from peers during blockchain synchronization. Before the fix, a node could accidentally reserve the same range of blocks twice from different peers, wasting bandwidth and potentially causing synchronization confusion. The fix adds a check to skip over any block ranges that are already reserved. A new test confirms the behavior.
Apply the patch and run the new unit test. Monitor for any related synchronization anomalies in P2P block queue management.
Security signals we found
Logic bug allowing overlapping reserved block spans in P2P synchronization
Potential duplicate block downloads causing bandwidth exhaustion or synchronization inefficiency
No input validation or memory corruption signals present
Fix includes regression unit test
Evidence from the diff
In block_queue::reserve_span(), the loop that walks through block hashes and reserves a contiguous span did not check whether a given hash/height was already covered by an existing internal span. This allowed overlapping reservations when the requested max_blocks size changed (e.g., after a span was removed and the sync size recalculated). The patch adds !requested_internal((*i).first) to the while-loop condition, so the queue skips already-requested blocks. A unit test demonstrates that removing a middle span and then requesting a larger span fills only the gap without overlapping the remaining reserved span.
Changed components
src/cryptonote_protocol/block_queue.cpptests/unit_tests/block_queue.cppInspect captured patch +22 / −1
diff --git a/src/cryptonote_protocol/block_queue.cpp b/src/cryptonote_protocol/block_queue.cpp
index c9603e5..e6daa00 100644
--- a/src/cryptonote_protocol/block_queue.cpp
+++ b/src/cryptonote_protocol/block_queue.cpp
@@ -297,7 +297,8 @@ std::pair<uint64_t, uint64_t> block_queue::reserve_span(uint64_t first_block_hei
uint64_t span_length = 0;
std::vector<crypto::hash> hashes;
bool first_is_pruned = sync_pruned_blocks && !tools::has_unpruned_block(span_start_height + span_length, blockchain_height, local_pruning_seed);
- while (i != block_hashes.end() && span_length < max_blocks && (sync_pruned_blocks || tools::has_unpruned_block(span_start_height + span_length, blockchain_height, pruning_seed)))
+ while (i != block_hashes.end() && span_length < max_blocks && !requested_internal((*i).first) &&
+ (sync_pruned_blocks || tools::has_unpruned_block(span_start_height + span_length, blockchain_height, pruning_seed)))
{
// if we want to sync pruned blocks, stop at the first block for which we need full data
if (sync_pruned_blocks && first_is_pruned == tools::has_unpruned_block(span_start_height + span_length, blockchain_height, local_pruning_seed))
diff --git a/tests/unit_tests/block_queue.cpp b/tests/unit_tests/block_queue.cpp
index a643b87..e9d1cd6 100644
--- a/tests/unit_tests/block_queue.cpp
+++ b/tests/unit_tests/block_queue.cpp
@@ -87,3 +87,23 @@ TEST(block_queue, flush_uuid)
bq.add_blocks(0, 200, uuid1(), na);
ASSERT_EQ(bq.get_max_block_height(), 399);
}
+
+TEST(block_queue, reserve_does_not_overlap_later_span)
+{
+ cryptonote::block_queue bq;
+ epee::net_utils::network_address na;
+ std::vector<std::pair<crypto::hash, uint64_t>> hashes;
+ hashes.reserve(100);
+ for (size_t i = 0; i < 100; ++i)
+ hashes.emplace_back(crypto::rand<crypto::hash>(), 0);
+
+ ASSERT_EQ(bq.reserve_span(1, 100, 28, uuid1(), na, false, 0, 0, 101, hashes), std::make_pair(uint64_t{1}, uint64_t{28}));
+ ASSERT_EQ(bq.reserve_span(1, 100, 28, uuid1(), na, false, 0, 0, 101, hashes), std::make_pair(uint64_t{29}, uint64_t{28}));
+ ASSERT_EQ(bq.reserve_span(1, 100, 28, uuid1(), na, false, 0, 0, 101, hashes), std::make_pair(uint64_t{57}, uint64_t{28}));
+
+ ASSERT_TRUE(bq.remove_span(29));
+
+ // A recalculated, larger sync size must fill only the gap, without
+ // overlapping the span which is already reserved at height 57.
+ ASSERT_EQ(bq.reserve_span(1, 100, 36, uuid2(), na, false, 0, 0, 101, hashes), std::make_pair(uint64_t{29}, uint64_t{28}));
+}
Why this scored 59/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.