diff --git a/src/valkey/valkey_protocol.zig b/src/valkey/valkey_protocol.zig index 7c6f12f8a6..b39de75cc5 100644 --- a/src/valkey/valkey_protocol.zig +++ b/src/valkey/valkey_protocol.zig @@ -26,6 +26,7 @@ pub const RedisError = error{ UnsupportedProtocol, ConnectionTimeout, IdleTimeout, + NestingTooDeep, }; pub fn valkeyErrorToJS(globalObject: *jsc.JSGlobalObject, message: ?[]const u8, err: RedisError) jsc.JSValue { @@ -52,6 +53,7 @@ pub fn valkeyErrorToJS(globalObject: *jsc.JSGlobalObject, message: ?[]const u8, error.InvalidCommand => .REDIS_INVALID_COMMAND, error.InvalidArgument => .REDIS_INVALID_ARGUMENT, error.UnsupportedProtocol => .REDIS_INVALID_RESPONSE, + error.NestingTooDeep => .REDIS_INVALID_RESPONSE, error.InvalidResponseType => .REDIS_INVALID_RESPONSE_TYPE, error.ConnectionTimeout => .REDIS_CONNECTION_TIMEOUT, error.IdleTimeout => .REDIS_IDLE_TIMEOUT, @@ -340,6 +342,10 @@ pub const ValkeyReader = struct { buffer: []const u8, pos: usize = 0, + /// Maximum allowed nesting depth for recursive RESP types (Array, Map, Set, etc.) + /// to prevent stack overflow from malicious deeply-nested responses. + const max_nesting_depth = 64; + pub fn init(buffer: []const u8) ValkeyReader { return .{ .buffer = buffer, @@ -421,6 +427,10 @@ pub const ValkeyReader = struct { } pub fn readValue(self: *ValkeyReader, allocator: std.mem.Allocator) RedisError!RESPValue { + return self.readValueDepth(allocator, 0); + } + + fn readValueDepth(self: *ValkeyReader, allocator: std.mem.Allocator, depth: usize) RedisError!RESPValue { const type_byte = try self.readByte(); return switch (RESPType.fromByte(type_byte) orelse return error.InvalidResponseType) { @@ -451,6 +461,7 @@ pub const ValkeyReader = struct { return RESPValue{ .BulkString = owned }; }, .Array => { + if (depth >= max_nesting_depth) return error.NestingTooDeep; const len = try self.readInteger(); if (len < 0) return RESPValue{ .Array = &[_]RESPValue{} }; const array = try allocator.alloc(RESPValue, @as(usize, @intCast(len))); @@ -462,7 +473,7 @@ pub const ValkeyReader = struct { } } while (i < len) : (i += 1) { - array[i] = try self.readValue(allocator); + array[i] = try self.readValueDepth(allocator, depth + 1); } return RESPValue{ .Array = array }; }, @@ -495,6 +506,7 @@ pub const ValkeyReader = struct { return RESPValue{ .VerbatimString = try self.readVerbatimString(allocator) }; }, .Map => { + if (depth >= max_nesting_depth) return error.NestingTooDeep; const len = try self.readInteger(); if (len < 0) return error.InvalidMap; @@ -508,11 +520,12 @@ pub const ValkeyReader = struct { } while (i < len) : (i += 1) { - entries[i] = .{ .key = try self.readValue(allocator), .value = try self.readValue(allocator) }; + entries[i] = .{ .key = try self.readValueDepth(allocator, depth + 1), .value = try self.readValueDepth(allocator, depth + 1) }; } return RESPValue{ .Map = entries }; }, .Set => { + if (depth >= max_nesting_depth) return error.NestingTooDeep; const len = try self.readInteger(); if (len < 0) return error.InvalidSet; @@ -525,11 +538,12 @@ pub const ValkeyReader = struct { } } while (i < len) : (i += 1) { - set[i] = try self.readValue(allocator); + set[i] = try self.readValueDepth(allocator, depth + 1); } return RESPValue{ .Set = set }; }, .Attribute => { + if (depth >= max_nesting_depth) return error.NestingTooDeep; const len = try self.readInteger(); if (len < 0) return error.InvalidAttribute; @@ -542,9 +556,9 @@ pub const ValkeyReader = struct { } } while (i < len) : (i += 1) { - var key = try self.readValue(allocator); + var key = try self.readValueDepth(allocator, depth + 1); errdefer key.deinit(allocator); - const value = try self.readValue(allocator); + const value = try self.readValueDepth(allocator, depth + 1); attrs[i] = .{ .key = key, .value = value }; } @@ -553,7 +567,7 @@ pub const ValkeyReader = struct { errdefer { allocator.destroy(value_ptr); } - value_ptr.* = try self.readValue(allocator); + value_ptr.* = try self.readValueDepth(allocator, depth + 1); return RESPValue{ .Attribute = .{ .attributes = attrs, @@ -561,11 +575,12 @@ pub const ValkeyReader = struct { } }; }, .Push => { + if (depth >= max_nesting_depth) return error.NestingTooDeep; const len = try self.readInteger(); if (len < 0 or len == 0) return error.InvalidPush; // First element is the push type - const push_type = try self.readValue(allocator); + const push_type = try self.readValueDepth(allocator, depth + 1); var push_type_str: []const u8 = ""; switch (push_type) { @@ -594,7 +609,7 @@ pub const ValkeyReader = struct { } } while (i < len - 1) : (i += 1) { - data[i] = try self.readValue(allocator); + data[i] = try self.readValueDepth(allocator, depth + 1); } return RESPValue{ .Push = .{ diff --git a/test/regression/issue/5928.test.ts b/test/regression/issue/5928.test.ts new file mode 100644 index 0000000000..c218a4c8fb --- /dev/null +++ b/test/regression/issue/5928.test.ts @@ -0,0 +1,80 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe } from "harness"; + +// Test for security issue: ValkeyReader.readValue() unbounded recursion +// A malicious Redis/Valkey server can send deeply nested RESP structures +// that exhaust the call stack and crash the Bun process via stack overflow. + +test("valkey RESP parser should reject deeply nested responses", async () => { + // Create a malicious server that sends deeply nested RESP arrays + // Each "*1\r\n" is a RESP array of length 1, nesting into the next level + const nestingDepth = 100_000; + + using server = Bun.listen({ + hostname: "127.0.0.1", + port: 0, + socket: { + open(socket) { + // Do nothing on open - wait for data + }, + data(socket, data) { + const request = Buffer.from(data).toString(); + + // Respond to HELLO (RESP3 protocol negotiation) with a simple OK + if (request.includes("HELLO")) { + socket.write("+OK\r\n"); + return; + } + + // For any other command (e.g., GET), send a deeply nested RESP array + // *1\r\n repeated nestingDepth times, then a leaf value + let response = ""; + for (let i = 0; i < nestingDepth; i++) { + response += "*1\r\n"; + } + response += "$3\r\nfoo\r\n"; + socket.write(response); + }, + close() {}, + error(socket, err) {}, + }, + }); + + const port = server.port; + + // Use a subprocess so a crash doesn't take down the test runner + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const client = new Bun.RedisClient("redis://127.0.0.1:${port}"); + try { + const result = await client.send("GET", ["test"]); + // If we get here without crashing, the parser handled it + console.log("RESULT:" + JSON.stringify(result)); + } catch (e) { + console.log("ERROR:" + e.message); + } finally { + client.close(); + } + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + // The process should NOT crash (exit code 0 or a handled error) + // Before the fix: process crashes with stack overflow (signal 11/SIGSEGV or similar) + // After the fix: parser returns an error about nesting depth exceeded + if (exitCode !== 0) { + console.log("stdout:", stdout); + console.log("stderr:", stderr); + console.log("exitCode:", exitCode); + } + expect(stdout).toContain("ERROR:"); + expect(exitCode).toBe(0); +});