Page MenuHomePhabricator

Require callers of AcceptBlockHeader() to perform anti-dos checks
ClosedPublic

Authored by PiRK on Jan 10 2024, 14:31.

Details

Summary

In order to prevent memory DoS, we must ensure that we don't accept a new
header into memory until we've performed anti-DoS checks, such as verifying
that the header is part of a sufficiently high work chain. This commit adds a
new argument to AcceptBlockHeader() so that we can ensure that all call-sites
which might cause a new header to be accepted into memory have to grapple with
the question of whether the header is safe to accept, or needs further
validation.

This patch also fixes two places where low-difficulty-headers could have been
processed without such validation (processing an unrequested block from the
network, and processing a compact block).

Credit to Niklas Gögge for noticing this issue, and thanks to Sjors Provoost
for test code.

This is a partial backport of core#25717
https://github.com/bitcoin/bitcoin/pull/25717/commits/ed6cddd98e32263fc116a4380af6d66da20da990

Depends on D15129

Test Plan

ninja all check-all

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

PiRK requested review of this revision.Jan 10 2024, 14:31
Fabien requested changes to this revision.Jan 11 2024, 15:09
Fabien added a subscriber: Fabien.
Fabien added inline comments.
src/net_processing.cpp
5318 ↗(On Diff #44027)

you don't need the else

5729 ↗(On Diff #44027)

What happens if we get the genesis block here ? It is explicitly tested for compact blocks but not for regular blocks ?

This revision now requires changes to proceed.Jan 11 2024, 15:09
src/net_processing.cpp
5729 ↗(On Diff #44027)

I don't think there is an issue here. If we get the genesis block min_pow_checked will remain false and we will just not store it in the index in AcceptBlockHeader, which we don't want to do for the genesis block anyway.

This revision is now accepted and ready to land.Jan 11 2024, 16:34