Audit report AR-31
Reported privately in an independent security review, 2026-09-06.
- Report date
- 2026-09-06
- Kind
- Bug
- Status on master
- Present
- Proposed issue state
- Open
- Linked bugs
- Bug 83: Reading a block template while submitting a solution for it is a data race
Summary
BlockTemplate::submitSolution rewrites the stored block (coinbase, header fields, merkle root and cached flags) without a lock, while the template's getters read the same block. An IPC client that uses one template from two threads at once, for example submitting a solution while another thread reads the block, causes a data race in bitcoin-node. The report is correct; the race is real but needs an unusual client pattern and has never been seen outside a unit test. Open bug, with a mergeable fix after review.
The report
submitSolution() modifies the stored template's coinbase, header, merkle root and cached block flags, while the getters (getBlock, getBlockHeader, getCoinbaseTx, getCoinbaseMerklePath and others) read the same object with no synchronization. The report names concurrent local IPC calls as one route to overlapping access, since each client thread is served by its own worker thread, and in-process callers as another. It also says the waitNext path reads the shared template while waiting.
It states that the prerequisite is sharing a live template across concurrent calls, and that the concern is a data race in one process: no corrupted template, invalid submission, crash, funds loss, persistent state or consensus effect was reproduced.
The reproduction is a Boost unit test, BlockTemplate_concurrent_submit_and_read, in src/test/miner_tests.cpp. It runs a submission and a block copy on the same template concurrently; under ThreadSanitizer it reports the conflicting access without the fix and runs clean with it. It is an in-process test, not an end-to-end IPC demonstration.
Proposed change
Add a mutex guarding the stored template, lock it in every getter, and build the submission copy under the lock but release it before validation:
- CBlock getBlock() override
+ CBlock getBlock() override EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
{
+ LOCK(m_mutex);
return m_block_template->block;
}
@@
bool submitSolution(...) override
{
if (!coinbase) return false;
- AddMerkleRootAndCoinbase(m_block_template->block, std::move(coinbase), version, timestamp, nonce);
- return SubmitBlock(chainman(), std::make_shared<const CBlock>(m_block_template->block), reason, debug);
+ std::shared_ptr<const CBlock> block;
+ {
+ LOCK(m_mutex);
+ AddMerkleRootAndCoinbase(m_block_template->block, std::move(coinbase), version, timestamp, nonce);
+ block = std::make_shared<const CBlock>(m_block_template->block);
+ }
+ return SubmitBlock(chainman(), block, reason, debug);
@@
- const std::unique_ptr<CBlockTemplate> m_block_template;
+ Mutex m_mutex;
+ const std::unique_ptr<CBlockTemplate> m_block_template GUARDED_BY(m_mutex);
waitNext takes a snapshot of the template's previous-block hash and, only when a fee threshold is set, its total fees under the lock, and WaitAndCreateNewBlock in src/node/miner.cpp takes those two values instead of the template. The report notes the change protects the template data, not the lifetime of the object or concurrent destruction, and that the test covers one reader against one submission.
Assessment
Present on Bitcoin Core master: BlockTemplateImpl::submitSolution calls AddMerkleRootAndCoinbase(m_block_template->block, ...), which assigns block.vtx[0] and rewrites header fields and cached flags with no lock, while the getters read the same object. Concurrent copying and assignment of the same shared_ptr element is undefined behaviour and can corrupt its reference count. Over IPC this happens only if a client uses one template from two threads at once, since each client thread maps to its own server worker. A typical pool client submits a solution for a template it has finished reading, so the practical window is small.
One part of the motivation is weaker than stated: the waitNext path (now BlockTemplateManager::WaitAndCreateNewBlock) reads only the template's hashPrevBlock and vTxFees, neither of which submitSolution writes. The snapshot change in the patch is harmless but not needed for this race. Since the report, bitcoin#35675 moved the wait code into BlockTemplateManager, so the diff needs a rebase.
The same function had a real bug from reusing the stored block: BlockTemplate::submitSolution reuses a template whose cached witness-commitment check is stale.
Reproduction (2026-09-26). A separate in-process test against Bitcoin Core master bfdcd97 calls getBlock() on one thread while submitSolution runs on another, on the same template. ThreadSanitizer reported 4 data races, the first on the coinbase shared_ptr (vtx[0]). Without the sanitizer the test passed, so no torn result or crash was seen. See the bug page's reproduction section.
Status
Open on Bitcoin Core master. Reported publicly in bitcoin#36515 (2026-10-13, found with fuzzamoto); a fix protecting the template with a mutex is in review in bitcoin#36525.
Reported privately in an independent security review on 2026-09-06.