terminal: clamp cursor left above scroll region with XTREVWRAP2

Fixes a crash found through fuzzing. This crash is also exhibited in
xterm (as of v384). The issue arises when you set the cursor above the top
scroll margin, then issue a large cursor left (CSI D) with extended reverse
wrap (?1045) set. Extended reverse wrap loops back until it reaches the top
scroll then wraps around. But since the cursor is before the top scroll,
xterm just arbitrarily moves back into negative row numbers, which reads into
bad memory.

We decided to fix this by clamping to (0,0) and exiting because this
will mimic the xterm behavior for valid values of cursor left count
(prior to crashing).
This commit is contained in:
Mitchell Hashimoto
2023-10-26 22:38:33 -07:00
parent be46bea40f
commit 9c165ecbd2
2 changed files with 31 additions and 5 deletions

View File

@ -1469,6 +1469,19 @@ pub fn cursorLeft(self: *Terminal, count_req: usize) void {
const row = self.screen.getRow(.{ .active = self.screen.cursor.y - 1 });
if (!row.isWrapped()) break;
}
// UNDEFINED TERMINAL BEHAVIOR. This situation is not handled in xterm
// and currently results in a crash in xterm. Given no other known
// terminal [to me] implements XTREVWRAP2, I decided to just mimick
// the behavior of xterm up and not including the crash by wrapping
// up to the (0, 0) and stopping there. My reasoning is that for an
// appropriately sized value of "count" this is the behavior that xterm
// would have. This is unit tested.
if (self.screen.cursor.y == 0) {
assert(self.screen.cursor.x == 0);
break;
}
self.screen.cursor.y -= 1;
self.screen.cursor.x = right_margin;
count -= 1;
@ -6125,6 +6138,22 @@ test "Terminal: cursorLeft extended reverse wrap is priority if both set" {
}
}
test "Terminal: cursorLeft extended reverse wrap above top scroll region" {
const alloc = testing.allocator;
var t = try init(alloc, 5, 5);
defer t.deinit(alloc);
t.modes.set(.wraparound, true);
t.modes.set(.reverse_wrap_extended, true);
t.setTopAndBottomMargin(3, 0);
t.setCursorPos(2, 1);
t.cursorLeft(1000);
try testing.expectEqual(@as(usize, 0), t.screen.cursor.x);
try testing.expectEqual(@as(usize, 0), t.screen.cursor.y);
}
test "Terminal: cursorDown basic" {
const alloc = testing.allocator;
var t = try init(alloc, 5, 5);

View File

@ -60,6 +60,8 @@ pub fn Stream(comptime Handler: type) type {
for (actions) |action_opt| {
const action = action_opt orelse continue;
// log.info("action: {}", .{action});
// If this handler handles everything manually then we do nothing
// if it can be processed.
if (@hasDecl(T, "handleManually")) {
@ -75,11 +77,6 @@ pub fn Stream(comptime Handler: type) type {
if (processed) continue;
}
// if (action_opt) |action| {
// if (action != .print)
// log.info("action: {}", .{action});
// }
switch (action) {
.print => |p| if (@hasDecl(T, "print")) try self.handler.print(p),
.execute => |code| try self.execute(code),