Skip to content

Commit

Permalink
Merge bitcoin/bitcoin#31235: addrman: cap the max_pct to not exceed…
Browse files Browse the repository at this point in the history
… 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
  • Loading branch information
fanquake committed Nov 13, 2024
2 parents 98ad249 + 9c5775c commit 36f5eff
Show file tree
Hide file tree
Showing 5 changed files with 7 additions and 5 deletions.
2 changes: 2 additions & 0 deletions src/addrman.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -812,9 +812,11 @@ nid_type AddrManImpl::GetEntry(bool use_tried, size_t bucket, size_t position) c
std::vector<CAddress> AddrManImpl::GetAddr_(size_t max_addresses, size_t max_pct, std::optional<Network> network, const bool filtered) const
{
AssertLockHeld(cs);
Assume(max_pct <= 100);

size_t nNodes = vRandom.size();
if (max_pct != 0) {
max_pct = std::min(max_pct, size_t{100});
nNodes = max_pct * nNodes / 100;
}
if (max_addresses != 0) {
Expand Down
2 changes: 1 addition & 1 deletion src/addrman.h
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ class AddrMan
* Return all or many randomly selected addresses, optionally by network.
*
* @param[in] max_addresses Maximum number of addresses to return (0 = all).
* @param[in] max_pct Maximum percentage of addresses to return (0 = all).
* @param[in] max_pct Maximum percentage of addresses to return (0 = all). Value must be from 0 to 100.
* @param[in] network Select only addresses of this network (nullopt = all).
* @param[in] filtered Select only addresses that are considered good quality (false = all).
*
Expand Down
2 changes: 1 addition & 1 deletion src/net.h
Original file line number Diff line number Diff line change
Expand Up @@ -1155,7 +1155,7 @@ class CConnman
* Return all or many randomly selected addresses, optionally by network.
*
* @param[in] max_addresses Maximum number of addresses to return (0 = all).
* @param[in] max_pct Maximum percentage of addresses to return (0 = all).
* @param[in] max_pct Maximum percentage of addresses to return (0 = all). Value must be from 0 to 100.
* @param[in] network Select only addresses of this network (nullopt = all).
* @param[in] filtered Select only addresses that are considered high quality (false = all).
*/
Expand Down
2 changes: 1 addition & 1 deletion src/test/fuzz/addrman.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ FUZZ_TARGET(addrman, .init = initialize_addrman)
network = fuzzed_data_provider.PickValueInArray(ALL_NETWORKS);
}
auto max_addresses = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 4096);
auto max_pct = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 4096);
auto max_pct = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 100);
auto filtered = fuzzed_data_provider.ConsumeBool();
(void)const_addr_man.GetAddr(max_addresses, max_pct, network, filtered);

Expand Down
4 changes: 2 additions & 2 deletions src/test/fuzz/connman.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,13 +110,13 @@ FUZZ_TARGET(connman, .init = initialize_connman)
},
[&] {
auto max_addresses = fuzzed_data_provider.ConsumeIntegral<size_t>();
auto max_pct = fuzzed_data_provider.ConsumeIntegral<size_t>();
auto max_pct = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 100);
auto filtered = fuzzed_data_provider.ConsumeBool();
(void)connman.GetAddresses(max_addresses, max_pct, /*network=*/std::nullopt, filtered);
},
[&] {
auto max_addresses = fuzzed_data_provider.ConsumeIntegral<size_t>();
auto max_pct = fuzzed_data_provider.ConsumeIntegral<size_t>();
auto max_pct = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 100);
(void)connman.GetAddresses(/*requestor=*/random_node, max_addresses, max_pct);
},
[&] {
Expand Down

0 comments on commit 36f5eff

Please sign in to comment.