terminal: fix csi parsing (#8417)

Make `MAX_PARAMS` public and increase CSI parameter limit from 16 to 24.
Fix potential out-of-bounds read in SGR partial sequence extraction.

Related discussion:
https://github.com/ghostty-org/ghostty/discussions/5198

DISCLAIMER: the tests were written with Claude Code's help.
pull/8427/head
Mitchell Hashimoto 2025-08-27 07:20:16 -07:00 committed by GitHub
commit 5013d028a3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 73 additions and 5 deletions

View File

@ -193,7 +193,7 @@ pub const Action = union(enum) {
/// Maximum number of intermediate characters during parsing. This is
/// 4 because we also use the intermediates array for UTF8 decoding which
/// can be at most 4 bytes.
const MAX_INTERMEDIATE = 4;
pub const MAX_INTERMEDIATE = 4;
/// Maximum number of CSI parameters. This is arbitrary. Practically, the
/// only CSI command that uses more than 3 parameters is the SGR command
@ -206,7 +206,7 @@ const MAX_INTERMEDIATE = 4;
/// number. I implore TUI authors to not use more than this number of CSI
/// params, but I suspect we'll introduce a slow path with heap allocation
/// one day.
const MAX_PARAMS = 24;
pub const MAX_PARAMS = 24;
/// Current state of the state machine
state: State,
@ -949,6 +949,55 @@ test "csi: too many params" {
}
}
test "csi: sgr with up to our max parameters" {
for (1..MAX_PARAMS + 1) |max| {
var p = init();
_ = p.next(0x1B);
_ = p.next('[');
for (0..max - 1) |_| {
_ = p.next('1');
_ = p.next(';');
}
_ = p.next('2');
{
const a = p.next('H');
try testing.expect(p.state == .ground);
try testing.expect(a[0] == null);
try testing.expect(a[1].? == .csi_dispatch);
try testing.expect(a[2] == null);
const csi = a[1].?.csi_dispatch;
try testing.expectEqual(@as(usize, max), csi.params.len);
try testing.expectEqual(@as(u16, 2), csi.params[max - 1]);
}
}
}
test "csi: sgr beyond our max drops it" {
// Has to be +2 for the loops below
const max = MAX_PARAMS + 2;
var p = init();
_ = p.next(0x1B);
_ = p.next('[');
for (0..max - 1) |_| {
_ = p.next('1');
_ = p.next(';');
}
_ = p.next('2');
{
const a = p.next('H');
try testing.expect(p.state == .ground);
try testing.expect(a[0] == null);
try testing.expect(a[1] == null);
try testing.expect(a[2] == null);
}
}
test "dcs: XTGETTCAP" {
var p = init();
_ = p.next(0x1B);

View File

@ -134,7 +134,7 @@ pub const Parser = struct {
self.idx += 1;
return .{ .unknown = .{
.full = self.params,
.partial = slice[0 .. self.idx - start + 1],
.partial = slice[0..@min(self.idx - start + 1, slice.len)],
} };
},
};

View File

@ -249,7 +249,7 @@ pub fn Stream(comptime Handler: type) type {
// the parser state to ground.
0x18, 0x1A => self.parser.state = .ground,
// A parameter digit:
'0'...'9' => if (self.parser.params_idx < 16) {
'0'...'9' => if (self.parser.params_idx < Parser.MAX_PARAMS) {
self.parser.param_acc *|= 10;
self.parser.param_acc +|= c - '0';
// The parser's CSI param action uses param_acc_idx
@ -259,7 +259,7 @@ pub fn Stream(comptime Handler: type) type {
self.parser.param_acc_idx |= 1;
},
// A parameter separator:
':', ';' => if (self.parser.params_idx < 16) {
':', ';' => if (self.parser.params_idx < Parser.MAX_PARAMS) {
self.parser.params[self.parser.params_idx] = self.parser.param_acc;
if (c == ':') self.parser.params_sep.set(self.parser.params_idx);
self.parser.params_idx += 1;
@ -2601,3 +2601,22 @@ test "stream CSI ? W reset tab stops" {
try s.nextSlice("\x1b[?1;2;3W");
try testing.expect(s.handler.reset);
}
test "stream: SGR with 17+ parameters for underline color" {
const H = struct {
attrs: ?sgr.Attribute = null,
called: bool = false,
pub fn setAttribute(self: *@This(), attr: sgr.Attribute) !void {
self.attrs = attr;
self.called = true;
}
};
var s: Stream(H) = .init(.{});
// Kakoune-style SGR with underline color as 17th parameter
// This tests the fix where param 17 was being dropped
try s.nextSlice("\x1b[4:3;38;2;51;51;51;48;2;170;170;170;58;2;255;97;136;0m");
try testing.expect(s.handler.called);
}