The current addr relay token logic opens an attack that allows a malicious outbound peer to abuse our unsolicited message relaying as mentioned on #34717 (comment):
- We establish an outbound connection to a node X.
- We increase the token counter to +1000 tokens since we expect to receive up to 1000 addresses in response from node X.
- Node X sends a buffer containing only 1 address, to bypasses the initial
m_getaddr_sentcheck (or this negligible protection disappears entirely if the variable is removed as proposed in this PR)- Node X then starts sending unsolicited messages of size 10, since it knows it can use up to 1000 tokens (each address they will consume 1 token and if the number of addresses in the buffer is less or equal to 10 our node will relay it).
- Our node become an intermediate relay, unintentionally helping hide this malicious node, since its (our node) the one who will relay the unsolicited messages.
- Our node they will consume all of its tokens with other nodes by forwarding malicious addresses instead of forwarding addresses for legitimate nodes.
This issue predates PR #34146, which introduced a new addr self-announcement mechanism that rendered m_getaddr_sent entirely ineffective, leading PR #34717 to propose removing it entirely.
Instead of removing m_getaddr_sent, this PR keeps it and improves the token handling logic.
The addr self-announcement is ordered to be sent after the GETADDR response, allowing the GETADDR sender to clearly distinguish between the response and the self-announcement, and safely assume the first incoming message is the legitimate response.
Once the GETADDR response is processed, the token bucket is adjusted to count only the addresses actually received.
