mirror of
https://github.com/oven-sh/bun
synced 2026-02-16 22:01:47 +00:00
fix(valkey): enforce max nesting depth in RESP parser to prevent stack overflow
A malicious Redis/Valkey server could send deeply nested RESP structures (e.g., arrays nested 100,000+ levels deep) causing unbounded recursion in readValue() and crashing the process via stack overflow (SIGSEGV). This adds a maximum nesting depth of 64 levels, which is far beyond any legitimate use case while preventing stack exhaustion. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -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 = .{
|
||||
|
||||
80
test/regression/issue/5928.test.ts
Normal file
80
test/regression/issue/5928.test.ts
Normal file
@@ -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);
|
||||
});
|
||||
Reference in New Issue
Block a user