| 描述 | I discovered a server-side memory corruption vulnerability in Valkey 9.1.0 in the module timer subsystem. The issue is triggered when a module timer callback stops its own currently firing timer by calling ValkeyModule_StopTimer() on the same timer ID that is being dispatched by the core. Under this condition, the timer object is freed inside VM_StopTimer(), but the outer dispatcher in moduleTimerHandler() still assumes that the object remains valid until callback return and frees it again. The result is a reproducible double free / invalid free in the valkey-server process.
The affected logic is in src/module.c, primarily around moduleTimerHandler(), VM_CreateTimer(), and VM_StopTimer(). The core data structure involved is the global Timers radix tree that stores ValkeyModuleTimer objects keyed by timer ID. A newly created timer is allocated by the core in VM_CreateTimer(), inserted into Timers, and identified by the returned 64-bit timer ID. When the timer expires, moduleTimerHandler() retrieves the corresponding ValkeyModuleTimer * from the radix tree iterator, constructs a ValkeyModuleCtx, and invokes timer->callback(&ctx, timer->data). The vulnerability appears because moduleTimerHandler() does not account for the possibility that the callback may destroy the currently executing timer through another public core API before the dispatcher resumes execution.
My local analysis shows the following execution model. First, a module command arms a short-lived timer using ValkeyModule_CreateTimer() and stores the returned ID inside callback data. When the timer expires, moduleTimerHandler() begins dispatching it. Inside the callback, the module calls ValkeyModule_StopTimer(ctx, saved_id, NULL) where saved_id is exactly the ID returned at timer creation time. VM_StopTimer() performs a lookup in Timers, finds the active timer object, verifies that timer->module matches ctx->module, removes the object from the radix tree, and immediately frees the ValkeyModuleTimer allocation. Control then returns to the callback and then back to moduleTimerHandler(). However, moduleTimerHandler() still holds the original timer pointer that it captured before invoking the callback, and it unconditionally executes its own cleanup path after callback return. Because the timer was already removed and freed inside VM_StopTimer(), this second cleanup turns into a second free of the same heap object.
The key reason this is a real core vulnerability rather than a simple bug in my test module is that the module does not free any core timer memory directly. It only invokes the documented public API ValkeyModule_StopTimer() on a timer that belongs to the same module. The object that is freed twice is the core-owned ValkeyModuleTimer allocated by VM_CreateTimer(). Therefore the root cause is in Valkey’s lifetime management and dispatcher assumptions, not in module-side manual memory handling. In other words, the outer framework does not track current-object self-destruction during callback execution.
I reproduced the issue against the official src/valkey-server binary compiled from the Valkey 9.1.0 source tree with AddressSanitizer enabled. The server was launched normally and a minimal local module was loaded through --loadmodule. The module exposes a command that arms a timer with a 1 ms delay. The callback then immediately calls ValkeyModule_StopTimer() on its own ID. Triggering the command causes the server to abort within the official event loop. It is a direct consequence of the core calling free on the same timer object twice.
The sanitizer evidence is clear and consistent. In the crashing run, the second free is reported from moduleTimerHandler() in src/module.c after the callback returns. The earlier free of the same allocation is reported from VM_StopTimer() reached through the timer callback. The original allocation is reported from VM_CreateTimer(). This produces a complete alloc -> free -> free lifecycle on the same ValkeyModuleTimer object. In my local output, ASan reports “attempting to call malloc_usable_size() for pointer which is not owned”, with the stack trace pointing to valkey_free() and then moduleTimerHandler() for the second invalid free. The “freed by” stack points to VM_StopTimer() called from the callback. The “previously allocated by” stack points to VM_CreateTimer() called by the module command that armed the timer. This is fully consistent with a double free / invalid free condition.
I also validated the root cause with GDB and real runtime values. During timer creation, my module stored the returned timer ID into a callback data structure. At the callback breakpoint, the saved ID matched the ID returned at creation time, and ctx->module matched the module instance that created the timer. At the VM_StopTimer() breakpoint, the looked-up result pointer resolved to the same ValkeyModuleTimer object being dispatched by moduleTimerHandler(). After the callback returned, I inspected state again in moduleTimerHandler() and observed that the dispatcher still held the same timer pointer, while a fresh lookup in Timers no longer found the entry. That combination is decisive: the owning container no longer contains the timer, but the dispatcher still treats the stale pointer as live and proceeds to free it. This is the exact window that creates the double free.
From a security perspective, the most conservative and defensible impact is a reliable server-side denial of service. The crash happens inside the official valkey-server process, not inside an external test harness process. Once the vulnerable path is exercised, the server aborts. I am not claiming remote code execution or stronger exploitation impact at this stage because my current evidence is centered on repeatable process termination under ASan. However, the bug is still a genuine memory safety issue in the core. It is not merely a harmless assertion failure or a module-local logic mistake. A double free in a core server component is security-relevant even if the current proof focuses on crash impact.
The attack surface must be described precisely. This issue requires that the server load a module that uses the timer API and exposes a reachable path to the vulnerable callback pattern. It is therefore not equivalent to an unauthenticated bug reachable in a stock server without modules. At the same time, this limitation should not minimize the bug. The vulnerability exists in Valkey core and is triggered through its public module API. A module author can reasonably expect that calling ValkeyModule_StopTimer() on a timer owned by the same module will not corrupt the server process. The current implementation violates that expectation when the timer being stopped is the one currently executing.
The core code pattern that creates the problem can be summarized as follows. moduleTimerHandler() fetches ValkeyModuleTimer *timer from the Timers iterator, calls timer->callback(&ctx, timer->data), and then unconditionally performs post-callback cleanup that includes removing and freeing the timer. Separately, VM_StopTimer() resolves the supplied timer ID through Timers, removes the entry, and frees the timer immediately. No state bit, no deferred reclamation mechanism, and no post-callback liveness revalidation prevent these two paths from acting on the same object in one dispatch cycle. In short, the dispatcher assumes exclusive ownership of the timer object until callback completion, but the stop path can destroy that ownership behind its back.
Based on my review, a proper fix should make timer destruction self-stop aware. One reasonable approach is to detach the timer from the owning radix tree before invoking the callback, so that callback-time stop operations cannot free the same object out from under the dispatcher. Another acceptable approach is to introduce an in-flight flag or a deferred-free mechanism so that VM_StopTimer() marks the current timer as canceled without freeing it immediately when it is actively being dispatched. After the callback returns, moduleTimerHandler() should check whether the timer was already canceled or reclaimed and avoid a second free. Any fix should ensure that there is only one owner responsible for final reclamation of the current timer object during a callback dispatch cycle.
In summary, this vulnerability is a reproducible double free / invalid free in Valkey 9.1.0. I discovered it in the official valkey-server process by exercising the public module timer API with a minimal reproducer module. The issue is rooted in src/module.c and is caused by a lifetime-management flaw between moduleTimerHandler() and VM_StopTimer(). A callback can stop and free its own active timer, but the outer dispatcher does not recognize that the current timer object has already been destroyed and frees it again. The practical effect is a reliable server crash, and the bug is significant because it occurs in Valkey core rather than only in third-party module code. |
|---|