Redis wrapper: unbounded RESP array recursion overflows the stack pre-auth (~47 KB); array size also unvalidated

[ruispereira]

Summary

The Redis wrapper parses an incoming RESP message with unbounded recursion, before authentication. A ~47 KB message of nested arrays overflows the stack and kills the connection thread with StackOverflowError. No credentials are needed.

Version: 26.8.1. Module redisw.

The recursion

// redisw/src/main/java/com/arcadedb/redis/RedisNetworkExecutor.java:751
} else if (b == '*') {
  // ARRAY
  final List<Object> array = new ArrayList<>();
  final int arraySize = Integer.parseInt(parseValueUntilLF());
  for (int i = 0; i < arraySize; ++i)
    array.add(parseNext());        // recursive; nesting depth = stack depth
  return array;
}

A RESP array element can itself be an array, so *1\r\n repeated N times nests N deep, one parseNext() frame per level. arraySize is never bounded.

Pre-authentication

receiveCommand() parses the whole message via parseNext() before the AUTH check runs:

// RedisNetworkExecutor.java:159
if ("AUTH".equals(cmdString)) { … }
if ("HELLO".equals(cmdString)) { … }
if (authenticatedUser == null) {
  value.append("-NOAUTH Authentication required.");   // reached only AFTER the message is fully parsed
  …
}

The overflow happens inside parsing, so the NOAUTH gate never gets a chance to reject the message. Anyone who can open the Redis port reaches it.

Measured against the real parser

Driven through the actual RedisNetworkExecutor.parseNext() over a loopback socket (not a re-implementation), bisected on Temurin 21.0.3 with the default stack:

nesting depth	result
11 860	parsed
11 861	StackOverflowError

Payload at the threshold: 47 451 bytes (*1\r\n × 11 861 + $1\r\nx\r\n).

final StringBuilder sb = new StringBuilder();
for (int i = 0; i < 11_861; i++) sb.append("*1\r\n");
sb.append("$1\r\nx\r\n");
// feed sb to a socket the RedisNetworkExecutor reads from -> StackOverflowError in parseNext()

Second, related problem: unvalidated array size

Even without nesting, arraySize is trusted:

final int arraySize = Integer.parseInt(parseValueUntilLF());
for (int i = 0; i < arraySize; ++i)
  array.add(parseNext());

A header *2000000000\r\n sets up a two-billion-iteration loop, each iteration blocking on parseNext() for bytes the client can dribble out slowly — a cheap way to tie up a connection thread indefinitely. A negative arraySize (e.g. *-5) is a different matter: the loop body is skipped, so that one is benign, but the value should still be validated.

The batch-string path ($) is safe by contrast: parseChars(size) reads byte-by-byte into a reused buffer without pre-allocating size bytes, so a large $ length is a long read, not an allocation spike.

Impact

• Reachable pre-auth by anyone who can connect to the Redis port.
• StackOverflowError is an Error; it unwinds the connection handler. Whether it also disturbs shared state depends on where it lands, which is not something a client should get to choose.
• 47 KB per hit, repeatable across connections.

Suggested fix

• Cap nesting depth: thread a depth counter through parseNext() and reject past a small maximum (Redis itself limits multi-bulk nesting).
• Bound arraySize against a configured maximum and reject negatives explicitly, before entering the loop.
• Consider converting the array parse to an explicit work stack rather than call recursion, so depth is bounded by memory rather than by the JVM stack.

This is the same class as the Bolt frame-length issue (#5894): an unauthenticated wire parser trusting a client-supplied size. A single "validate sizes before you act on them" pass over the protocol wrappers would cover both.