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.
Calling update after squeezing is undocumented and non standard,
but if an application still decides to do it, permute the state
before absorbing so that it's still safe to do so.
We can easily do it since we keep track of the state.
Still return an error as this is not the expected usage of SHAKE,
and zeroing the state is another thing we could do.
Frank Denis