IPC stability bugs

Bug 84

Negative or NaN wait timeout from a mining client aborts debug builds of the node

Status
Open
Symptom
Assertion failure; worst effect seen: assertion failure
Where seen
Bitcoin Core functional tests
Trigger
Unusual input from a peer or caller
First reported
Reported privately in an independent security review, 2026-09-18
Fixed
Not yet
In released binaries
Yes (v30.0 onward)
Defect ever on master
Yes

Summary

An IPC client that calls Mining.waitTipChanged (wait until the chain tip changes, or until a timeout) with a negative or NaN timeout makes bitcoin-node abort on Assume(timeout >= 0ms) in builds with ABORT_ON_FAILED_ASSUME (the Debug build type and all fuzz builds). In release builds the Assume does nothing and a negative timeout is clamped to zero, but NaN passes both clamps and reaches std::condition_variable::wait_until, which is undefined behavior. It was reported privately in an independent security review, with a functional test that aborts a checked build, and a separate test gives the same abort over IPC; the release-build NaN effect comes from reading the code. Released binaries from v30.0 on accept the value; their NaN case is the only one that can misbehave there. No fix yet.

What happened

No user has hit this. An independent security review of Bitcoin Core's IPC code reported it privately (2026-09-18). The report adds an interface_ipc_mining.py case that calls waitTipChanged with -1.0 and float("nan"); against a build with runtime assumption checks, the node aborted on the Assume and the client was disconnected. The NaN effect in release builds was shown by reading the code and by a standalone UBSan test of libc++'s condition_variable::wait_until, not against a node.

Root cause

Mining.waitTipChanged takes timeout :Float64 (src/ipc/capnp/mining.capnp). libmultiprocess's CustomReadField for std::chrono::duration (include/mp/type-chrono.h), the code that turns the wire value into a C++ argument, builds the MillisecondsDouble straight from the wire value without checking it. MinerImpl::waitTipChanged (src/node/interfaces.cpp) passes it on to BlockTemplateManager::WaitTipChanged (src/node/block_template_manager.cpp), which begins:

Assume(timeout >= 0ms); // No internal callers should use a negative timeout
if (timeout < 0ms) timeout = 0ms;
if (timeout > std::chrono::years{100}) timeout = std::chrono::years{100}; // Upper bound to avoid UB in std::chrono

The Assume was written for internal callers, but over IPC the function's input comes from the client. Assume calls assertion_fail and aborts when ABORT_ON_FAILED_ASSUME is set. NaN fails the Assume as well, and in release builds it passes both clamps because every comparison with NaN is false. It then reaches std::chrono::steady_clock::now() + timeout and m_tip_block_cv.wait_until(lock, deadline, ...), where, if the tip has not changed, the standard library converts the NaN time point to an integer clock value. That conversion is undefined behavior; the most likely result is an immediate or erratic timeout rather than a crash, since on common hardware the conversion just produces some arbitrary integer deadline, but this has not been tested against a node.

A C++ client can also send a negative value, for example by computing deadline - now after the deadline has passed.

This is the same kind of mistake as the one fixed by bitcoin#33222 ("miner: clamp options instead of asserting"), where client-supplied BlockCreateOptions tripped an Assert: client input treated as a precondition.

Fix

No fix yet. The private report proposes treating NaN and negative timeouts as zero before the deadline is computed.

Reproduction

A new first case in the functional test interface_ipc_mining.py gets the current tip over IPC and calls mining.waitTipChanged(ctx, tip, -1.0), expecting the call to return like a zero timeout and the node to keep running. The node was a Debug build, which sets ABORT_ON_FAILED_ASSUME. It was run on 2026-09-26 against Bitcoin Core master bfdcd97, which carries libmultiprocess v13 in its subtree.

Patch: bitcoin-wait-timeout-functional.patch. It applies to Bitcoin Core master; last checked against badc957 on 2026-10-15. From a Bitcoin Core checkout:

curl -sO https://ipcbugs.ofsky.org/repro/bitcoin-wait-timeout-functional.patch
git apply bitcoin-wait-timeout-functional.patch
cmake --build build
build/test/functional/interface_ipc_mining.py

The node aborted on the Assume, and the client lost its connection:

node/block_template_manager.cpp:254 std::optional<BlockRef> node::BlockTemplateManager::WaitTipChanged(const uint256 &, MillisecondsDouble &, bool &): Assertion `timeout >= 0ms' failed.
capnp.lib.capnp.KjException: capnp/rpc.c++:2779: disconnected: Peer disconnected.

The result is the same on every run. NaN was not tried. The test patch is available on request and will be included in the GitHub issue for this bug when it is opened.

Next steps

Timeline

Lessons

What would have prevented this bug, found it sooner, or made it easier to debug. The patterns page groups these lessons across all bugs.

Pattern An in-process interface trusting what now comes from another process: An assertion on what is now input from a peer

  • Prevented by a design rule, a review checklist item, a different API shape
  • Earliest detector fuzzing (did not exist then)
  • Diagnosis easy
  • Fix design easy
  • Debugging aids a clear assertion message
  • Rust partly

Would Rust have prevented it? Partly. Std::time::Duration is unsigned, so a negative internal timeout is unrepresentable, but the wire value is still an f64 and Duration::from_secs_f64 panics on negative or NaN input, so the abort remains unless the conversion is written with try_from_secs_f64. More on Rust

Could it have been prevented?

Yes. Assume(timeout >= 0ms) was added by f9cf8bd0ab77 with the comment "No internal callers should use a negative timeout", six months after Mining.waitTipChanged had gone over IPC, where the same function is the entry point for a value any client sends. The rule that would have stopped it is the one bitcoin#33222 later stated for BlockCreateOptions: argument checks on an IPC-reachable method are validation, never Assert or Assume. It was knowable in April 2025; the author's own clamp on the next line shows the value was expected to arrive. NaN is a second, older instance of the same gap: a Float64 timeout on the wire admits values a std::chrono duration built from internal arithmetic never takes, so the type conversion in type-chrono.h is the natural place to reject or clamp them. Using an integer milliseconds field in the schema would have removed the NaN case at the API level.

Could it have been found sooner?

Yes. A fuzzer at the IPC boundary sends negative and NaN doubles early; fuzzamoto existed from September 2025 and found the malformed-argument sibling, but its harness did not exercise waitTipChanged with odd timeouts. A Python client (bitcoin#33201-style pycapnp tests) could send both values from August 2025; nobody wrote the case until the independent security review did in September 2026. The C++ client cannot easily produce NaN, and its negative case needs a late deadline - now, so the in-tree functional tests had no path to it. The Assume only aborts in Debug and fuzz builds, which is why fuzzing is the check that would have paid off first.

What made it hard to debug?

Nothing. The abort names the failed expression and the file, and the survey's reproduction is deterministic on the first call. The release-build NaN path is harder to observe (an arbitrary deadline rather than a crash) and was established by reading the code and a standalone UBSan probe, which is the honest limit of what is known.

Lesson

A check that says "internal callers never do this" is wrong the day the function gets an external caller; when an interface goes over the wire, re-read every Assume on its arguments, and choose wire types that cannot express the values you would have to reject.

Notes

Classification

The survey's classification of this bug. The explorer charts these fields across all bugs; the method page explains them.

Field Value
Symptom assert
Worst effect observed assert
Seen in functional-test
Scope production
Trigger Unusual input (unusual-input)
Phase steady-state
Side server
Component bitcoin-mining
Root cause kind assumption-violated
Fix kind none
Detection review
Needs a peer that is not libmultiprocess C++ no
Needed a sanitizer to see no
Defect ever on master yes
Released binary
Confidence high