Merge bitcoin/bitcoin#30538: Doc: add a comment referencing past vulnerability next to where it was fixed

eb0724f0de doc: banman: reference past vuln due to unbounded banlist (Antoine Poinsot)
ad616b6c01 doc: net: mention past vulnerability as rationale to limit incoming message size (Antoine Poinsot)
4489117c3f doc: txrequest: point to past censorship vulnerability in tx re-request handling (Antoine Poinsot)
68ac9542c4 doc: net_proc: reference past DoS vulnerability in orphan processing (Antoine Poinsot)
c02d9f6dd5 doc: net_proc: reference past defect regarding invalid GETDATA types (Antoine Poinsot)
5e3d9f21df doc: validation: add a reference to historical header spam vulnerability (Antoine Poinsot)

Pull request description:

  It is useful when reading code to have context about why it is written or behaves the way it does. Some instances in this PR may seem obvious but i think nonetheless offer important context to anyone willing to change (or review a change to) this code.

ACKs for top commit:
  ryanofsky:
    Code review ACK eb0724f0de. No changes since last review other than rebase

Tree-SHA512: 271902f45b8130d44153d793bc1096cd22b6ce05494e67c665a5bc45754e3fc72573d303ec8fc7db4098d473760282ddbf0c1cf316947539501dfd8d7d5b8828
This commit is contained in:
Ryan Ofsky
2025-03-23 10:16:41 -04:00
5 changed files with 20 additions and 0 deletions

View File

@@ -2425,6 +2425,9 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic
}
// else: If the first item on the queue is an unknown type, we erase it
// and continue processing the queue on the next call.
// NOTE: previously we wouldn't do so and the peer sending us a malformed GETDATA could
// result in never making progress and this thread using 100% allocated CPU. See
// https://bitcoincore.org/en/2024/07/03/disclose-getdata-cpu.
}
peer.m_getdata_requests.erase(peer.m_getdata_requests.begin(), it);
@@ -3068,6 +3071,8 @@ void PeerManagerImpl::ProcessPackageResult(const node::PackageToValidate& packag
}
}
// NOTE: the orphan processing used to be uninterruptible and quadratic, which could allow a peer to stall the node for
// hours with specially crafted transactions. See https://bitcoincore.org/en/2024/07/03/disclose-orphan-dos.
bool PeerManagerImpl::ProcessOrphanTx(Peer& peer)
{
AssertLockHeld(g_msgproc_mutex);