Skip to content

redisMacOSAttach: use-after-free if CF object creation fails and the caller then calls redisAsyncFree #1360

Description

@yayong3

Summary

redisMacOSAttach() publishes ev.data and ev.cleanup before it creates the CFSocket and the run-loop source. If either CoreFoundation call fails, it frees the RedisRunLoop object via freeRedisRunLoop() and returns REDIS_ERR, but it does not clear ev.data or the hook pointers.

The usual follow-up after a failed attach is redisAsyncFree(). That path always runs _EL_CLEANUP(ac), which calls redisMacOSCleanup(ev.data) on the pointer that was just freed.

This is one ownership bug with two equivalent failure returns:

Site adapters/macosx.h What failed
CFSocketCreateWithNative returns NULL 132 RedisRunLoop freed, ev.data still set
CFSocketCreateRunLoopSource returns NULL 135 same

The success path is fine. The crash is the error path: attach failed, so free the context.

Confirmed on 1.5.0-dev (29ea279) with AddressSanitizer, linking the system CoreFoundation framework (not a stub). A successful attach followed by redisAsyncFree does not fault. After an attach failure, the same redisAsyncFree is a heap-use-after-free.

Code

redisMacOSAttach installs the destructor before either CF object exists:

/* adapters/macosx.h */

redisAsyncCtx->ev.addRead  = redisMacOSAddRead;
redisAsyncCtx->ev.delRead  = redisMacOSDelRead;
redisAsyncCtx->ev.addWrite = redisMacOSAddWrite;
redisAsyncCtx->ev.delWrite = redisMacOSDelWrite;
redisAsyncCtx->ev.cleanup  = redisMacOSCleanup;
redisAsyncCtx->ev.data     = redisRunLoop;   /* published, :123 */

redisRunLoop->socketRef = CFSocketCreateWithNative(...);
if (!redisRunLoop->socketRef)
    return freeRedisRunLoop(redisRunLoop);   /* :132 */

redisRunLoop->sourceRef = CFSocketCreateRunLoopSource(...);
if (!redisRunLoop->sourceRef)
    return freeRedisRunLoop(redisRunLoop);   /* :135 */

freeRedisRunLoop releases the object and does not touch ac->ev:

static int freeRedisRunLoop(RedisRunLoop *redisRunLoop) {
    /* invalidate/release CF objects if non-NULL, then: */
    hi_free(redisRunLoop);   /* :58; ev.data still points here */
    return REDIS_ERR;
}

redisAsyncFree always runs the registered cleanup:

/* async.c — __redisAsyncFree */
_EL_CLEANUP(ac);   /* if (ev.cleanup) ev.cleanup(ev.data); */

Once the hooks are installed, _EL_CLEANUP is unconditional. A failed attach does not skip it.

Reproduce

macOS, unmodified tree at 29ea279. No Redis server is required: redisAsyncConnect is used only to obtain a native socket.

make USE_WERROR=0 static
clang -O0 -g -fsanitize=address -fno-omit-frame-pointer \
    -I. -o poc poc.c libhiredis.a -framework CoreFoundation
ASAN_OPTIONS='abort_on_error=1:halt_on_error=1' ./poc          # control
ASAN_OPTIONS='abort_on_error=1:halt_on_error=1' ./poc --fail   # error path
#include "hiredis.h"
#include "async.h"
#include "adapters/macosx.h"

#include <stdio.h>
#include <string.h>
#include <unistd.h>

int main(int argc, char **argv)
{
    int force_fail = (argc >= 2 && strcmp(argv[1], "--fail") == 0);
    redisAsyncContext *ac = redisAsyncConnect("127.0.0.1", 1);
    int rc;

    if (ac == NULL)
        return 1;

    if (force_fail && ac->c.fd >= 0) {
        close(ac->c.fd);
        ac->c.fd = REDIS_INVALID_FD;
    }

    rc = redisMacOSAttach(ac, CFRunLoopGetCurrent());
    fprintf(stderr, "attach rc=%d fd=%d ev.data=%p cleanup=%p\n",
            rc, ac->c.fd, ac->ev.data, (void *)ac->ev.cleanup);

    redisAsyncFree(ac);
    fprintf(stderr, "redisAsyncFree returned\n");
    return force_fail ? 1 : 0;
}

--fail invalidates the native socket so that CoreFoundation refuses to finish creating the run-loop source. That is only a way to enter the existing error returns; the UAF is in hiredis, not in CoreFoundation.

Observed

Apple Silicon, macOS 26.6.2 (25G83), Apple clang 21.0.0, hiredis 29ea279, system -framework CoreFoundation.

Control (./poc), live fd:

connect fd=3 err=0
attach rc=0 fd=3 ev.data=0x603000005950
redisAsyncFree returned

Exit 0. No AddressSanitizer report. Happy-path attach and teardown are clean.

Error path (./poc --fail), fd set to REDIS_INVALID_FD (−1):

attach rc=-1 fd=-1 ev.data=0x603000005950 cleanup=0x1020a58c8

redisMacOSAttach returned REDIS_ERR, but ev.data was still the pointer that freeRedisRunLoop had just freed. redisAsyncFree then aborted:

==85697==ERROR: AddressSanitizer: heap-use-after-free on address 0x603000005960
READ of size 8 at 0x603000005960 thread T0
    #0 0x1020a59bc in freeRedisRunLoop macosx.h:50
    #1 0x1020a58e4 in redisMacOSCleanup macosx.h:85
    #2 0x1020ae0d8 in __redisAsyncFree async.c:392
    #3 0x1020a4d20 in main poc.c:52

0x603000005960 is located 16 bytes inside of 24-byte region [0x603000005950,0x603000005968)
freed by thread T0 here:
    #0 free
    #1 hi_free alloc.h:79
    #2 freeRedisRunLoop macosx.h:58
    #3 redisMacOSAttach macosx.h:135
    #4 main

previously allocated by thread T0 here:
    #0 calloc
    #1 hi_calloc alloc.h:67
    #2 redisMacOSAttach macosx.h:111
    #3 main

SUMMARY: AddressSanitizer: heap-use-after-free macosx.h:50 in freeRedisRunLoop

On this system CFSocketCreateWithNative did not return NULL for fd −1. The failure was the next call, CFSocketCreateRunLoopSource (macosx.h:135). The destructor is the same for both returns: ev.data is published at :123, freeRedisRunLoop at :58 does not clear it, _EL_CLEANUP at async.c:392 calls redisMacOSCleanup on the freed object.

The 24-byte region matches sizeof(RedisRunLoop) (three pointers). The use-after-free address is 16 bytes inside that region (sourceRef).

Suggested fix

Do not publish ev.data / cleanup until both CF objects exist, or clear them on the failure returns so _EL_CLEANUP is a no-op.

Minimal change on the existing returns:

--- a/adapters/macosx.h
+++ b/adapters/macosx.h
@@ -129,10 +129,16 @@ static int redisMacOSAttach(...)
                                                        redisMacOSAsyncCallback,
                                                        &socketCtx);
-        if( !redisRunLoop->socketRef ) return freeRedisRunLoop(redisRunLoop);
+        if( !redisRunLoop->socketRef ) {
+            redisAsyncCtx->ev.cleanup = NULL;
+            redisAsyncCtx->ev.data = NULL;
+            return freeRedisRunLoop(redisRunLoop);
+        }

     redisRunLoop->sourceRef = CFSocketCreateRunLoopSource(NULL, redisRunLoop->socketRef, 0);
-        if( !redisRunLoop->sourceRef ) return freeRedisRunLoop(redisRunLoop);
+        if( !redisRunLoop->sourceRef ) {
+            redisAsyncCtx->ev.cleanup = NULL;
+            redisAsyncCtx->ev.data = NULL;
+            return freeRedisRunLoop(redisRunLoop);
+        }

Moving the ev.* assignments to after both creates is cleaner: a failure then only needs freeRedisRunLoop, and redisAsyncFree will not invoke a macOS cleanup hook.

Environment

  • redis/hiredis 29ea2791c102fdffff350f1ea6e04d1f71903d0d (1.5.0-dev)
  • Adapter: adapters/macosx.h (unmodified)
  • macOS 26.6.2 (25G83), arm64
  • Apple clang 21.0.0 (clang-2100.1.1.101)
  • AddressSanitizer: -O0 -g -fsanitize=address, libclang_rt.asan_osx_dynamic.dylib

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions