From 63c2d83e58c7506a1030e7fe4dcea84cdce97147 Mon Sep 17 00:00:00 2001 From: practicalswift Date: Fri, 10 Nov 2017 15:22:54 +0100 Subject: [PATCH] Explicitly state assumption that state.m_chain_sync.m_work_header != nullptr in ConsiderEviction Static analyzer (and humans!) will see ... ``` else if (state.m_chain_sync.m_timeout == 0 || (state.m_chain_sync.m_work_header != nullptr && ... ``` ... and infer that state.m_chain_sync.m_work_header might be set to nullptr, and thus flag `state.m_chain_sync.m_work_header->GetBlockHash().ToString()` as a potential null pointer dereference. This commit makes the tacit assumption (m_work_header != nullptr) explicit. Code introduced in 5a6d00 ("Permit disconnection of outbound peers on bad/slow chains") which was merged into master four days ago. --- src/net_processing.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 8e503f89d..38b9703d9 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3006,6 +3006,7 @@ void PeerLogicValidation::ConsiderEviction(CNode *pto, int64_t time_in_seconds) LogPrintf("Disconnecting outbound peer %d for old chain, best known block = %s\n", pto->GetId(), state.pindexBestKnownBlock != nullptr ? state.pindexBestKnownBlock->GetBlockHash().ToString() : ""); pto->fDisconnect = true; } else { + assert(state.m_chain_sync.m_work_header); LogPrint(BCLog::NET, "sending getheaders to outbound peer=%d to verify chain work (current best known block:%s, benchmark blockhash: %s)\n", pto->GetId(), state.pindexBestKnownBlock != nullptr ? state.pindexBestKnownBlock->GetBlockHash().ToString() : "", state.m_chain_sync.m_work_header->GetBlockHash().ToString()); connman->PushMessage(pto, msgMaker.Make(NetMsgType::GETHEADERS, chainActive.GetLocator(state.m_chain_sync.m_work_header->pprev), uint256())); state.m_chain_sync.m_sent_getheaders = true;