From 8cc4b24c74ecf7a3c3d2853fe8ecb474eb77284c Mon Sep 17 00:00:00 2001 From: dergoegge Date: Fri, 19 Jan 2024 15:25:01 +0000 Subject: [net processing] Don't process mutated blocks We preemptively perform a block mutation check before further processing a block message (similar to early sanity checks on other messsage types). The main reasons for this change are as follows: - `CBlock::GetHash()` is a foot-gun without a prior mutation check, as the hash returned only commits to the header but not to the actual transactions (`CBlock::vtx`) contained in the block. - We have observed attacks that abused mutated blocks in the past, which could have been prevented by simply not processing mutated blocks (e.g. https://github.com/bitcoin/bitcoin/pull/27608). Github-Pull: #29412 Rebased-From: 49257c0304828a185c273fcb99742c54bbef0c8e --- src/net_processing.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index d48a118dd6..5c368aaaad 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -4629,6 +4629,16 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, LogPrint(BCLog::NET, "received block %s peer=%d\n", pblock->GetHash().ToString(), pfrom.GetId()); + const CBlockIndex* prev_block{WITH_LOCK(m_chainman.GetMutex(), return m_chainman.m_blockman.LookupBlockIndex(pblock->hashPrevBlock))}; + + if (IsBlockMutated(/*block=*/*pblock, + /*check_witness_root=*/DeploymentActiveAfter(prev_block, m_chainman, Consensus::DEPLOYMENT_SEGWIT))) { + LogPrint(BCLog::NET, "Received mutated block from peer=%d\n", peer->m_id); + Misbehaving(*peer, 100, "mutated block"); + WITH_LOCK(cs_main, RemoveBlockRequest(pblock->GetHash(), peer->m_id)); + return; + } + bool forceProcessing = false; const uint256 hash(pblock->GetHash()); bool min_pow_checked = false; @@ -4644,7 +4654,6 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, mapBlockSource.emplace(hash, std::make_pair(pfrom.GetId(), true)); // Check work on this block against our anti-dos thresholds. - const CBlockIndex* prev_block = m_chainman.m_blockman.LookupBlockIndex(pblock->hashPrevBlock); if (prev_block && prev_block->nChainWork + CalculateHeadersWork({pblock->GetBlockHeader()}) >= GetAntiDoSWorkThreshold()) { min_pow_checked = true; } -- cgit v1.2.3 From cf7d3a8cd07c26c700eee4bc1a16092982625326 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Thu, 29 Feb 2024 16:38:58 -0500 Subject: p2p: Don't consider blocks mutated if they don't connect to known prev block Github-Pull: #29524 Rebased-From: a1fbde0ef7cf6c94d4a5181f8ceb327096713160 --- src/net_processing.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 5c368aaaad..43e15487f3 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -4631,7 +4631,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, const CBlockIndex* prev_block{WITH_LOCK(m_chainman.GetMutex(), return m_chainman.m_blockman.LookupBlockIndex(pblock->hashPrevBlock))}; - if (IsBlockMutated(/*block=*/*pblock, + // Check for possible mutation if it connects to something we know so we can check for DEPLOYMENT_SEGWIT being active + if (prev_block && IsBlockMutated(/*block=*/*pblock, /*check_witness_root=*/DeploymentActiveAfter(prev_block, m_chainman, Consensus::DEPLOYMENT_SEGWIT))) { LogPrint(BCLog::NET, "Received mutated block from peer=%d\n", peer->m_id); Misbehaving(*peer, 100, "mutated block"); -- cgit v1.2.3