fix(net): verify RandomX at correct height in AcceptBlockHeader + cap locator
header-pow: AcceptBlockHeader passed the caller's reused *ppindex (and a height
derived from it, ==0 for a new header) to CheckBlockHeader instead of the
header's own local pindex + real height. Post-IBD this made
RandomXValidationRequired(0) false, so CheckRandomXSolution returned true WITHOUT
verifying (and the fRandomXVerified short-circuit could fire on an unverified
header) - silently defeating the header-flood PoW gate from b9fdc7981. Resolve
pindexPrev up-front, pass real height (parent+1) and the local (NULL) pindex so
the post-IBD RandomX check actually runs; IBD stays fast (fCheckPOW=0).
Stability-tested: 303 valid headers accepted across a 4-node RandomX net,
0 false rejects / bans.
#9 (MEDIUM) GETBLOCKS/GETHEADERS deserialized an unbounded CBlockLocator.vHave
(~130k hashes) and scanned it linearly under cs_main with no ban - a
message-thread liveness DoS. Add MAX_LOCATOR_SZ=101 + Misbehaving, matching the
adjacent vInv/headers caps.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
37
src/main.cpp
37
src/main.cpp
@@ -5457,7 +5457,24 @@ bool AcceptBlockHeader(int32_t *futureblockp,const CBlockHeader& block, CValidat
|
||||
// SECURITY (header-flood DoS): once synced, verify PoW at header-accept time so a peer cannot
|
||||
// flood unbounded PoW-less headers into mapBlockIndex (they now fail RandomX -> DoS-ban). During
|
||||
// IBD keep fCheckPOW=0 for fast header sync; the full RandomX/target check runs at block connect.
|
||||
if (!CheckBlockHeader(futureblockp,*ppindex!=0?(*ppindex)->GetHeight():0,*ppindex, block, state, IsInitialBlockDownload() ? 0 : 1)) {
|
||||
// Resolve the parent up-front so CheckBlockHeader receives THIS header's own (still-NULL) pindex
|
||||
// and its CORRECT height (parent height + 1) — never the caller's reused *ppindex, which in a
|
||||
// HEADERS batch aliases the PREVIOUS header. Passing *ppindex here made (a) the height a stale
|
||||
// value (0 for a fresh header, or the prior header's height when aliased) so post-IBD
|
||||
// RandomXValidationRequired() saw a below-activation height and CheckRandomXSolution returned true
|
||||
// WITHOUT verifying, and (b) the (pindex && pindex->fRandomXVerified) short-circuit fire on an
|
||||
// as-yet-unverified header — both silently defeating the post-IBD header-flood PoW gate. The
|
||||
// authoritative parent validation (prev-not-found / prev-invalid) still runs unchanged below; this
|
||||
// lookup is read-only and under cs_main, so it cannot disagree with it. IBD stays fast: fCheckPOW
|
||||
// is still 0 during IBD, so no RandomX is computed here regardless of the height.
|
||||
CBlockIndex* pindexPrevForHeight = NULL;
|
||||
{
|
||||
BlockMap::iterator miPrev = mapBlockIndex.find(block.hashPrevBlock);
|
||||
if (miPrev != mapBlockIndex.end())
|
||||
pindexPrevForHeight = miPrev->second;
|
||||
}
|
||||
int32_t nHeaderHeight = (pindexPrevForHeight != NULL) ? pindexPrevForHeight->GetHeight() + 1 : 0;
|
||||
if (!CheckBlockHeader(futureblockp,nHeaderHeight,pindex, block, state, IsInitialBlockDownload() ? 0 : 1)) {
|
||||
if ( *futureblockp == 0 ) {
|
||||
LogPrintf("%s: CheckBlockHeader futureblock=0\n", __func__);
|
||||
return false;
|
||||
@@ -7587,6 +7604,15 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
|
||||
uint256 hashStop;
|
||||
vRecv >> locator >> hashStop;
|
||||
|
||||
// Bound the locator before FindForkInGlobalIndex() scans it linearly under cs_main. An honest
|
||||
// GetLocator() never exceeds MAX_LOCATOR_SZ, so this cannot reject a valid peer; an oversized
|
||||
// vHave (~130k hashes fit in one message) is a message-thread liveness DoS. Ban like the
|
||||
// adjacent vInv > MAX_INV_SZ path.
|
||||
if (locator.vHave.size() > MAX_LOCATOR_SZ) {
|
||||
Misbehaving(pfrom->GetId(), 20);
|
||||
return true;
|
||||
}
|
||||
|
||||
LOCK(cs_main);
|
||||
|
||||
// Find the last block the caller has in the main chain
|
||||
@@ -7619,6 +7645,15 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
|
||||
uint256 hashStop;
|
||||
vRecv >> locator >> hashStop;
|
||||
|
||||
// Bound the locator before FindForkInGlobalIndex() scans it linearly under cs_main. An honest
|
||||
// GetLocator() never exceeds MAX_LOCATOR_SZ, so this cannot reject a valid peer; an oversized
|
||||
// vHave (~130k hashes fit in one message) is a message-thread liveness DoS. Ban like the
|
||||
// adjacent vInv > MAX_INV_SZ path.
|
||||
if (locator.vHave.size() > MAX_LOCATOR_SZ) {
|
||||
Misbehaving(pfrom->GetId(), 20);
|
||||
return true;
|
||||
}
|
||||
|
||||
LOCK(cs_main);
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user