diff --git a/src/terminal/c/kitty_graphics.zig b/src/terminal/c/kitty_graphics.zig index e47e6894f..c1c738a0b 100644 --- a/src/terminal/c/kitty_graphics.zig +++ b/src/terminal/c/kitty_graphics.zig @@ -631,9 +631,8 @@ fn computeViewportPos( // is above the viewport, or its top edge is at or below the // viewport's last row. const grid_size = p.gridSize(image.*, t); - const rows_i32: i32 = @intCast(grid_size.rows); - const term_rows: i32 = @intCast(t.rows); - const visible = vp_row + rows_i32 > 0 and vp_row < term_rows; + const bottom_row = @as(i64, vp_row) + @as(i64, grid_size.rows); + const visible = bottom_row > 0 and vp_row < @as(i32, t.rows); return .{ .col = vp_col, .row = vp_row, .visible = visible }; } @@ -1624,6 +1623,39 @@ test "placement_render_info returns all fields" { try testing.expectEqual(2, ri.source_height); } +test "placement_render_info handles maximum grid dimensions" { + if (comptime !build_options.kitty_graphics) return error.SkipZigTest; + + var t: terminal_c.Terminal = null; + try testing.expectEqual(Result.success, terminal_c.new( + &lib.alloc.test_allocator, + &t, + 80, + 24, + )); + defer terminal_c.free(t); + try testing.expectEqual(Result.success, terminal_c.resize(t, 80, 24, 10, 20)); + + const cmd = "\x1b_Ga=T,t=d,f=24,i=1,p=1,s=1,v=2,c=1,r=4294967295,C=1;////////\x1b\\"; + terminal_c.vt_write(t, cmd.ptr, cmd.len); + + var graphics: KittyGraphics = undefined; + try testing.expectEqual(Result.success, terminal_c.get(t, .kitty_graphics, @ptrCast(&graphics))); + const img = image_get_handle(graphics, 1); + try testing.expect(img != null); + + var iter: PlacementIterator = null; + try testing.expectEqual(Result.success, placement_iterator_new(&lib.alloc.test_allocator, &iter)); + defer placement_iterator_free(iter); + try testing.expectEqual(Result.success, get(graphics, .placement_iterator, @ptrCast(&iter))); + try testing.expect(placement_iterator_next(iter)); + + var ri: PlacementRenderInfo = .{}; + try testing.expectEqual(Result.success, placement_render_info(iter, img, t, &ri)); + try testing.expect(ri.viewport_visible); + try testing.expectEqual(std.math.maxInt(u32), ri.grid_rows); +} + test "placement_render_info off-screen sets viewport_visible false" { if (comptime !build_options.kitty_graphics) return error.SkipZigTest; diff --git a/src/terminal/kitty/graphics_exec.zig b/src/terminal/kitty/graphics_exec.zig index e0756ccc7..449e859f6 100644 --- a/src/terminal/kitty/graphics_exec.zig +++ b/src/terminal/kitty/graphics_exec.zig @@ -273,14 +273,21 @@ fn display( .after => { // We use terminal.index to properly handle scroll regions. const size = p.gridSize(img, terminal); - for (0..size.rows) |_| terminal.index() catch |err| { + // Once the requested movement leaves the screen, its exact + // position is undefined by the Kitty graphics protocol. Bound + // the work so an untrusted row count can't make us spin. + const rows_to_move: usize = @min( + @as(usize, size.rows), + @as(usize, terminal.rows), + ); + for (0..rows_to_move) |_| terminal.index() catch |err| { log.warn("failed to move cursor: {}", .{err}); break; }; terminal.setCursorPos( terminal.screens.active.cursor.y, - pin.x + size.cols + 1, + @as(usize, pin.x) +| @as(usize, size.cols) +| 1, ); }, }, @@ -673,3 +680,22 @@ test "kittygfx delete then retransmit same id gets fresh generation" { try testing.expect(gen2 > gen1); try testing.expect(gen2 > gen_delete); } + +test "kittygfx placement bounds cursor movement for untrusted dimensions" { + const testing = std.testing; + const alloc = testing.allocator; + const io = testing.io; + + var t = try Terminal.init(io, alloc, .{ .rows = 5, .cols = 5 }); + defer t.deinit(alloc); + + const cmd = try command.Parser.parseString( + alloc, + "a=T,t=d,f=24,i=1,s=1,v=1,c=4294967295,r=4294967295;////", + ); + defer cmd.deinit(alloc); + + const resp = execute(io, alloc, &t, &cmd).?; + try testing.expect(resp.ok()); + try testing.expectEqual(@as(usize, 1), t.screens.active.kitty_images.placements.count()); +} diff --git a/src/terminal/kitty/graphics_storage.zig b/src/terminal/kitty/graphics_storage.zig index 3db1cdde1..eb476c33a 100644 --- a/src/terminal/kitty/graphics_storage.zig +++ b/src/terminal/kitty/graphics_storage.zig @@ -849,6 +849,24 @@ pub const ImageStorage = struct { } } + /// Multiply two protocol-controlled values without allowing them to + /// wrap. Placement geometry is exposed as u32, so values larger than + /// that are represented by the largest possible value. + fn saturatingMul(lhs: u32, rhs: u32) u32 { + return std.math.mul(u32, lhs, rhs) catch std.math.maxInt(u32); + } + + /// Scale a dimension by an aspect ratio and round to the nearest + /// integer. The u64 intermediate can hold the product of two u32s as + /// well as the rounding adjustment. + fn scaleDimension(value: u32, numerator: u32, denominator: u32) u32 { + if (denominator == 0) return 0; + + const rounded = (@as(u64, value) * @as(u64, numerator) + + @as(u64, denominator) / 2) / @as(u64, denominator); + return std.math.cast(u32, rounded) orelse std.math.maxInt(u32); + } + /// Returns the size of this placement's image in pixels, /// taking into account the source rectangle, specified /// rows/columns, and aspect ratio. @@ -880,15 +898,12 @@ pub const ImageStorage = struct { const cell_width: u32 = t.width_px / t.cols; const cell_height: u32 = t.height_px / t.rows; - const width_f64: f64 = @floatFromInt(width); - const height_f64: f64 = @floatFromInt(height); - // If we have a specified cols AND rows then we calculate // the width and height from them directly, we don't need // to adjust for aspect ratio. if (self.columns > 0 and self.rows > 0) { - const calc_width = cell_width * self.columns; - const calc_height = cell_height * self.rows; + const calc_width = saturatingMul(cell_width, self.columns); + const calc_height = saturatingMul(cell_height, self.rows); return .{ .width = calc_width, @@ -902,11 +917,8 @@ pub const ImageStorage = struct { // If only the columns were specified, we determine // the height of the image based on the aspect ratio. if (self.columns > 0) { - const aspect = height_f64 / width_f64; - const calc_width: u32 = cell_width * self.columns; - const calc_height: u32 = @intFromFloat(@round( - @as(f64, @floatFromInt(calc_width)) * aspect, - )); + const calc_width = saturatingMul(cell_width, self.columns); + const calc_height = scaleDimension(calc_width, height, width); return .{ .width = calc_width, @@ -917,11 +929,8 @@ pub const ImageStorage = struct { // Otherwise, only the rows were specified, so we // determine the width based on the aspect ratio. { - const aspect = width_f64 / height_f64; - const calc_height: u32 = cell_height * self.rows; - const calc_width: u32 = @intFromFloat(@round( - @as(f64, @floatFromInt(calc_height)) * aspect, - )); + const calc_height = saturatingMul(cell_height, self.rows); + const calc_width = scaleDimension(calc_height, width, height); return .{ .width = calc_width, @@ -951,12 +960,12 @@ pub const ImageStorage = struct { return .{ .cols = std.math.divCeil( u32, - calc_size.width + self.x_offset, + calc_size.width +| self.x_offset, t.width_px / t.cols, ) catch 0, .rows = std.math.divCeil( u32, - calc_size.height + self.y_offset, + calc_size.height +| self.y_offset, t.height_px / t.rows, ) catch 0, }; @@ -965,8 +974,8 @@ pub const ImageStorage = struct { } /// Returns a selection of the entire rectangle this placement - /// occupies within the screen. This can return null if the placement - /// doesn't have an associated rect (i.e. a virtual placement). + /// occupies within the screen. This can return null for a virtual + /// placement or when unavailable pixel geometry makes it empty. pub fn rect( self: Placement, image: Image, @@ -978,17 +987,21 @@ pub const ImageStorage = struct { .virtual => return null, }; + // A zero pixel-sized placement can produce a zero grid size when + // pixel geometry is unavailable. It occupies no rectangle. + if (grid_size.cols == 0 or grid_size.rows == 0) return null; + var br = switch (pin.downOverflow(grid_size.rows - 1)) { .offset => |v| v, .overflow => |v| v.end, }; - br.x = @min( + br.x = @intCast(@min( // We need to sub one here because the x value is // one width already. So if the image is width "1" // then we add zero to X because X itself is width 1. - pin.x + (grid_size.cols - 1), - t.cols - 1, - ); + @as(u32, pin.x) +| (grid_size.cols - 1), + @as(u32, t.cols) - 1, + )); return .{ .top_left = pin.*, @@ -1587,6 +1600,100 @@ test "storage: aspect ratio calculation when only columns or rows specified" { } } +test "storage: placement geometry handles untrusted dimensions" { + const testing = std.testing; + const alloc = testing.allocator; + const io = testing.io; + const max = std.math.maxInt(u32); + + var t = try terminal.Terminal.init(io, alloc, .{ .cols = 2, .rows = 2 }); + defer t.deinit(alloc); + t.width_px = max; + t.height_px = max; + + // Cell dimensions multiplied by protocol-controlled row and column + // counts saturate instead of panicking or wrapping. + { + const placement: ImageStorage.Placement = .{ + .location = .{ .virtual = {} }, + .columns = 3, + .rows = 3, + }; + const actual = placement.pixelSize(.{ .width = 1, .height = 1 }, &t); + try testing.expectEqual(max, actual.width); + try testing.expectEqual(max, actual.height); + } + + // Aspect-ratio scaling also saturates when the derived dimension does + // not fit in the public u32 geometry type. + { + const placement: ImageStorage.Placement = .{ + .location = .{ .virtual = {} }, + .columns = 3, + .source_height = max, + }; + const actual = placement.pixelSize(.{ .width = 1, .height = 1 }, &t); + try testing.expectEqual(max, actual.width); + try testing.expectEqual(max, actual.height); + } + { + const placement: ImageStorage.Placement = .{ + .location = .{ .virtual = {} }, + .rows = 3, + .source_width = max, + }; + const actual = placement.pixelSize(.{ .width = 1, .height = 1 }, &t); + try testing.expectEqual(max, actual.width); + try testing.expectEqual(max, actual.height); + } + + // Pixel offsets are protocol-controlled too. Include them without + // allowing the grid-size numerator to wrap. + t.width_px = 2; + t.height_px = 2; + { + const placement: ImageStorage.Placement = .{ + .location = .{ .virtual = {} }, + .x_offset = max, + .y_offset = max, + }; + const actual = placement.gridSize(.{ .width = 1, .height = 1 }, &t); + try testing.expectEqual(max, actual.cols); + try testing.expectEqual(max, actual.rows); + } + + const pin = try trackPin(&t, .{ .x = 0, .y = 0 }); + defer t.screens.active.pages.untrackPin(pin); + + // Explicit maximum dimensions must clamp the rectangle to the terminal + // without overflowing its horizontal extent. + { + const placement: ImageStorage.Placement = .{ + .location = .{ .pin = pin }, + .columns = max, + .rows = 1, + }; + const rect = placement.rect(.{ .width = 1, .height = 1 }, &t).?; + try testing.expectEqual(@as(size.CellCountInt, 1), rect.bottom_right.x); + } + + // Terminals can temporarily have no pixel geometry. A placement whose + // computed grid is empty has no rectangle, so rect must not subtract one + // from a zero row count. + t.width_px = 0; + t.height_px = 0; + { + const placement: ImageStorage.Placement = .{ + .location = .{ .pin = pin }, + .columns = 1, + }; + const actual = placement.gridSize(.{ .width = 1, .height = 1 }, &t); + try testing.expectEqual(@as(u32, 0), actual.cols); + try testing.expectEqual(@as(u32, 0), actual.rows); + try testing.expect(placement.rect(.{ .width = 1, .height = 1 }, &t) == null); + } +} + test "storage: generation stamps on image add and replace" { const testing = std.testing; const io = testing.io;