Audit report AR-17

Reported privately in an independent security review, 2026-08-13.

Report date: 2026-08-13
Kind: Bug
Status on master: Present
Proposed issue state: Open
Linked bugs: Bug 81: Node crashes when a client calls a method on an object it already destroyed
Bug 82: Destroying an object during a running call frees it under the call

Summary

For interfaces that define a destroy method (Bitcoin Core's BlockTemplate and Echo), destroy resets the server's implementation pointer without a lock, while the capability itself stays usable. A client that calls a method after destroy makes bitcoin-node call through a null pointer, and a client that calls destroy while another of its threads has a call in progress frees the implementation under that call. The libmultiprocess C++ client cannot do either, but a Python or Rust client can. Real bugs, open, reachable only by a misbehaving client.

The report

The report describes two problems in libmultiprocess's server dispatch, as bundled in Bitcoin Core:

Ordinary method dispatch reads the implementation through a raw pointer (m_impl.get()), while the generated destroy handler resets the shared_ptr that owns it (m_impl.reset()), possibly on another worker thread. The same shared_ptr is accessed from both threads with no synchronization.
After destroy, the capability still exists, so a later call dereferences the cleared pointer.

It adds that return values are serialized after the implementation method returns, so keeping the object alive only for the duration of the method call would not be enough. It states that the attack needs access to an operator-enabled local IPC socket, which already grants trusted RPC authority, that the problem is established from source, and that no crash of a stock node was demonstrated. It found the code path in releases v30.0 and v31.1.

The report's first test exercised the proposed lease API directly and did not reproduce either crash. A later revision (October 2026) added functional tests that drive both cases over real IPC through BlockTemplate: the report says a release build crashes with SIGSEGV on a call after destroy, and an AddressSanitizer build shows the use-after-free when destroy runs during waitNext. Those runs were not repeated for this page; the survey's own reproductions show the same two failures in mptest.

Proposed change

The patch adds a lease that each call holds on the implementation, from dispatch through result serialization:

 struct ServerInvokeContext : InvokeContext
 {
+    //! Keeps the wrapped implementation alive through result serialization.
+    std::shared_ptr<void> impl_lease;
@@ ProxyServerBase::acquireImpl()
+    const std::lock_guard lock{m_lease_state->mutex};
+    if (!m_impl) throw std::runtime_error("IPC method called after object destruction");
+    auto lease = std::make_shared<ImplLease>(m_lease_state, m_impl);
+    ++m_lease_state->count;
@@ void ProxyServerBase<Interface, Impl>::invokeDestroy()
-    m_impl.reset();
+    std::shared_ptr<Impl> impl;
+    {
+        const std::lock_guard lock{m_lease_state->mutex};
+        if (!m_impl) throw std::runtime_error("IPC object already destroyed");
+        if (m_lease_state->count > 0) throw std::runtime_error("IPC object has calls in progress");
+        impl = std::move(m_impl);
+    }
+    impl.reset();
@@ ProxyMethodTraits::invoke
-        return (server_context.proxy_server.m_impl.get()->*ProxyMethod<MethodParams>::impl)(std::forward<Args>(args)...);
+        auto lease = server_context.proxy_server.acquireImpl();
+        server_context.impl_lease = lease;
+        return (lease->m_impl.get()->*ProxyMethod<MethodParams>::impl)(std::forward<Args>(args)...);

acquireImpl takes a counted lease under a mutex and throws if the object was already destroyed.
invokeDestroy throws if the object is already destroyed or if any lease is outstanding, and otherwise moves the implementation out under the lock before releasing it.
invoke holds the lease in the server invoke context, so it lives until results are built.

The lease counter has its own shared ownership, so releasing a lease never touches the server object. A new KJ test, "Synchronize proxy implementation destruction", takes a lease, checks that destroy fails with "calls in progress", releases it, destroys, and checks that later calls and a second destroy throw. The report considered making destroy wait for active calls instead, and rejected it because that needs a cancellation and deadlock contract; rejecting lets the client retry. It says the change belongs in libmultiprocess before a subtree import.

Assessment

Both mechanisms are present on libmultiprocess master: ProxyServerBase::invokeDestroy() is m_impl.reset(); CleanupRun(m_context.cleanup_fns); with no lock, and ProxyMethodTraits::invoke in include/mp/proxy.h calls m_impl.get()->*impl with no null check. destroy is defined in Bitcoin Core's mining.capnp (BlockTemplate) and echo.capnp.

They reproduce differently, so the survey tracks them as two bugs:

Call after destroy. The libmultiprocess C++ client only sends destroy from ~ProxyClientBase, so it can never call afterwards. A Python or Rust mining client that calls destroy() on a BlockTemplate and keeps using the capability, for example from a stale task, would crash bitcoin-node deterministically. Bug page: bitcoin-node dereferences a null implementation pointer and crashes when an IPC client calls a method on an object it has already destroyed.
Destroy during a call. BlockTemplate.destroy takes a Context, so it runs on a worker thread, while another client thread's call can be running on a different worker. A C++ client doing this would already have undefined behaviour on its own side, so only a misbehaving peer triggers it. Bug page: bitcoin-node frees an IPC object's implementation while another client thread's call is still running on it.

Under Bitcoin Core's trust model such a client already has unauthenticated RPC access, so this is robustness against buggy non-C++ clients rather than a security boundary. Rejecting destroy while calls are active changes client-visible behaviour, so the lease design needs review in libmultiprocess. A cheaper first step, useful on its own, is to throw an exception instead of dereferencing when the implementation is already gone.

Where the check goes matters. The proposed lease, and a null check placed in ProxyMethodTraits::invoke, only cover methods that use the generated default dispatch. Server methods with hand-written implementations read the implementation pointer directly and would keep the bug. Bitcoin Core master has none on interfaces that declare destroy, but pending multiprocess PRs add them, for example Chain.handleRpc in bitcoin#29409. Counting calls and checking for destroy in serverInvoke, which every request passes through, covers both kinds and also counts calls that are queued but not yet running.

Related: AR-02 concerns servers that never own their implementation, a different code path.

Reproduction (2026-09-26). Both mechanisms reproduced with tests against Bitcoin Core master bfdcd97 (libmultiprocess v13 subtree), using a server-owned test object and raw Cap'n Proto requests, as a Python or Rust client would send them. destroy followed by a call crashed the server with SIGSEGV, and UndefinedBehaviorSanitizer reported a member call on a null pointer (reproduction). A destroy sent on a second server thread while a call was held inside the implementation returned at once, and the running call then read freed memory: AddressSanitizer reported a heap-use-after-free, and a build without sanitizers returned a garbage value (reproduction).

Status

Open on libmultiprocess and Bitcoin Core master. Next step: a libmultiprocess change that at least rejects calls after destroy, and a design decision on synchronizing destroy with in-progress calls. Tracked in mp#378; no PR yet.

Reported privately in an independent security review on 2026-08-13. The call-after-destroy crash was later reported publicly in bitcoin#36502 (2026-10-11), found independently with fuzzamoto.