Audit report AR-54

Reported privately in an independent security review, 2026-09-27.

Report date

2026-09-27

Kind

Bug

Status on master

Present

Proposed issue state

Open

Linked bugs

Bug 97: Failed IPC startup leaks the socket descriptor and the spawned child

Summary

Several steps of Bitcoin Core's IPC setup leave resources behind when they fail. If starting the IPC event loop fails after a socket has been opened, the socket descriptor is never closed, and if it fails after a child process has been spawned, the child keeps running and is never reaped. The report is accurate and its fixes are correct. On master the effect is small: every caller either exits after the error or is a test-only RPC, and the natural trigger is running out of threads or descriptors. Bug page: Failed IPC startup leaks the socket descriptor and the spawned child.

The report

IPC setup acquires resources in stages before an object exists that will clean them up. Process::bind() and Process::connect() return raw socket descriptors, and CapnpProtocol::listen() and makeStream() start the event loop before handing the descriptor to Cap'n Proto. If the loop fails to start, the error reaches the caller but the descriptor stays open. listen() also leaks it when the listen(2) system call fails.

The report later added two more stages. SpawnProcess() in libmultiprocess creates a socket pair before it builds the child's argument list, and leaks the pair if that throws. And after a child has been forked, Bitcoin Core starts the loop and creates the client object for the child; if that fails, nothing kills or waits for the child, so it keeps running and stays in the process table until the parent exits.

The report rates it low: one or two descriptors, or one child process, per failed attempt, which repeated retries could accumulate. No IPC message, peer-to-peer path or persistent effect is involved.

Proposed change

Close the descriptor when loop startup or listen(2) fails, close the socket pair in SpawnProcess() when building the arguments fails, and stop and reap the child when setup fails after the fork, then pass the original error on:

     mp::Stream makeStream(mp::SocketId socket) override
     {
-        startLoop();
+        try {
+            startLoop();
+        } catch (...) {
+            (void)!::close(socket);
+            throw;
+        }
         return mp::MakeStream(*m_loop, socket);
     }

Unit tests inject a loop-startup failure and check that the descriptors are closed. A Linux-only test makes thread creation really fail by lowering the process limit right after the child is forked, then checks that no descriptors leaked and the child was reaped. A libmultiprocess test makes the argument callback throw. The report considered a move-only descriptor type throughout the API (cleaner, but a broader change).

Assessment

Accurate. In src/ipc/capnp/protocol.cpp, listen() and makeStream() call startLoop() with the raw descriptor in hand and do not close it on failure, and in src/ipc/interfaces.cpp spawnProcess() only arranges to wait for the child once the client object exists. The SpawnProcess() leak is still present in libmultiprocess and was already noted by an earlier review (AR-12).

On master, loop startup fails with an exception mainly when the operating system refuses to create the loop thread. A failure inside the new thread aborts the process instead (AR-50), so fixing AR-50 makes these leaks reachable more often, and the two fixes belong together. The practical effect today is small: if bitcoin-node -ipcbind fails here the node exits, bitcoin-cli exits after its one command, and spawning a child is only used by the hidden test-only echoipc RPC. The planned multiprocess setup, where bitcoin-gui starts bitcoin-node and the node starts bitcoin-wallet, would make the spawn path real.

The fixes read as correct, though they were not run here. The thread-limit test is clever but heavy, since it wraps fork() for the whole test_bitcoin binary and is skipped when run as root; the descriptor cases can be covered with an in-process injected failure. The SpawnProcess() part belongs in libmultiprocess.

Status

Present on master. Next step: a Bitcoin Core PR with the descriptor and child cleanup, together with the AR-50 fix, and the SpawnProcess() part in libmultiprocess. No public issue or PR yet.

Reported privately in an independent security review on 2026-09-27.