Audit report AR-59

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

Report date

2026-09-27

Kind

Hardening

Status on master

Present

Proposed issue state

Open

Summary

When a server method finishes on its worker thread, libmultiprocess hands the result back by running a short function on the event loop that resolves the call's promise. The report shows that if that function threw, the event loop would drop the error and the client call would never get its result. The mechanism is real, but nothing in that function can throw before the result is delivered; only the report's injected test hook reaches it. Robustness hardening, not a bug that can be hit today.

The report

ProxyServer<Thread>::post() runs the server method on a worker thread and then calls EventLoop::sync() with a function that fulfills or rejects the promise the client's call is waiting on. If that function throws, the event loop logs the exception and returns normally, and the promise is left unresolved, so the client waits forever for a method that has already run.

The report lists allocation, conversion, promise fulfillment, logging and task registration as possible triggers. The reproduction is a Bitcoin Core unit test that adds an asynchronous method to the test interface, makes a testing hook throw std::bad_alloc at the start of the delivery function, and checks that the client gets that error within five seconds and that the next call succeeds. The report rates it low: one blocked client call, with no message from a peer, peer-to-peer path, stored state or funds involved.

Proposed change

Catch an exception in the delivery function on the event loop thread and reject the still-pending promise with it (comments and assertions omitted):

-                cancel_monitor_ptr = nullptr;
-                KJ_IF_MAYBE(e, exception) {
-                    result_fulfiller->reject(kj::mv(*e));
-                } else {
-                    result_fulfiller->fulfill(kj::mv(*result_value));
+                KJ_IF_MAYBE(delivery_error, kj::runCatchingExceptions([&] {
+                    cancel_monitor_ptr = nullptr;
+                    KJ_IF_MAYBE(e, exception) {
+                        result_fulfiller->reject(kj::mv(*e));
+                    } else {
+                        result_fulfiller->fulfill(kj::mv(*result_value));
+                    }
+                    result_fulfiller = nullptr;
+                })) {
+                    cancel_monitor_ptr = nullptr;
+                    if (result_fulfiller.get()) {
+                        result_fulfiller->reject(kj::mv(*delivery_error));
+                        result_fulfiller = nullptr;
+                    }
                 }

The report considered having the event loop's generic error handler record the failure, which would couple unrelated callers of sync().

Assessment

The code is as described on libmultiprocess master and in Bitcoin Core's copy, but the listed triggers do not apply to it. Before the result is delivered, the function only destroys a small cancellation-tracking object and calls fulfill or reject, and none of these steps allocate or can throw. Converting the method's results happens earlier, on the worker thread, and an error there is already passed to the client as a rejected call. Nothing is logged in this function. The one allocation, scheduling the release of a reference to the thread object, comes after the result has been delivered, and the Bitcoin Core node and GUI terminate on allocation failure anyway.

Reading the code, the failure would also look a little different from the report's description. If the function did throw early, the objects it holds would be destroyed afterwards on the worker thread, and kj rejects a promise whose fulfiller is destroyed unfulfilled. That rejection would come from the wrong thread without waking the event loop, so a test with nothing else to wake the loop sees a hang, while a busy process might see the error later. The proposed change avoids this by handling the error on the event loop thread, and it looks correct, though it was not run here. It is reasonable defensive code; it is not needed to fix a reachable bug.

Related reports on the same pattern, where an exception thrown inside EventLoop::sync() is logged and dropped, did find reachable cases: AR-57 (socket setup) and AR-58 (request setup). A change that makes sync() rethrow on the calling thread would not cover this one, because here the calling thread is a worker with no handler for it.

Status

Present on master. Hardening; no public issue or PR yet.

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