From 0509f00ad2f0e56dc4c0807d2e22f80baf1688f9 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 20 Apr 2026 08:48:05 -0700 Subject: [PATCH] terminal/apc: introduce a max_bytes parameter to prevent DoS --- src/terminal/apc.zig | 49 +++++++++++++++++++++- src/terminal/kitty/graphics_command.zig | 54 ++++++++++++++----------- 2 files changed, 78 insertions(+), 25 deletions(-) diff --git a/src/terminal/apc.zig b/src/terminal/apc.zig index 5c025fbb3..7e6f08a7a 100644 --- a/src/terminal/apc.zig +++ b/src/terminal/apc.zig @@ -12,6 +12,14 @@ const log = std.log.scoped(.terminal_apc); pub const Handler = struct { state: State = .inactive, + /// Maximum bytes each APC protocol can buffer. This is to prevent + /// malicious input from causing us to allocate too much memory. + /// If you want to be lazy and set a single value for all protocols, + /// use `.initFull`. + max_bytes: std.EnumMap(Protocol, usize) = .initFullWith(.{ + .kitty = Protocol.defaultMaxBytes(.kitty), + }), + pub fn deinit(self: *Handler) void { self.state.deinit(); } @@ -34,7 +42,11 @@ pub const Handler = struct { switch (byte) { // Kitty graphics protocol 'G' => self.state = if (comptime build_options.kitty_graphics) - .{ .kitty = kitty_gfx.CommandParser.init(alloc) } + .{ .kitty = .init( + alloc, + self.max_bytes.get(.kitty) orelse + Protocol.defaultMaxBytes(.kitty), + ) } else .ignore, @@ -107,8 +119,22 @@ pub const State = union(enum) { } }; +/// Possible APC command types. +pub const Protocol = enum { + kitty, + + /// Returns the default maximum bytes for the given protocol. + pub fn defaultMaxBytes(self: Protocol) usize { + return switch (self) { + // Kitty graphics payloads can be very large (e.g. full images + // encoded as base64), so the default is set to 65 MiB. + .kitty => 65 * 1024 * 1024, + }; + } +}; + /// Possible APC commands. -pub const Command = union(enum) { +pub const Command = union(Protocol) { kitty: if (build_options.kitty_graphics) kitty_gfx.Command else @@ -186,6 +212,25 @@ test "kitty feed error deinits parser" { try testing.expect(h.state == .ignore); } +test "kitty max bytes exceeded" { + if (comptime !build_options.kitty_graphics) return error.SkipZigTest; + + const testing = std.testing; + const alloc = testing.allocator; + + var h: Handler = .{ .max_bytes = .init(.{ .kitty = 4 }) }; + defer h.deinit(); + h.start(); + // 'G' identifies kitty, 'a=t;' moves to data state, then feed exceeds max_bytes. + for ("Ga=t;") |c| h.feed(alloc, c); + try testing.expect(h.state != .ignore); + for ("abcd") |c| h.feed(alloc, c); + try testing.expect(h.state != .ignore); + // The 5th data byte exceeds the 4-byte limit. + h.feed(alloc, 'e'); + try testing.expect(h.state == .ignore); +} + test "valid Kitty command" { if (comptime !build_options.kitty_graphics) return error.SkipZigTest; diff --git a/src/terminal/kitty/graphics_command.zig b/src/terminal/kitty/graphics_command.zig index d1f0e6b63..aa35fbeb9 100644 --- a/src/terminal/kitty/graphics_command.zig +++ b/src/terminal/kitty/graphics_command.zig @@ -38,6 +38,10 @@ pub const Parser = struct { /// here instead of a fixed buffer so that we can too. data: std.ArrayList(u8), + /// Maximum bytes the data payload can take. This is to prevent + /// malicious input from causing us to allocate too much memory. + max_bytes: usize, + /// Internal state for parsing. state: State, @@ -55,7 +59,7 @@ pub const Parser = struct { /// Initialize the parser. The allocator given will be used for both /// temporary data and long-lived values such as the final image blob. - pub fn init(alloc: Allocator) Parser { + pub fn init(alloc: Allocator, max_bytes: usize) Parser { var arena = ArenaAllocator.init(alloc); errdefer arena.deinit(); var result: Parser = .{ @@ -64,6 +68,7 @@ pub const Parser = struct { .kv = .{}, .kv_temp_len = 0, .kv_current = 0, + .max_bytes = max_bytes, .state = .control_key, .kv_temp = undefined, @@ -84,7 +89,7 @@ pub const Parser = struct { /// Parse a complete command string. pub fn parseString(alloc: Allocator, data: []const u8) !Command { - var parser = init(alloc); + var parser = init(alloc, 1024 * 1024); defer parser.deinit(); for (data) |c| try parser.feed(c); return try parser.complete(alloc); @@ -137,7 +142,10 @@ pub const Parser = struct { else => {}, }, - .data => try self.data.append(self.arena.child_allocator, c), + .data => { + if (self.data.items.len >= self.max_bytes) return error.OutOfMemory; + try self.data.append(self.arena.child_allocator, c); + }, } } @@ -964,7 +972,7 @@ pub const CompositionMode = enum { test "transmission command" { const testing = std.testing; const alloc = testing.allocator; - var p = Parser.init(alloc); + var p = Parser.init(alloc, 1024 * 1024); defer p.deinit(); const input = "f=24,s=10,v=20"; @@ -982,7 +990,7 @@ test "transmission command" { test "transmission ignores 'm' if medium is not direct" { const testing = std.testing; const alloc = testing.allocator; - var p = Parser.init(alloc); + var p = Parser.init(alloc, 1024 * 1024); defer p.deinit(); const input = "a=t,t=t,m=1"; @@ -999,7 +1007,7 @@ test "transmission ignores 'm' if medium is not direct" { test "transmission respects 'm' if medium is direct" { const testing = std.testing; const alloc = testing.allocator; - var p = Parser.init(alloc); + var p = Parser.init(alloc, 1024 * 1024); defer p.deinit(); const input = "a=t,t=d,m=1"; @@ -1016,7 +1024,7 @@ test "transmission respects 'm' if medium is direct" { test "query command" { const testing = std.testing; const alloc = testing.allocator; - var p = Parser.init(alloc); + var p = Parser.init(alloc, 1024 * 1024); defer p.deinit(); const input = "i=31,s=1,v=1,a=q,t=d,f=24;QUFBQQ"; @@ -1036,7 +1044,7 @@ test "query command" { test "display command" { const testing = std.testing; const alloc = testing.allocator; - var p = Parser.init(alloc); + var p = Parser.init(alloc, 1024 * 1024); defer p.deinit(); const input = "a=p,U=1,i=31,c=80,r=120"; @@ -1054,7 +1062,7 @@ test "display command" { test "delete command" { const testing = std.testing; const alloc = testing.allocator; - var p = Parser.init(alloc); + var p = Parser.init(alloc, 1024 * 1024); defer p.deinit(); const input = "a=d,d=p,x=3,y=4"; @@ -1074,7 +1082,7 @@ test "delete command" { test "no control data" { const testing = std.testing; const alloc = testing.allocator; - var p = Parser.init(alloc); + var p = Parser.init(alloc, 1024 * 1024); defer p.deinit(); const input = ";QUFBQQ"; @@ -1089,7 +1097,7 @@ test "no control data" { test "ignore unknown keys (long)" { const testing = std.testing; const alloc = testing.allocator; - var p = Parser.init(alloc); + var p = Parser.init(alloc, 1024 * 1024); defer p.deinit(); const input = "f=24,s=10,v=20,hello=world"; @@ -1107,7 +1115,7 @@ test "ignore unknown keys (long)" { test "ignore very long values" { const testing = std.testing; const alloc = testing.allocator; - var p = Parser.init(alloc); + var p = Parser.init(alloc, 1024 * 1024); defer p.deinit(); const input = "f=24,s=10,v=2000000000000000000000000000000000000000"; @@ -1125,7 +1133,7 @@ test "ignore very long values" { test "ensure very large negative values don't get skipped" { const testing = std.testing; const alloc = testing.allocator; - var p = Parser.init(alloc); + var p = Parser.init(alloc, 1024 * 1024); defer p.deinit(); const input = "a=p,i=1,z=-2000000000"; @@ -1142,7 +1150,7 @@ test "ensure very large negative values don't get skipped" { test "ensure proper overflow error for u32" { const testing = std.testing; const alloc = testing.allocator; - var p = Parser.init(alloc); + var p = Parser.init(alloc, 1024 * 1024); defer p.deinit(); const input = "a=p,i=10000000000"; @@ -1153,7 +1161,7 @@ test "ensure proper overflow error for u32" { test "ensure proper overflow error for i32" { const testing = std.testing; const alloc = testing.allocator; - var p = Parser.init(alloc); + var p = Parser.init(alloc, 1024 * 1024); defer p.deinit(); const input = "a=p,i=1,z=-9999999999"; @@ -1167,7 +1175,7 @@ test "all i32 values" { { // 'z' (usually z-axis values) - var p = Parser.init(alloc); + var p = Parser.init(alloc, 1024 * 1024); defer p.deinit(); const input = "a=p,i=1,z=-1"; for (input) |c| try p.feed(c); @@ -1182,7 +1190,7 @@ test "all i32 values" { { // 'H' (relative placement, horizontal offset) - var p = Parser.init(alloc); + var p = Parser.init(alloc, 1024 * 1024); defer p.deinit(); const input = "a=p,i=1,H=-1"; for (input) |c| try p.feed(c); @@ -1197,7 +1205,7 @@ test "all i32 values" { { // 'V' (relative placement, vertical offset) - var p = Parser.init(alloc); + var p = Parser.init(alloc, 1024 * 1024); defer p.deinit(); const input = "a=p,i=1,V=-1"; for (input) |c| try p.feed(c); @@ -1254,7 +1262,7 @@ test "response: encode with image ID and number" { test "delete range command 1" { const testing = std.testing; const alloc = testing.allocator; - var p = Parser.init(alloc); + var p = Parser.init(alloc, 1024 * 1024); defer p.deinit(); const input = "a=d,d=r,x=3,y=4"; @@ -1274,7 +1282,7 @@ test "delete range command 1" { test "delete range command 2" { const testing = std.testing; const alloc = testing.allocator; - var p = Parser.init(alloc); + var p = Parser.init(alloc, 1024 * 1024); defer p.deinit(); const input = "a=d,d=R,x=5,y=11"; @@ -1294,7 +1302,7 @@ test "delete range command 2" { test "delete range command 3" { const testing = std.testing; const alloc = testing.allocator; - var p = Parser.init(alloc); + var p = Parser.init(alloc, 1024 * 1024); defer p.deinit(); const input = "a=d,d=R,x=5,y=4"; @@ -1305,7 +1313,7 @@ test "delete range command 3" { test "delete range command 4" { const testing = std.testing; const alloc = testing.allocator; - var p = Parser.init(alloc); + var p = Parser.init(alloc, 1024 * 1024); defer p.deinit(); const input = "a=d,d=R,x=5"; @@ -1316,7 +1324,7 @@ test "delete range command 4" { test "delete range command 5" { const testing = std.testing; const alloc = testing.allocator; - var p = Parser.init(alloc); + var p = Parser.init(alloc, 1024 * 1024); defer p.deinit(); const input = "a=d,d=R,y=5";