Skip to content

Commit 36f5eff

Browse files
committed
Merge bitcoin/bitcoin#31235: addrman: cap the max_pct to not exceed the maximum number of addresses
9c5775c addrman: cap the `max_pct` to not exceed the maximum number of addresses (brunoerg) Pull request description: Fixes #31234 This PR fixes a bad alloc issue in `GetAddresses` by capping the value `max_pct`. In practice, values greater than 100 should be treated as 100 since it's the percentage of addresses to return. Also, it limites the value `max_pct` in connman target to exercise values between 0 and 100. ACKs for top commit: adamandrews1: Code Review ACK bitcoin/bitcoin@9c5775c marcofleon: Tested ACK 9c5775c. Reproduced the crash on master and checked that this fixed it. The checks added to `GetAddr_` look reasonable. mzumsande: Code Review ACK 9c5775c vasild: ACK 9c5775c Tree-SHA512: 2957ae561ccc37df71f43c1863216d2e563522ea70b9a4baee6990e0b4a1ddadccabdcb9115c131a9a57480367b5ebdd03e0e3d4c8583792e2b7d1911a0a06d3
2 parents 98ad249 + 9c5775c commit 36f5eff

File tree

5 files changed

+7
-5
lines changed

5 files changed

+7
-5
lines changed

src/addrman.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -812,9 +812,11 @@ nid_type AddrManImpl::GetEntry(bool use_tried, size_t bucket, size_t position) c
812812
std::vector<CAddress> AddrManImpl::GetAddr_(size_t max_addresses, size_t max_pct, std::optional<Network> network, const bool filtered) const
813813
{
814814
AssertLockHeld(cs);
815+
Assume(max_pct <= 100);
815816

816817
size_t nNodes = vRandom.size();
817818
if (max_pct != 0) {
819+
max_pct = std::min(max_pct, size_t{100});
818820
nNodes = max_pct * nNodes / 100;
819821
}
820822
if (max_addresses != 0) {

src/addrman.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ class AddrMan
166166
* Return all or many randomly selected addresses, optionally by network.
167167
*
168168
* @param[in] max_addresses Maximum number of addresses to return (0 = all).
169-
* @param[in] max_pct Maximum percentage of addresses to return (0 = all).
169+
* @param[in] max_pct Maximum percentage of addresses to return (0 = all). Value must be from 0 to 100.
170170
* @param[in] network Select only addresses of this network (nullopt = all).
171171
* @param[in] filtered Select only addresses that are considered good quality (false = all).
172172
*

src/net.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1155,7 +1155,7 @@ class CConnman
11551155
* Return all or many randomly selected addresses, optionally by network.
11561156
*
11571157
* @param[in] max_addresses Maximum number of addresses to return (0 = all).
1158-
* @param[in] max_pct Maximum percentage of addresses to return (0 = all).
1158+
* @param[in] max_pct Maximum percentage of addresses to return (0 = all). Value must be from 0 to 100.
11591159
* @param[in] network Select only addresses of this network (nullopt = all).
11601160
* @param[in] filtered Select only addresses that are considered high quality (false = all).
11611161
*/

src/test/fuzz/addrman.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ FUZZ_TARGET(addrman, .init = initialize_addrman)
173173
network = fuzzed_data_provider.PickValueInArray(ALL_NETWORKS);
174174
}
175175
auto max_addresses = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 4096);
176-
auto max_pct = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 4096);
176+
auto max_pct = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 100);
177177
auto filtered = fuzzed_data_provider.ConsumeBool();
178178
(void)const_addr_man.GetAddr(max_addresses, max_pct, network, filtered);
179179

src/test/fuzz/connman.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,13 +110,13 @@ FUZZ_TARGET(connman, .init = initialize_connman)
110110
},
111111
[&] {
112112
auto max_addresses = fuzzed_data_provider.ConsumeIntegral<size_t>();
113-
auto max_pct = fuzzed_data_provider.ConsumeIntegral<size_t>();
113+
auto max_pct = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 100);
114114
auto filtered = fuzzed_data_provider.ConsumeBool();
115115
(void)connman.GetAddresses(max_addresses, max_pct, /*network=*/std::nullopt, filtered);
116116
},
117117
[&] {
118118
auto max_addresses = fuzzed_data_provider.ConsumeIntegral<size_t>();
119-
auto max_pct = fuzzed_data_provider.ConsumeIntegral<size_t>();
119+
auto max_pct = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 100);
120120
(void)connman.GetAddresses(/*requestor=*/random_node, max_addresses, max_pct);
121121
},
122122
[&] {

0 commit comments

Comments
 (0)