From 33cc75ab1565d9dcbe808354191bd572ad6b64d0 Mon Sep 17 00:00:00 2001 From: Frank Denis Date: Thu, 16 Apr 2026 23:00:13 +0200 Subject: [PATCH] Fix: `sodium_misuse()` callback runs under global lock and can deadlock SUMMARY `src/libsodium/sodium/core.c` invokes the process-global misuse callback from `sodium_misuse()` while still holding the library-wide critical section. If the application-installed callback re-enters any API path that acquires the same lock, including `sodium_set_misuse_handler()`, execution deadlocks before `abort()` is reached. This breaks the intended fail-stop behavior of misuse handling. PROVENANCE Verified from the provided finding, reproduced locally from the committed control flow, and documented for Swival Security Scanner (https://swival.dev). PRECONDITIONS - A caller installs a misuse handler via `sodium_set_misuse_handler()`. - The handler re-enters an API path that takes the same global critical section, including `sodium_set_misuse_handler()`. PROOF 1. `sodium_set_misuse_handler()` writes the global `_misuse_handler` under `sodium_crit_enter()` / `sodium_crit_leave()` in `src/libsodium/sodium/core.c:170` and `src/libsodium/sodium/core.c:174`. 2. `sodium_misuse()` acquires that same critical section, copies `_misuse_handler`, and invokes `handler()` before releasing the lock in `src/libsodium/sodium/core.c:155` and `src/libsodium/sodium/core.c:160`. 3. The pthread-backed critical section in this file is non-recursive (`src/libsodium/sodium/core.c:89`), so a callback that calls `sodium_set_misuse_handler()` blocks in `sodium_crit_enter()` waiting on the lock already held by `sodium_misuse()`. 4. Because `sodium_misuse()` is waiting for the callback to return, it never reaches `abort()`, converting a fail-stop misuse path into a hang. 5. Reproduction confirmed this with a minimal pthread harness: the handler printed `handler: before reentry` and then hung until terminated by `timeout`. WHY THIS IS A REAL BUG The callback target is application-controlled through an exported setter, and `sodium_misuse()` calls it on a misuse path without enforcing any non-reentrancy contract. On pthread targets, a handler that performs a supported API call can permanently block process termination. That is a reachable behavioral failure, not a theoretical lock-order concern. FIX REQUIREMENT Load `_misuse_handler` while holding the lock, release the critical section, and only then invoke the callback. This removes lock-dependent behavior from arbitrary user code while preserving synchronized access to the global handler pointer. --- src/libsodium/sodium/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsodium/sodium/core.c b/src/libsodium/sodium/core.c index 214ef5f8..bd35b6ae 100644 --- a/src/libsodium/sodium/core.c +++ b/src/libsodium/sodium/core.c @@ -193,7 +193,7 @@ sodium_misuse(void) (void) sodium_crit_leave(); if (sodium_crit_enter() == 0) { handler = _misuse_handler; - if (handler != NULL) { + if (sodium_crit_leave() == 0 && handler != NULL) { handler(); } }