diff --git a/include/ghostty/vt/terminal.h b/include/ghostty/vt/terminal.h index 433308c84..0e6d048e1 100644 --- a/include/ghostty/vt/terminal.h +++ b/include/ghostty/vt/terminal.h @@ -573,6 +573,25 @@ typedef enum GHOSTTY_ENUM_TYPED { * Input type: bool* */ GHOSTTY_TERMINAL_OPT_KITTY_IMAGE_MEDIUM_SHARED_MEM = 18, + + /** + * Set the maximum bytes the APC handler will buffer for all protocols. + * This prevents malicious input from causing unbounded memory allocation. + * A NULL value pointer removes all overrides, reverting to the built-in + * defaults. + * + * Input type: size_t* + */ + GHOSTTY_TERMINAL_OPT_APC_MAX_BYTES = 19, + + /** + * Set the maximum bytes the APC handler will buffer for Kitty graphics + * protocol data. A NULL value pointer removes the override, reverting + * to the built-in default. + * + * Input type: size_t* + */ + GHOSTTY_TERMINAL_OPT_APC_MAX_BYTES_KITTY = 20, GHOSTTY_TERMINAL_OPT_MAX_VALUE = GHOSTTY_ENUM_MAX_VALUE, } GhosttyTerminalOption; diff --git a/src/terminal/apc.zig b/src/terminal/apc.zig index 3cbadb8e0..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, @@ -46,6 +58,7 @@ pub const Handler = struct { .kitty => |*p| if (comptime build_options.kitty_graphics) { p.feed(byte) catch |err| { log.warn("kitty graphics protocol error: {}", .{err}); + p.deinit(); self.state = .ignore; }; } else unreachable, @@ -106,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 @@ -169,6 +196,41 @@ test "Kitty command with overflow i32" { try testing.expect(h.end() == null); } +test "kitty feed error deinits parser" { + if (comptime !build_options.kitty_graphics) return error.SkipZigTest; + + const testing = std.testing; + const alloc = testing.allocator; + + // Feed a valid kitty command start to allocate parser state, then + // trigger an error during feed via an integer overflow. The testing + // allocator will detect leaks if deinit is not called. + var h: Handler = .{}; + defer h.deinit(); + h.start(); + for ("Ga=p,i=10000000000;") |c| h.feed(alloc, c); + 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/c/terminal.zig b/src/terminal/c/terminal.zig index 0931b1685..670c2aea3 100644 --- a/src/terminal/c/terminal.zig +++ b/src/terminal/c/terminal.zig @@ -7,6 +7,7 @@ const ZigTerminal = @import("../Terminal.zig"); const Stream = @import("../stream_terminal.zig").Stream; const ScreenSet = @import("../ScreenSet.zig"); const PageList = @import("../PageList.zig"); +const apc = @import("../apc.zig"); const kitty = @import("../kitty/key.zig"); const kitty_gfx_c = @import("kitty_graphics.zig"); const modes = @import("../modes.zig"); @@ -310,6 +311,8 @@ pub const Option = enum(c_int) { kitty_image_medium_file = 16, kitty_image_medium_temp_file = 17, kitty_image_medium_shared_mem = 18, + apc_max_bytes = 19, + apc_max_bytes_kitty = 20, /// Input type expected for setting the option. pub fn InType(comptime self: Option) type { @@ -331,6 +334,7 @@ pub const Option = enum(c_int) { .kitty_image_medium_temp_file, .kitty_image_medium_shared_mem, => ?*const bool, + .apc_max_bytes, .apc_max_bytes_kitty => ?*const usize, }; } }; @@ -425,6 +429,19 @@ fn setTyped( } } }, + .apc_max_bytes => { + wrapper.stream.handler.apc_handler.max_bytes = if (value) |ptr| + .initFull(ptr.*) + else + .{}; + }, + .apc_max_bytes_kitty => { + if (value) |ptr| { + wrapper.stream.handler.apc_handler.max_bytes.put(.kitty, ptr.*); + } else { + wrapper.stream.handler.apc_handler.max_bytes.remove(.kitty); + } + }, } return .success; } 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";