mirror of
https://github.com/valkey-io/valkey.git
synced 2026-05-06 05:26:42 -04:00
Fix UAF in unblockClientOnKey when reprocessed command frees the client (CVE-2026-23479) (#3616)
When a blocked client is woken up on a key, unblockClientOnKey() re-executes its pending command via processCommandAndResetClient(). That function returns C_ERR when the client was freed as a side effect of the re-execution (it detects this by server.current_client becoming NULL inside freeClient()). The pre-fix code ignored the return value and unconditionally accessed c->flag.blocked / c->flag.module afterwards -- a use-after-free on the freed client. The fix mirrors the existing dead-client handling in processPendingCommandAndInputBuffer() (src/networking.c): on C_ERR, unwind the execution unit, restore server.current_client, and return without touching c. Note on testing: reaching the C_ERR branch requires the reprocessed command to synchronously free the current client (for example a replica/primary-link teardown or a module path that calls freeClient()). None of the blocking commands routed through unblockClientOnKey (BLPOP/BLMOVE/BRPOPLPUSH/BLMPOP/ XREADGROUP and friends) take that path from a plain RESP client, so a Tcl-level integration reproducer is not feasible without adding a test-only debug hook; this patch does not add such a hook. A targeted gtest reproducer was also considered but rejected as net-negative: it would require ~200 lines of fake-client scaffolding to exercise a 7-line defensive guard and would become a "change detector" as warned against in src/unit/README.md. The existing tests/unit/type/list.tcl suite (288 tests) exercises the happy path of this function and continues to pass unchanged. Running the suite under AddressSanitizer (make SANITIZER=address) provides the strongest signal against regressions of this class of bug. Signed-off-by: ikolomi <ikolomin@amazon.com> Co-authored-by: ikolomi <ikolomin@amazon.com>
This commit is contained in:
+7
-1
@@ -705,7 +705,13 @@ static void unblockClientOnKey(client *c, robj *key) {
|
||||
client *old_client = server.current_client;
|
||||
server.current_client = c;
|
||||
enterExecutionUnit(1, 0);
|
||||
processCommandAndResetClient(c);
|
||||
if (processCommandAndResetClient(c) == C_ERR) {
|
||||
/* Client was freed during command processing, exit immediately */
|
||||
exitExecutionUnit();
|
||||
server.current_client = old_client;
|
||||
return;
|
||||
}
|
||||
|
||||
if (!c->flag.blocked) {
|
||||
if (c->flag.module) {
|
||||
moduleCallCommandUnblockedHandler(c);
|
||||
|
||||
Reference in New Issue
Block a user