Audit report AR-24
Reported privately in an independent security review, 2026-08-20.
- Report date
- 2026-08-20
- Kind
- API
- Status on master
- Present
- Proposed issue state
- Open
- Linked bugs
- Bug 53: Node accepts its own invalid block when a mining client omits the witness commitment
Summary
The Mining interface's checkBlock() and submitBlock() pass the caller's CBlock to validation with its cached check flags intact, so an in-process C++ caller that changes a block after it was checked and reuses the object can get a wrong validation result. IPC clients cannot trigger this, because a block read from the wire starts with clean flags. The report is accurate and calls it API hardening. It is the same class of problem as a real bug fixed in submitSolution.
The report
CBlock caches the results of some validation checks in mutable fields (for example whether the witness commitment and the full block were already checked). The mining interface should validate the block contents it is given independently of earlier state, but checkBlock() hands the caller's block to TestBlockValidity as is, and submitBlock() copies it with std::make_shared<const CBlock>(block_in), which copies the flags too. The underlying checks can return early when the flags describe an earlier version of the object.
The report requires an in-process C++ caller that reuses a block object after changing its contents without clearing its cached state. It identifies no RPC or peer route, no deployed affected embedder, no funds loss, consensus split or persistent corruption, and says the wrong result can be silent to the caller. Recovery after an invalid block was accepted was not tested.
The reproduction is two Boost unit tests in src/test/miner_tests.cpp against the real node code, submit_block_rechecks_copied_witness_cache and submit_block_rechecks_copied_full_cache. They build blocks whose cached flags are stale, and show wrong validation and submission results, including a change to the local active tip, then check that the fix rejects the stale cases and still accepts the corresponding valid blocks.
Proposed change
Build a fresh CBlock from the header and transactions at both entry points, so its caches start unset and the caller's object is not updated:
bool checkBlock(const CBlock& block, const node::BlockCheckOptions& options, std::string& reason, std::string& debug) override
{
+ // Interface callers may pass copies containing stale mutable validation caches.
+ CBlock clean_block{static_cast<const CBlockHeader&>(block)};
+ clean_block.vtx = block.vtx;
LOCK(chainman().GetMutex());
- BlockValidationState state{TestBlockValidity(chainman().ActiveChainstate(), block, ...)};
+ BlockValidationState state{TestBlockValidity(chainman().ActiveChainstate(), clean_block, ...)};
@@
bool submitBlock(const CBlock& block_in, std::string& reason, std::string& debug) override
{
- return SubmitBlock(chainman(), std::make_shared<const CBlock>(block_in), reason, debug);
+ // Never let caller-controlled validation caches reach chainstate.
+ auto block{std::make_shared<CBlock>(static_cast<const CBlockHeader&>(block_in))};
+ block->vtx = block_in.vtx;
+ return SubmitBlock(chainman(), std::move(block), reason, debug);
Assessment
Present on Bitcoin Core master: checkBlock passes block straight to TestBlockValidity, and submitBlock (now through block_template_manager().SubmitBlock) still copies with make_shared<const CBlock>(block_in), which keeps the flags. The diff needs a trivial rebase for that move.
Only in-process C++ callers are affected. Over IPC the block is deserialized fresh on the node side, with every cached flag false, so no IPC client can reach this. It is outside the survey's stability scope.
The same stale-flag problem was a real bug in submitSolution, which reuses the node's own template block: BlockTemplate::submitSolution reuses a template whose cached witness-commitment check is stale, fixed by bitcoin#33745. This report extends the same defensive idea to blocks supplied by the caller.
Whether the mining API should defend against callers reusing a mutated block, or document that callers must reset the flags, is the design question. The change itself is small.
Status
Present on master. Next step: a small Bitcoin Core PR with the fix and tests, if the API should be defensive. No public issue or PR yet.
Reported privately in an independent security review on 2026-08-20.