[v92-audit] exec-if: EXEC with more than 256 conditions reads uninitialised keyReference entries and segfaults

[madolson]

EXEC with more than 256 conditions returns a key list whose first 256 entries are uninitialised heap memory, which crashes the server. execGetKeys in src/multi.c:232 grows the result one key at a time with getKeysPrepareResult(result, numkeys + 1) but only publishes result->numkeys after the loop ends (src/multi.c:251). getKeysPrepareResult uses result->numkeys to decide how much of the 256-entry stack buffer to copy when it moves to the heap (src/db.c:2374), so at the 257th condition it allocates a fresh buffer and copies nothing, leaving keys[0..255].pos as whatever was in that heap block. The memory-prefetch path then reads argv[result.keys[i].pos] with that garbage index at src/memory_prefetch.c:360 and dereferences the result, so any client that can send EXEC with 257 or more conditions can segfault the server.

Details

Repro

Clean server at 3f9062e, no config, default user. The ECHO calls only fill the heap block with a known byte so the garbage index is reliably far out of range; the bug is present without them, it just crashes less often.

import socket
s = socket.create_connection(('127.0.0.1', 7496))
def send(*a):
    out = b'*%d\r\n' % len(a)
    for x in a:
        x = x.encode() if isinstance(x, str) else x
        out += b'$%d\r\n%s\r\n' % (len(x), x)
    s.sendall(out)

n = 257
for _ in range(20):
    send('ECHO', 'A' * (n * 8)); s.recv(65536)
    send('ECHO', 'A' * (n * 8 - 8)); s.recv(65536)
    send('ECHO', 'A' * (n * 8 + 8)); s.recv(65536)

send('MULTI'); send('SET', 'x', '1')
args = []
for i in range(n):
    args += ['NX', 'k%d' % i]
send('EXEC', *args)
print(s.recv(65536))   # b'' -- server is gone

Three fresh servers, three crashes (two at n=257, one at n=260 on the next loop iteration):

--- fresh server run 1 (port 7496)
n=257 -> SERVER CLOSED CONNECTION (EOF)
ping: Could not connect to Valkey at 127.0.0.1:7496: Connection refused
--- fresh server run 2 (port 7497)
n=257 EXEC -> ['OK']
n=260 -> SERVER CLOSED CONNECTION (EOF)
ping: Could not connect to Valkey at 127.0.0.1:7497: Connection refused
--- fresh server run 3 (port 7498)
n=257 EXEC -> ['OK']
n=260 -> SERVER CLOSED CONNECTION (EOF)
ping: Could not connect to Valkey at 127.0.0.1:7498: Connection refused

=== VALKEY BUG REPORT START: Cut & paste starting from here ===
15364:M 16 Sep 2026 06:32:57.459 # valkey 255.255.255 crashed by signal: 11, si_code: 1
15364:M 16 Sep 2026 06:32:57.459 # Accessing address: 0x7f17fc968080
15364:M 16 Sep 2026 06:32:57.459 # Crashed running the instruction at: 0x4f7472

------ STACK TRACE ------
15364 valkey-server *
/lib64/libpthread.so.0(+0x118e0)[0x7f17ad45a8e0]
/home/matolson/v92audit/build-plain/src/valkey-server *:7491[0x4f7472]
/home/matolson/v92audit/build-plain/src/valkey-server *:7491(processInputBuffer+0x4ad)[0x50188d]
/home/matolson/v92audit/build-plain/src/valkey-server *:7491(readQueryFromClient+0xaa)[0x5019da]
/home/matolson/v92audit/build-plain/src/valkey-server *:7491[0x5be7af]
/home/matolson/v92audit/build-plain/src/valkey-server *:7491(aeMain+0x10e)[0x48303e]
/home/matolson/v92audit/build-plain/src/valkey-server *:7491(main+0x5d7)[0x460637]

0x4f7472 is inside addKeysToIncrFindBatch (symbol base 0x4f7360), which is the inlined body of addCommandToBatch in src/memory_prefetch.c.

Deterministic view of the same bug

COMMAND GETKEYS shows the uninitialised entries directly. At 256 conditions the whole list lives in getKeysResult.keysbuf and is correct; at 257 the first 256 pos values are garbage, here all zero, so they resolve to argv[0]:

n=255 -> ['k0', 'k1'] ... ['k254'] (len 255)
n=256 -> ['k0', 'k1'] ... ['k255'] (len 256)
n=257 -> ['EXEC', 'EXEC'] ... ['k256'] (len 257)
n=258 -> ['EXEC', 'EXEC'] ... ['k257'] (len 258)
n=300 -> ['EXEC', 'EXEC'] ... ['k299'] (len 300)

The garbage also reaches ACL, so a key-restricted user gets a transaction denied that should be allowed. Every condition key matches ~k*:

n=256 EXEC(lim user) -> RUN
n=257 EXEC(lim user) -> EXECABORT Transaction discarded because of: NOPERM No permissions to access a key
n=300 EXEC(lim user) -> EXECABORT Transaction discarded because of: NOPERM No permissions to access a key
n=1000 EXEC(lim user) -> EXECABORT Transaction discarded because of: NOPERM No permissions to access a key

In cluster mode the same list feeds slot routing, so it can also produce a wrong CROSSSLOT verdict or route on a garbage slot.

Why

src/multi.c:238-252:

    while (index < argc) {
        int key_index = index + 1;
        execCondition condition;

        if (parseExecCondition(argv, argc, &index, &condition) != C_OK) {
            result->numkeys = 0;
            return 0;
        }
        keys = getKeysPrepareResult(result, numkeys + 1);
        keys[numkeys].pos = key_index;
        keys[numkeys].flags = CMD_KEY_RO | CMD_KEY_ACCESS;
        numkeys++;
    }
    result->numkeys = numkeys;

initGetKeysResult sets result->size = MAX_KEYS_BUFFER (256, src/server.h:2535) and result->numkeys = 0. On the 257th iteration getKeysPrepareResult takes the "we are using a static buffer" branch and guards the copy on result->numkeys, which is still 0:

            result->keys = zmalloc(numkeys * sizeof(keyReference));
            if (result->numkeys) memcpy(result->keys, result->keysbuf, result->numkeys * sizeof(keyReference));

src/db.c:2373-2374. Every other incremental caller keeps result->numkeys in step as it appends, for example getKeysUsingKeySpecs at src/db.c:2474.

Fix

Publish numkeys inside the loop:

         keys[numkeys].pos = key_index;
         keys[numkeys].flags = CMD_KEY_RO | CMD_KEY_ACCESS;
         numkeys++;
+        result->numkeys = numkeys;
     }
     result->numkeys = numkeys;

With only that change applied at 3f9062e, two fresh servers survive the repro at n = 257 through 2000, COMMAND GETKEYS with 257 conditions returns first3=['k0', 'k1', 'k2'] last=['k256'] len=257, and the ~k* user's 257-condition EXEC returns ['OK'].

Alternatively count the conditions in a first pass and size the result once, which also drops the O(n) reallocs.

Introduced by

#4019, 0adf657af9327f906430ab526e5cbe326ba5410a, which added execGetKeys. EXEC had no keys before it.

Testing

tests/unit/multi.tcl and tests/unit/acl-v2.tcl from #4019 only use one or two conditions, so nothing crosses the 256-entry buffer. A case with 257 conditions is what is missing.

This was generated by AI but verified, with love, by a human.

[madolson]

Scope correction on reachability, after re-testing at 3f9062e.

The crash needs a heap that has already been used. On a freshly started server zmalloc hands back zeroed pages, so pos is 0, COMMAND GETKEYS returns 256 copies of EXEC (argv[0]) and nothing faults. After any real workload it faults in the reply builder:

valkey 255.255.255 crashed by signal: 11, si_code: 1
Accessing address: 0x7f2e18d0e010
EIP: getKeysSubcommandImpl+0x1b3

Repro: start a server, run any churn (40 rounds of a Lua loop setting and deleting 200 keys is enough), then COMMAND GETKEYS EXEC IFEQ k0 v0 ... with 257 or more conditions.

Plain MULTI/EXEC does not reach it. With the same dirtied heap and 300 conditions, standalone replies *-1 and cluster-enabled replies -CROSSSLOT, neither crashing, so the ACL and slot-routing consumers of the result are not affected the way the reply builder is.

No ACL bypass. A user limited to ~k* gets NOPERM with the denied key at condition 0, at condition 260, and at 256 conditions. With pos 0 the check reads argv[0], which matches no pattern, so it fails closed. Whether a non-zero pos can fail open is heap dependent and I could not construct it.

COMMAND is not privilege-gated by default, and cluster-aware clients and proxies call COMMAND GETKEYS to route commands they do not parse themselves.

[madolson]

Round 3 has a working AddressSanitizer build, so here is a deterministic ASAN confirmation plus a shorter repro that needs no MULTI at all.

COMMAND GETKEYS EXEC NX k0 ... NX k256 kills the server. 257 is the exact threshold, and both COMMAND GETKEYS and COMMAND GETKEYSANDFLAGS reach it. This matters for reachability: COMMAND is not key-permission checked, so any client that can run COMMAND GETKEYS can crash the server without holding permission on any key and without opening a transaction.

The EXEC path itself crashes only when io-threads is greater than 1, because prefetchCommandQueueKeys is what consumes the garbage pos. With io-threads 1 the 257-condition EXEC returns a normal reply and the corruption stays latent in ACL and cluster-slot decisions.

Details

Repro 1: COMMAND GETKEYS, no MULTI, default config

ASAN_OPTIONS=detect_leaks=0:abort_on_error=0:log_path=/tmp/asan_b \
  build-asan/src/valkey-server --port 7491 --logfile /tmp/srv_b.log --dir /tmp --daemonize yes

Then COMMAND GETKEYS EXEC followed by NX k<i> repeated n times:

n=256 gk  -> A len=256 first3=[b'k0', b'k1', b'k2'] last=[b'k255']
  ping: ('S', 'PONG')
n=257 gk  -> EOF (server died)
n=257 gkf -> EOF (server died)
n=258 gk  -> EOF (server died)
n=258 gkf -> EOF (server died)

gk is COMMAND GETKEYS, gkf is COMMAND GETKEYSANDFLAGS.

Verbatim ASAN report for n=257 COMMAND GETKEYS:

=================================================================
==3521==ERROR: AddressSanitizer: SEGV on unknown address 0x620df5f82700 (pc 0x00000070c55c bp 0x7fff515c4e90 sp 0x7fff515c45b0 T0)
==3521==The signal is caused by a READ memory access.
    #0 0x70c55b in getKeysSubcommandImpl src/server.c:5896
    #1 0x70c7c1 in getKeysSubcommand src/server.c:5914
    #2 0x70e985 in commandGetKeysCommand src/server.c:6150
    #3 0x704621 in call src/server.c:4226
    #4 0x708704 in processCommand src/server.c:5023
    #5 0x62c2b6 in processCommandAndResetClient src/networking.c:4356
    #6 0x63bab1 in processInputBuffer src/networking.c:4685
    #7 0x647ab3 in readQueryFromClient src/networking.c:4839
    #8 0x7259ff in callHandler src/connhelpers.h:79
    #9 0x7259ff in connSocketEventHandler src/socket.c:318
   #10 0x483963 in aeFireFileEvent src/ae.c:480
   #11 0x483963 in aeProcessEvents src/ae.c:620
   #12 0x48449e in aeMain src/ae.c:663
   #13 0x71d2fb in main src/server.c:8238

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV src/server.c:5896 in getKeysSubcommandImpl
==3521==ABORTING

COMMAND GETKEYSANDFLAGS lands one line later, at src/server.c:5899:

==4893==ERROR: AddressSanitizer: SEGV on unknown address 0x620df5f81300 (pc 0x00000070c6b3 bp 0x7fffb3a184c0 sp 0x7fffb3a17be0 T0)
SUMMARY: AddressSanitizer: SEGV src/server.c:5899 in getKeysSubcommandImpl

Repro 2: the EXEC path, requires io-threads > 1

build-asan/src/valkey-server --port 7492 --io-threads 3 ...

('A', [('b', b'io-threads'), ('b', b'3')])
n=256 EXEC -> ('A', [('S', 'OK')])
  ping ('S', 'PONG')
n=257 EOF

==9406==ERROR: AddressSanitizer: SEGV on unknown address 0x620df5f826f0 (pc 0x00000061afcc bp 0x7ffd42dbc180 sp 0x7ffd42dbb8b0 T0)
    #0 0x61afcb in addKeysToIncrFindBatch src/networking.c:4561
    #1 0x63b992 in prefetchCommandQueueKeys src/networking.c:4586
    #2 0x63b992 in processInputBuffer src/networking.c:4659
    #3 0x647ab3 in readQueryFromClient src/networking.c:4839
SUMMARY: AddressSanitizer: SEGV src/networking.c:4561 in addKeysToIncrFindBatch

With io-threads 1, the same n=257 and n=258 EXEC return ('A', [('S', 'OK')]) and the server stays up, so a maintainer reproducing this on a default config must use COMMAND GETKEYS, not EXEC.

Still unfixed on unstable

$ git log --oneline 3f9062ed5..unstable -- src/multi.c
(empty)

src/multi.c:246-251 on unstable is unchanged: getKeysPrepareResult(result, numkeys + 1) inside the loop, result->numkeys = numkeys only after it, so the 256-entry keysbuf contents are never copied into the heap buffer that getKeysPrepareResult allocates on the 257th append.

This was generated by AI but verified, with love, by a human.