Merge bitcoin/bitcoin#30568: addrman: change internal id counting to int64_t

51f7668d31 addrman: change nid_type from int to int64_t (Martin Zumsande)
051ba3290e addrman, refactor: introduce user-defined type for internal nId (Martin Zumsande)

Pull request description:

  With `nIdCount` being incremented for each addr received, an attacker could cause an overflow in the past, see https://bitcoincore.org/en/2024/07/31/disclose-addrman-int-overflow/
  Even though that attack was made infeasible indirectly by addr rate-limiting (PR #22387), to be on the safe side and prevent any regressions change the `nId`s used internally to `int64_t`.
  This is being done by first introducing a user-defined type for `nId`s in the first commit, and then updating it to `int64_t` (thanks sipa for help with this!).

  Note that `nId` is only used internally, it is not part of the serialization, so `peers.dat` should not be affected by this.

  I assume that the only reason this was not done in the past is to not draw attention to this previously undisclosed issue.

ACKs for top commit:
  naumenkogs:
    ACK 51f7668d31
  stratospher:
    ACK 51f7668d31. I think it's a good change to make the nId space large(64 bits) so that the nId values are distinct.
  achow101:
    ACK 51f7668d31

Tree-SHA512: 68d4b8b0269a01a9544bedfa7c1348ffde00a288537e4c8bf2b88372ac7d96c4566a44dd6b06285f2fcf31b4f9336761e3bca7253fbc20db5e0d04e887156224
This commit is contained in:
Ava Chow
2024-09-20 12:55:22 -04:00
3 changed files with 45 additions and 37 deletions

View File

@@ -186,7 +186,7 @@ public:
return false;
}
auto IdsReferToSameAddress = [&](int id, int other_id) EXCLUSIVE_LOCKS_REQUIRED(m_impl->cs, other.m_impl->cs) {
auto IdsReferToSameAddress = [&](nid_type id, nid_type other_id) EXCLUSIVE_LOCKS_REQUIRED(m_impl->cs, other.m_impl->cs) {
if (id == -1 && other_id == -1) {
return true;
}