diff --git a/src/datastruct/limited_allocator.zig b/src/datastruct/limited_allocator.zig new file mode 100644 index 000000000..11c4fbeaf --- /dev/null +++ b/src/datastruct/limited_allocator.zig @@ -0,0 +1,124 @@ +const std = @import("std"); +const Allocator = std.mem.Allocator; + +/// An allocator that rejects any single allocation or resize larger than a +/// configured byte limit. The limit applies to each request independently, +/// not to the total amount of memory currently allocated. +pub const LimitedAllocator = struct { + child: Allocator, + limit: usize, + + /// Set after an allocation, resize, or remap is rejected for exceeding + /// the limit. This remains set until the caller clears it. + limit_exceeded: bool = false, + + pub fn init(child: Allocator, limit: usize) LimitedAllocator { + return .{ + .child = child, + .limit = limit, + }; + } + + pub fn allocator(self: *LimitedAllocator) Allocator { + return .{ + .ptr = self, + .vtable = &.{ + .alloc = alloc, + .resize = resize, + .remap = remap, + .free = free, + }, + }; + } + + fn check(self: *LimitedAllocator, size: usize) bool { + if (size <= self.limit) return true; + self.limit_exceeded = true; + return false; + } + + fn alloc( + ctx: *anyopaque, + len: usize, + alignment: std.mem.Alignment, + ret_addr: usize, + ) ?[*]u8 { + const self: *LimitedAllocator = @ptrCast(@alignCast(ctx)); + if (!self.check(len)) return null; + return self.child.rawAlloc(len, alignment, ret_addr); + } + + fn resize( + ctx: *anyopaque, + memory: []u8, + alignment: std.mem.Alignment, + new_len: usize, + ret_addr: usize, + ) bool { + const self: *LimitedAllocator = @ptrCast(@alignCast(ctx)); + if (!self.check(new_len)) return false; + return self.child.rawResize(memory, alignment, new_len, ret_addr); + } + + fn remap( + ctx: *anyopaque, + memory: []u8, + alignment: std.mem.Alignment, + new_len: usize, + ret_addr: usize, + ) ?[*]u8 { + const self: *LimitedAllocator = @ptrCast(@alignCast(ctx)); + if (!self.check(new_len)) return null; + return self.child.rawRemap(memory, alignment, new_len, ret_addr); + } + + fn free( + ctx: *anyopaque, + memory: []u8, + alignment: std.mem.Alignment, + ret_addr: usize, + ) void { + const self: *LimitedAllocator = @ptrCast(@alignCast(ctx)); + self.child.rawFree(memory, alignment, ret_addr); + } +}; + +test "LimitedAllocator allows allocations through the limit" { + const testing = std.testing; + + var limited: LimitedAllocator = .init(testing.allocator, 8); + const alloc = limited.allocator(); + const data = try alloc.alloc(u8, 8); + defer alloc.free(data); + + try testing.expect(!limited.limit_exceeded); +} + +test "LimitedAllocator rejects allocations before the child" { + const testing = std.testing; + + var failing = testing.FailingAllocator.init(testing.allocator, .{ + .fail_index = 0, + }); + var limited: LimitedAllocator = .init(failing.allocator(), 8); + + try testing.expectError(error.OutOfMemory, limited.allocator().alloc(u8, 9)); + try testing.expect(limited.limit_exceeded); + try testing.expect(!failing.has_induced_failure); +} + +test "LimitedAllocator rejects oversized resize and remap" { + const testing = std.testing; + + var limited: LimitedAllocator = .init(testing.allocator, 8); + const alloc = limited.allocator(); + const data = try alloc.alloc(u8, 8); + defer alloc.free(data); + + try testing.expect(!alloc.resize(data, 9)); + try testing.expect(limited.limit_exceeded); + + limited.limit_exceeded = false; + try testing.expect(alloc.remap(data, 9) == null); + try testing.expect(limited.limit_exceeded); +} diff --git a/src/datastruct/main.zig b/src/datastruct/main.zig index 16a61ae5a..b3bdc6a99 100644 --- a/src/datastruct/main.zig +++ b/src/datastruct/main.zig @@ -12,6 +12,7 @@ pub const BlockingQueue = blocking_queue.BlockingQueue; pub const CacheTable = cache_table.CacheTable; pub const CircBuf = circ_buf.CircBuf; pub const IntrusiveDoublyLinkedList = intrusive_linked_list.DoublyLinkedList; +pub const LimitedAllocator = @import("limited_allocator.zig").LimitedAllocator; pub const MessageData = @import("message_data.zig").MessageData; pub const SegmentedPool = segmented_pool.SegmentedPool; pub const SplitTree = split_tree.SplitTree; diff --git a/src/renderer/image.zig b/src/renderer/image.zig index 3d5416209..927c7371e 100644 --- a/src/renderer/image.zig +++ b/src/renderer/image.zig @@ -1000,7 +1000,7 @@ test "kitty renderer ignores pending payloads and retains native placements" { const pin = try t.screens.active.pages.trackPin( t.screens.active.cursor.page_pin.*, ); - try storage.addPlacement(io, alloc, 1, 1, .{ + try storage.addPlacement(io, alloc, t.screens.active, 1, 1, .{ .location = .{ .pin = pin }, .columns = 1, .rows = 1, diff --git a/src/terminal/c/kitty_graphics.zig b/src/terminal/c/kitty_graphics.zig index e47e6894f..4e36b88a8 100644 --- a/src/terminal/c/kitty_graphics.zig +++ b/src/terminal/c/kitty_graphics.zig @@ -595,9 +595,9 @@ pub fn placement_render_info( /// the placement's origin has scrolled above the top of the viewport. /// /// A placement is considered not visible if it is a virtual (unicode -/// placeholder) placement, or if it is fully off-screen (its bottom -/// edge is above the viewport or its top edge is at or below the -/// viewport's last row). +/// placeholder) placement, its tracked content has been pruned, or it is +/// fully off-screen (its bottom edge is above the viewport or its top edge is +/// at or below the viewport's last row). fn computeViewportPos( p: *const kitty_storage.ImageStorage.Placement, image: *const Image, @@ -609,6 +609,7 @@ fn computeViewportPos( .pin => |pin| pin, .virtual => return .{ .col = 0, .row = 0, .visible = false }, }; + if (pin.garbage) return .{ .col = 0, .row = 0, .visible = false }; // Convert both the placement's pin and the viewport's top-left // corner to screen-absolute coordinates so we can subtract them @@ -631,9 +632,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 }; } @@ -1622,6 +1622,53 @@ test "placement_render_info returns all fields" { try testing.expectEqual(0, ri.source_y); try testing.expectEqual(1, ri.source_width); try testing.expectEqual(2, ri.source_height); + + const entry = iter.?.entry.?; + const pin = switch (entry.value_ptr.location) { + .pin => |pin| pin, + .virtual => unreachable, + }; + pin.garbage = true; + + ri = .{}; + try testing.expectEqual(Result.success, placement_render_info(iter, img, t, &ri)); + try testing.expect(!ri.viewport_visible); + + var rect: selection_c.CSelection = undefined; + try testing.expectEqual(Result.no_value, placement_rect(iter, img, t, &rect)); +} + +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" { diff --git a/src/terminal/kitty/graphics_exec.zig b/src/terminal/kitty/graphics_exec.zig index e0756ccc7..f499ca5bd 100644 --- a/src/terminal/kitty/graphics_exec.zig +++ b/src/terminal/kitty/graphics_exec.zig @@ -256,6 +256,7 @@ fn display( storage.addPlacement( io, alloc, + terminal.screens.active, img.id, result.placement_id, p, @@ -273,14 +274,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 +681,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_image.zig b/src/terminal/kitty/graphics_image.zig index bfb8e6b44..5078ac4c6 100644 --- a/src/terminal/kitty/graphics_image.zig +++ b/src/terminal/kitty/graphics_image.zig @@ -9,6 +9,7 @@ const fastmem = @import("../../fastmem.zig"); const command = @import("graphics_command.zig"); const PageList = @import("../PageList.zig"); const sys = @import("../sys.zig"); +const LimitedAllocator = @import("../../datastruct/main.zig").LimitedAllocator; const log = std.log.scoped(.kitty_gfx); @@ -139,36 +140,18 @@ pub const LoadingImage = struct { if (comptime builtin.os.tag != .windows) { if (std.mem.indexOfScalar(u8, cmd.data, 0) != null) { - // posix.realpath *asserts* that the path does not have - // internal nulls instead of erroring. - log.warn("failed to get absolute path: BadPathName", .{}); + // POSIX paths cannot contain internal nulls. + log.warn("invalid image path: BadPathName", .{}); return error.InvalidData; } } - var abs_buf: [std.fs.max_path_bytes]u8 = undefined; - const path = switch (t.medium) { - .direct => unreachable, // handled above - .file, .temporary_file => path: { - const len = std.Io.Dir.cwd().realPathFile( - io, - cmd.data, - &abs_buf, - ) catch |err| { - log.warn("failed to get absolute path: {}", .{err}); - return error.InvalidData; - }; - break :path abs_buf[0..len]; - }, - .shared_memory => cmd.data, - }; - // Depending on the medium, load the data from the path. switch (t.medium) { .direct => unreachable, // handled above - .file => try result.readFile(.file, io, alloc, t, path), - .temporary_file => try result.readFile(.temporary_file, io, alloc, t, path), - .shared_memory => try result.readSharedMemory(io, alloc, t, path), + .file => try result.readFile(.file, io, alloc, t, cmd.data), + .temporary_file => try result.readFile(.temporary_file, io, alloc, t, cmd.data), + .shared_memory => try result.readSharedMemory(io, alloc, t, cmd.data), } return result; @@ -222,30 +205,13 @@ pub const LoadingImage = struct { return error.InvalidData; }; if (stat.size <= 0) return error.InvalidData; - break :stat @intCast(stat.size); + break :stat std.math.cast(usize, stat.size) orelse + return error.InvalidData; }; - const expected_size: usize = switch (self.image.format) { - // Png we decode the full data size because later decoding will - // get the proper dimensions and assert validity. - .png => stat_size, - - // For these formats we have a size we must have. - .gray, .gray_alpha, .rgb, .rgba => size: { - const bpp = command.Transmission.formatBpp(self.image.format); - break :size self.image.width * self.image.height * bpp; - }, - }; - - // Our stat size must be at least the expected size otherwise - // the shared memory data is invalid. - if (stat_size < expected_size) { - log.warn( - "shared memory size too small expected={} actual={}", - .{ expected_size, stat_size }, - ); - return error.InvalidData; - } + // Get the memory range we'll read. Validate it to make sure + // it doesn't overflow. + const range = try self.sharedMemoryRange(t, stat_size); const map = std.posix.mmap( null, @@ -260,16 +226,63 @@ pub const LoadingImage = struct { }; defer std.posix.munmap(map); - // Our end size always uses the expected size so we cut off the - // padding for mmap alignment. - const start: usize = @intCast(t.offset); - const end: usize = if (t.size > 0) @min( - @as(usize, @intCast(t.offset)) + @as(usize, @intCast(t.size)), - expected_size, - ) else expected_size; - assert(self.data.items.len == 0); - try self.data.appendSlice(alloc, map[start..end]); + try self.data.appendSlice(alloc, map[range.start..range.end]); + } + + const SharedMemoryRange = struct { + start: usize, + end: usize, + }; + + /// Returns the byte range to copy from a shared memory object. + fn sharedMemoryRange( + self: *const LoadingImage, + t: command.Transmission, + stat_size: usize, + ) error{ + InvalidData, + DimensionsTooLarge, + }!SharedMemoryRange { + const expected_size: ?usize = switch (self.image.format) { + // PNG dimensions come from the decoded data. + .png => null, + + // Validate before multiplying because protocol dimensions are + // u32 values and may otherwise overflow in safe builds. + .gray, .gray_alpha, .rgb, .rgba => size: { + if (self.image.width > max_dimension or + self.image.height > max_dimension) + { + return error.DimensionsTooLarge; + } + + const bpp: usize = command.Transmission.formatBpp(self.image.format); + break :size @as(usize, self.image.width) * + @as(usize, self.image.height) * bpp; + }, + }; + + // Get our start offset and validate its within the range of + // the statted data. + const start = std.math.cast(usize, t.offset) orelse + return error.InvalidData; + if (start > stat_size) return error.InvalidData; + + // Validate that our length is within the stat range too. + const available = stat_size - start; + const data_size: usize = if (t.size > 0) + std.math.cast(usize, t.size) orelse return error.InvalidData + else if (self.image.compression == .none and expected_size != null) + expected_size.? + else + available; + if (data_size > max_size or data_size > available) { + return error.InvalidData; + } + + // data_size <= available guarantees this addition cannot overflow. + return .{ .start = start, .end = start + data_size }; } /// Reads the data from a temporary file and returns it. This allocates @@ -289,35 +302,56 @@ pub const LoadingImage = struct { else => @compileError("readFile only supports file and temporary_file"), } - // Verify file seems "safe". This is logic copied directly from Kitty, - // mostly. This is really rough but it will catch obvious bad actors. - if (std.mem.startsWith(u8, path, "/proc/") or - std.mem.startsWith(u8, path, "/sys/") or - (std.mem.startsWith(u8, path, "/dev/") and - !std.mem.startsWith(u8, path, "/dev/shm/"))) - { + // Open our file right away before we do validation. This avoids + // TOCTOU issues. + var file = std.Io.Dir.cwd().openFile( + io, + path, + .{}, + ) catch |err| { + log.warn("failed to open image file: {}", .{err}); return error.InvalidData; + }; + + // We'll populate a delete path if this is a temporary file. + var delete_path: ?[]const u8 = null; + defer { + file.close(io); + if (delete_path) |p| { + std.Io.Dir.cwd().deleteFile(io, p) catch |err| { + log.warn("failed to delete temporary file: {}", .{err}); + }; + } } + // Derive the path from the open handle so the file we validate is the + // exact file we read. Resolving a path before opening it would allow a + // cooperating process to swap a symlink or directory entry in between. + var abs_buf: [std.fs.max_path_bytes]u8 = undefined; + const abs_path = validatedFilePath( + io, + file, + &abs_buf, + ) catch |err| { + log.warn("failed to validate image file path: {}", .{err}); + return error.InvalidData; + }; + // Temporary file logic if (medium == .temporary_file) { assert(self.temporary_directory != null); - if (!isPathInTempDir(io, self.temporary_directory.?, path)) return error.TemporaryFileNotInTempDir; - if (std.mem.indexOf(u8, path, "tty-graphics-protocol") == null) { - return error.TemporaryFileNotNamedCorrectly; - } + if (!isPathInTempDir( + io, + self.temporary_directory.?, + abs_path, + )) return error.TemporaryFileNotInTempDir; + if (std.mem.indexOf( + u8, + abs_path, + "tty-graphics-protocol", + ) == null) return error.TemporaryFileNotNamedCorrectly; + delete_path = abs_path; } - defer if (medium == .temporary_file) { - std.Io.Dir.cwd().deleteFile(io, path) catch |err| { - log.warn("failed to delete temporary file: {}", .{err}); - }; - }; - - var file = std.Io.Dir.cwd().openFile(io, path, .{}) catch |err| { - log.warn("failed to open temporary file: {}", .{err}); - return error.InvalidData; - }; - defer file.close(io); // File must be a regular file if (file.stat(io)) |stat| { @@ -354,12 +388,30 @@ pub const LoadingImage = struct { self.data = .{ .items = managed.items, .capacity = managed.capacity }; } + /// Returns the canonical path of an open file after applying the file + /// transmission blocklist. + fn validatedFilePath(io: std.Io, file: std.Io.File, buf: []u8) ![]const u8 { + const path = buf[0..try file.realPath(io, buf)]; + + // This is logic copied directly from Kitty, mostly. This is really + // rough but it will catch obvious bad actors. + if (std.mem.startsWith(u8, path, "/proc/") or + std.mem.startsWith(u8, path, "/sys/") or + (std.mem.startsWith(u8, path, "/dev/") and + !std.mem.startsWith(u8, path, "/dev/shm/"))) + { + return error.InvalidData; + } + + return path; + } + /// Returns true if path appears to be in a temporary directory. /// Copies logic from Kitty. fn isPathInTempDir(io: std.Io, dir: []const u8, path: []const u8) bool { - if (std.mem.startsWith(u8, path, "/tmp")) return true; - if (std.mem.startsWith(u8, path, "/dev/shm")) return true; - if (std.mem.startsWith(u8, path, dir)) return true; + if (isPathInDir("/tmp", path)) return true; + if (isPathInDir("/dev/shm", path)) return true; + if (isPathInDir(dir, path)) return true; // The temporary dir is sometimes a symlink. On macOS for // example /tmp is /private/var/... @@ -369,7 +421,7 @@ pub const LoadingImage = struct { dir, &buf, ) catch return false]; - if (std.mem.startsWith(u8, path, real_dir)) return true; + if (isPathInDir(real_dir, path)) return true; return false; } @@ -500,14 +552,20 @@ pub const LoadingImage = struct { const decode_png_fn = sys.decode_png orelse return error.UnsupportedFormat; + + var limited: LimitedAllocator = .init(alloc, max_size); + const decode_alloc = limited.allocator(); const result = decode_png_fn( - alloc, + decode_alloc, self.data.items, ) catch |err| switch (err) { error.InvalidData => return error.InvalidData, - error.OutOfMemory => return error.OutOfMemory, + error.OutOfMemory => if (limited.limit_exceeded) + return error.InvalidData + else + return error.OutOfMemory, }; - defer alloc.free(result.data); + defer decode_alloc.free(result.data); if (result.data.len > max_size) { log.warn("png image too large size={} max_size={}", .{ result.data.len, max_size }); @@ -626,6 +684,88 @@ pub const Rect = struct { bottom_right: PageList.Pin, }; +/// Returns true if `path` is `dir` or is contained within it, requiring a +/// path-separator boundary so similarly prefixed directories do not match. +fn isPathInDir(dir: []const u8, path: []const u8) bool { + if (dir.len == 0 or !std.mem.startsWith(u8, path, dir)) return false; + if (path.len == dir.len or std.fs.path.isSep(dir[dir.len - 1])) return true; + return std.fs.path.isSep(path[dir.len]); +} + +test "temporary file path must be inside directory" { + const testing = std.testing; + + try testing.expect(isPathInDir("/tmp", "/tmp/tty-graphics-protocol-image.data")); + try testing.expect(isPathInDir("/tmp/", "/tmp/tty-graphics-protocol-image.data")); + try testing.expect(isPathInDir("/tmp", "/tmp")); + + try testing.expect(!isPathInDir("", "/tmp/tty-graphics-protocol-image.data")); + try testing.expect(!isPathInDir("/tmp", "/tmpX/tty-graphics-protocol-image.data")); + try testing.expect(!isPathInDir("/dev/shm", "/dev/shm-evil/tty-graphics-protocol-image.data")); + try testing.expect(!isPathInDir("/custom/tmp", "/custom/tmp-suffix/tty-graphics-protocol-image.data")); +} + +test "shared memory range with offset and size" { + const testing = std.testing; + + const loading: LoadingImage = .{ + .image = .{ + .width = 1, + .height = 1, + .format = .rgb, + }, + .quiet = .no, + .temporary_directory = null, + }; + + const explicit = try loading.sharedMemoryRange(.{ + .offset = 2, + .size = 3, + }, 5); + try testing.expectEqual(@as(usize, 2), explicit.start); + try testing.expectEqual(@as(usize, 5), explicit.end); + + const implicit = try loading.sharedMemoryRange(.{ + .offset = 2, + }, 5); + try testing.expectEqual(@as(usize, 2), implicit.start); + try testing.expectEqual(@as(usize, 5), implicit.end); +} + +test "shared memory range rejects out of bounds offset" { + const loading: LoadingImage = .{ + .image = .{ + .width = 1, + .height = 1, + .format = .rgb, + }, + .quiet = .no, + .temporary_directory = null, + }; + + try std.testing.expectError( + error.InvalidData, + loading.sharedMemoryRange(.{ .offset = 4 }, 3), + ); +} + +test "shared memory range validates dimensions before multiplication" { + const loading: LoadingImage = .{ + .image = .{ + .width = std.math.maxInt(u32), + .height = std.math.maxInt(u32), + .format = .rgba, + }, + .quiet = .no, + .temporary_directory = null, + }; + + try std.testing.expectError( + error.DimensionsTooLarge, + loading.sharedMemoryRange(.{}, 1), + ); +} + // This specifically tests we ALLOW invalid RGB data because Kitty // documents that this should work. test "image load with invalid RGB data" { @@ -861,6 +1001,54 @@ test "image load: temporary file without correct path" { try tmp_dir.dir.access(testing.io, path, .{}); } +test "image load: temporary file outside directory prefix is rejected" { + const testing = std.testing; + const alloc = testing.allocator; + const io = testing.io; + + var tmp_dir = testing.tmpDir(.{}); + defer tmp_dir.cleanup(); + try tmp_dir.dir.createDir(io, "temp", .default_dir); + try tmp_dir.dir.createDir(io, "temp-suffix", .default_dir); + + var trusted_dir = try tmp_dir.dir.openDir(io, "temp", .{}); + defer trusted_dir.close(io); + var outside_dir = try tmp_dir.dir.openDir(io, "temp-suffix", .{}); + defer outside_dir.close(io); + + const filename = "tty-graphics-protocol-image.data"; + const data = @embedFile("testdata/image-rgb-none-20x15-2147483647-raw.data"); + try outside_dir.writeFile(io, .{ + .sub_path = filename, + .data = data, + }); + + var trusted_path_buf: [std.fs.max_path_bytes]u8 = undefined; + const trusted_path = trusted_path_buf[0..try trusted_dir.realPath(io, &trusted_path_buf)]; + var outside_path_buf: [std.fs.max_path_bytes]u8 = undefined; + const outside_path = outside_path_buf[0..try outside_dir.realPathFile(io, filename, &outside_path_buf)]; + + var cmd: command.Command = .{ + .control = .{ .transmit = .{ + .format = .rgb, + .medium = .temporary_file, + .compression = .none, + .width = 20, + .height = 15, + .image_id = 31, + } }, + .data = try alloc.dupe(u8, outside_path), + }; + defer cmd.deinit(alloc); + try testing.expectError( + error.TemporaryFileNotInTempDir, + LoadingImage.init(io, alloc, &cmd, .allWithTempDir(trusted_path)), + ); + + // Rejection must happen before temporary-file cleanup is armed. + try outside_dir.access(io, filename, .{}); +} + test "image load: rgb, not compressed, temporary file" { const testing = std.testing; const alloc = testing.allocator; @@ -987,6 +1175,39 @@ test "image load: rgb, not compressed, relative regular file" { try testing.expect(img.compression == .none); } +test "image load: blocklist applies to opened file after symlink swap" { + if (builtin.os.tag == .windows) return error.SkipZigTest; + + const testing = std.testing; + const io = testing.io; + + var tmp_dir = testing.tmpDir(.{}); + defer tmp_dir.cleanup(); + try tmp_dir.dir.writeFile(io, .{ + .sub_path = "safe.data", + .data = "safe", + }); + try tmp_dir.dir.symLink(io, "/dev/null", "image.data", .{}); + + // Pin the blocked file, then simulate the cooperating process replacing + // the path with a safe target before validation. + const blocked_file = try tmp_dir.dir.openFile(io, "image.data", .{}); + defer blocked_file.close(io); + try tmp_dir.dir.symLinkAtomic(io, "safe.data", "image.data", .{}); + + var path_buf: [std.fs.max_path_bytes]u8 = undefined; + try testing.expectError( + error.InvalidData, + LoadingImage.validatedFilePath(io, blocked_file, &path_buf), + ); + + // The pathname now resolves to the safe replacement, demonstrating that + // the rejection above came from the already-open file handle. + const safe_file = try tmp_dir.dir.openFile(io, "image.data", .{}); + defer safe_file.close(io); + _ = try LoadingImage.validatedFilePath(io, safe_file, &path_buf); +} + test "image load: png, not compressed, regular file" { if (sys.decode_png == null) return error.SkipZigTest; @@ -1032,6 +1253,72 @@ test "image load: png, not compressed, regular file" { try tmp_dir.dir.access(testing.io, path, .{}); } +test "image load: png rejects oversized decoder allocation" { + const testing = std.testing; + + const oversized_decoder = struct { + fn decode( + alloc: Allocator, + _: []const u8, + ) sys.DecodeError!sys.Image { + const data = try alloc.alloc(u8, max_size + 1); + return .{ + .width = 1, + .height = 1, + .data = data, + }; + } + }.decode; + + const original_decode_png = sys.decode_png; + defer sys.decode_png = original_decode_png; + sys.decode_png = &oversized_decoder; + + // Fail any allocation which reaches the underlying allocator. The size + // limiter should reject the decoder's request before it gets that far. + var failing = testing.FailingAllocator.init(testing.allocator, .{ + .fail_index = 0, + }); + const alloc = failing.allocator(); + + var loading: LoadingImage = .{ + .image = .{ .format = .png }, + .quiet = .no, + .temporary_directory = null, + }; + defer loading.deinit(alloc); + + try testing.expectError(error.InvalidData, loading.complete(alloc)); + try testing.expect(!failing.has_induced_failure); +} + +test "image load: png rejects oversized Wuffs image before allocation" { + if (sys.decode_png == null) return error.SkipZigTest; + + const testing = std.testing; + const alloc = testing.allocator; + + // Turn the small test PNG into a 32768x32767 image. Its decoded RGBA + // size is just under Wuffs' 4 GiB package limit but over Kitty's 400 MiB + // limit, which previously allowed the large allocation to happen first. + var data = @embedFile("testdata/image-png-none-50x76-2147483647-raw.data").*; + std.mem.writeInt(u32, data[16..20], 32768, .big); + std.mem.writeInt(u32, data[20..24], 32767, .big); + std.mem.writeInt(u32, data[29..33], std.hash.Crc32.hash(data[12..29]), .big); + + const cmd: command.Command = .{ + .control = .{ .transmit = .{ + .format = .png, + .medium = .direct, + } }, + .data = &data, + }; + var loading = try LoadingImage.init(testing.io, alloc, &cmd, .direct); + defer loading.deinit(alloc); + + try testing.expectError(error.InvalidData, loading.complete(alloc)); +} + test "limits: direct medium always allowed" { const testing = std.testing; const alloc = testing.allocator; diff --git a/src/terminal/kitty/graphics_storage.zig b/src/terminal/kitty/graphics_storage.zig index d59de54d4..83bd5f9cb 100644 --- a/src/terminal/kitty/graphics_storage.zig +++ b/src/terminal/kitty/graphics_storage.zig @@ -300,11 +300,14 @@ pub const ImageStorage = struct { } /// Add a placement for a given image. The caller must verify in advance - /// the image exists to prevent memory corruption. + /// the image exists to prevent memory corruption. On success, storage + /// owns `p`; on error, the caller retains ownership. The screen must own + /// any tracked pin in `p` and in the placement it replaces. pub fn addPlacement( self: *ImageStorage, io: std.Io, alloc: Allocator, + s: *terminal.Screen, image_id: u32, placement_id: u32, p: Placement, @@ -316,6 +319,14 @@ pub const ImageStorage = struct { p, }); + // Tracked pins are marked garbage when their underlying history is + // pruned. Kitty removes placements once they scroll out of retained + // history, so reclaim those placements before growing the map for a + // new one. If allocation below fails, the sweep is still a content + // mutation and must be visible to consumers. + const removed_garbage = self.removeGarbagePlacements(s); + errdefer if (removed_garbage) self.markMutated(io); + // The important piece here is that the placement ID needs to // be marked internal if it is zero. This allows multiple placements // to be added for the same image. If it is non-zero, then it is @@ -336,11 +347,35 @@ pub const ImageStorage = struct { }; const gop = try self.placements.getOrPut(alloc, key); + if (gop.found_existing) gop.value_ptr.deinit(s); gop.value_ptr.* = p; self.markMutated(io); } + /// Remove pin-backed placements whose tracked content has been pruned. + /// Virtual placements have no tracked screen location and are retained. + fn removeGarbagePlacements( + self: *ImageStorage, + s: *terminal.Screen, + ) bool { + var removed = false; + var it = self.placements.iterator(); + while (it.next()) |entry| { + const pin = switch (entry.value_ptr.location) { + .pin => |pin| pin, + .virtual => continue, + }; + if (!pin.garbage) continue; + + entry.value_ptr.deinit(s); + self.placements.removeByPtr(entry.key_ptr); + removed = true; + } + + return removed; + } + fn clearPlacements(self: *ImageStorage, s: *terminal.Screen) void { var it = self.placements.iterator(); while (it.next()) |entry| entry.value_ptr.deinit(s); @@ -567,7 +602,7 @@ pub const ImageStorage = struct { var it = self.placements.iterator(); while (it.next()) |entry| { - if (entry.key_ptr.image_id >= v.first or entry.key_ptr.image_id <= v.last) { + if (entry.key_ptr.image_id >= v.first and entry.key_ptr.image_id <= v.last) { const image_id = entry.key_ptr.image_id; entry.value_ptr.deinit(t.screens.active); self.placements.removeByPtr(entry.key_ptr); @@ -849,6 +884,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 +933,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 +952,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 +964,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 +995,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 +1009,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, @@ -977,18 +1021,23 @@ pub const ImageStorage = struct { .pin => |p| p, .virtual => return null, }; + if (pin.garbage) 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.*, @@ -1021,8 +1070,8 @@ test "storage: add placement with zero placement id" { defer s.deinit(alloc, t.screens.active); try s.addImage(io, alloc, .{ .id = 1, .width = 50, .height = 50 }); try s.addImage(io, alloc, .{ .id = 2, .width = 25, .height = 25 }); - try s.addPlacement(io, alloc, 1, 0, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 25, .y = 25 }) } }); - try s.addPlacement(io, alloc, 1, 0, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 25, .y = 25 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 0, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 25, .y = 25 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 0, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 25, .y = 25 }) } }); try testing.expectEqual(@as(usize, 2), s.placements.count()); try testing.expectEqual(@as(usize, 2), s.images.count()); @@ -1038,6 +1087,81 @@ test "storage: add placement with zero placement id" { }) != null); } +test "storage: replacing placement releases tracked pin" { + const testing = std.testing; + const alloc = testing.allocator; + const io = testing.io; + var t = try terminal.Terminal.init(io, alloc, .{ .cols = 3, .rows = 3 }); + defer t.deinit(alloc); + + var s: ImageStorage = .{}; + defer s.deinit(alloc, t.screens.active); + try s.addImage(io, alloc, .{ .id = 1 }); + + const tracked = t.screens.active.pages.countTrackedPins(); + try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{ + .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) }, + }); + try testing.expectEqual( + tracked + 1, + t.screens.active.pages.countTrackedPins(), + ); + + const replacement_pin = try trackPin(&t, .{ .x = 2, .y = 2 }); + try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{ + .location = .{ .pin = replacement_pin }, + }); + try testing.expectEqual(@as(usize, 1), s.placements.count()); + try testing.expectEqual( + tracked + 1, + t.screens.active.pages.countTrackedPins(), + ); + try testing.expectEqual( + replacement_pin, + s.placements.get(.{ + .image_id = 1, + .placement_id = .{ .tag = .external, .id = 1 }, + }).?.location.pin, + ); +} + +test "storage: adding placement reclaims garbage placements" { + const testing = std.testing; + const alloc = testing.allocator; + const io = testing.io; + var t = try terminal.Terminal.init(io, alloc, .{ .cols = 3, .rows = 3 }); + defer t.deinit(alloc); + + var s: ImageStorage = .{}; + defer s.deinit(alloc, t.screens.active); + try s.addImage(io, alloc, .{ .id = 1 }); + + const tracked = t.screens.active.pages.countTrackedPins(); + const old_pin = try trackPin(&t, .{ .x = 0, .y = 0 }); + try s.addPlacement(io, alloc, t.screens.active, 1, 0, .{ + .location = .{ .pin = old_pin }, + }); + old_pin.garbage = true; + + const new_pin = try trackPin(&t, .{ .x = 1, .y = 1 }); + try s.addPlacement(io, alloc, t.screens.active, 1, 0, .{ + .location = .{ .pin = new_pin }, + }); + + try testing.expectEqual(@as(usize, 1), s.placements.count()); + try testing.expectEqual( + tracked + 1, + t.screens.active.pages.countTrackedPins(), + ); + try testing.expectEqual( + new_pin, + s.placements.get(.{ + .image_id = 1, + .placement_id = .{ .tag = .internal, .id = 1 }, + }).?.location.pin, + ); +} + test "storage: delete all placements and images" { const testing = std.testing; const alloc = testing.allocator; @@ -1051,8 +1175,8 @@ test "storage: delete all placements and images" { try s.addImage(io, alloc, .{ .id = 1 }); try s.addImage(io, alloc, .{ .id = 2 }); try s.addImage(io, alloc, .{ .id = 3 }); - try s.addPlacement(io, alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); - try s.addPlacement(io, alloc, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); s.dirty = false; s.delete(io, alloc, &t, .{ .all = true }); @@ -1076,8 +1200,8 @@ test "storage: delete all placements and images preserves limit" { try s.addImage(io, alloc, .{ .id = 1 }); try s.addImage(io, alloc, .{ .id = 2 }); try s.addImage(io, alloc, .{ .id = 3 }); - try s.addPlacement(io, alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); - try s.addPlacement(io, alloc, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); s.dirty = false; s.delete(io, alloc, &t, .{ .all = true }); @@ -1101,8 +1225,8 @@ test "storage: delete all placements" { try s.addImage(io, alloc, .{ .id = 1 }); try s.addImage(io, alloc, .{ .id = 2 }); try s.addImage(io, alloc, .{ .id = 3 }); - try s.addPlacement(io, alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); - try s.addPlacement(io, alloc, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); s.dirty = false; s.delete(io, alloc, &t, .{ .all = false }); @@ -1125,8 +1249,8 @@ test "storage: delete all placements by image id" { try s.addImage(io, alloc, .{ .id = 1 }); try s.addImage(io, alloc, .{ .id = 2 }); try s.addImage(io, alloc, .{ .id = 3 }); - try s.addPlacement(io, alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); - try s.addPlacement(io, alloc, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); s.dirty = false; s.delete(io, alloc, &t, .{ .id = .{ .image_id = 2 } }); @@ -1149,8 +1273,8 @@ test "storage: delete all placements by image id and unused images" { try s.addImage(io, alloc, .{ .id = 1 }); try s.addImage(io, alloc, .{ .id = 2 }); try s.addImage(io, alloc, .{ .id = 3 }); - try s.addPlacement(io, alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); - try s.addPlacement(io, alloc, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); s.dirty = false; s.delete(io, alloc, &t, .{ .id = .{ .delete = true, .image_id = 2 } }); @@ -1173,9 +1297,9 @@ test "storage: delete placement by specific id" { try s.addImage(io, alloc, .{ .id = 1 }); try s.addImage(io, alloc, .{ .id = 2 }); try s.addImage(io, alloc, .{ .id = 3 }); - try s.addPlacement(io, alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); - try s.addPlacement(io, alloc, 1, 2, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); - try s.addPlacement(io, alloc, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 2, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); s.dirty = false; s.delete(io, alloc, &t, .{ .id = .{ @@ -1203,8 +1327,8 @@ test "storage: delete intersecting cursor" { defer s.deinit(alloc, t.screens.active); try s.addImage(io, alloc, .{ .id = 1, .width = 50, .height = 50 }); try s.addImage(io, alloc, .{ .id = 2, .width = 25, .height = 25 }); - try s.addPlacement(io, alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 0, .y = 0 }) } }); - try s.addPlacement(io, alloc, 1, 2, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 25, .y = 25 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 0, .y = 0 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 2, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 25, .y = 25 }) } }); t.screens.active.cursorAbsolute(12, 12); @@ -1236,8 +1360,8 @@ test "storage: delete intersecting cursor plus unused" { defer s.deinit(alloc, t.screens.active); try s.addImage(io, alloc, .{ .id = 1, .width = 50, .height = 50 }); try s.addImage(io, alloc, .{ .id = 2, .width = 25, .height = 25 }); - try s.addPlacement(io, alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 0, .y = 0 }) } }); - try s.addPlacement(io, alloc, 1, 2, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 25, .y = 25 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 0, .y = 0 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 2, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 25, .y = 25 }) } }); t.screens.active.cursorAbsolute(12, 12); @@ -1269,8 +1393,8 @@ test "storage: delete intersecting cursor hits multiple" { defer s.deinit(alloc, t.screens.active); try s.addImage(io, alloc, .{ .id = 1, .width = 50, .height = 50 }); try s.addImage(io, alloc, .{ .id = 2, .width = 25, .height = 25 }); - try s.addPlacement(io, alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 0, .y = 0 }) } }); - try s.addPlacement(io, alloc, 1, 2, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 25, .y = 25 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 0, .y = 0 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 2, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 25, .y = 25 }) } }); t.screens.active.cursorAbsolute(26, 26); @@ -1296,8 +1420,8 @@ test "storage: delete by column" { defer s.deinit(alloc, t.screens.active); try s.addImage(io, alloc, .{ .id = 1, .width = 50, .height = 50 }); try s.addImage(io, alloc, .{ .id = 2, .width = 25, .height = 25 }); - try s.addPlacement(io, alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 0, .y = 0 }) } }); - try s.addPlacement(io, alloc, 1, 2, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 25, .y = 25 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 0, .y = 0 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 2, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 25, .y = 25 }) } }); s.dirty = false; s.delete(io, alloc, &t, .{ .column = .{ @@ -1328,9 +1452,9 @@ test "storage: delete by column 1x1" { var s: ImageStorage = .{}; defer s.deinit(alloc, t.screens.active); try s.addImage(io, alloc, .{ .id = 1, .width = 1, .height = 1 }); - try s.addPlacement(io, alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 0, .y = 0 }) } }); - try s.addPlacement(io, alloc, 1, 2, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 0 }) } }); - try s.addPlacement(io, alloc, 1, 3, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 2, .y = 0 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 0, .y = 0 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 2, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 0 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 3, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 2, .y = 0 }) } }); s.delete(io, alloc, &t, .{ .column = .{ .delete = false, @@ -1364,8 +1488,8 @@ test "storage: delete by row" { defer s.deinit(alloc, t.screens.active); try s.addImage(io, alloc, .{ .id = 1, .width = 50, .height = 50 }); try s.addImage(io, alloc, .{ .id = 2, .width = 25, .height = 25 }); - try s.addPlacement(io, alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 0, .y = 0 }) } }); - try s.addPlacement(io, alloc, 1, 2, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 25, .y = 25 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 0, .y = 0 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 2, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 25, .y = 25 }) } }); s.dirty = false; s.delete(io, alloc, &t, .{ .row = .{ @@ -1396,9 +1520,9 @@ test "storage: delete by row 1x1" { var s: ImageStorage = .{}; defer s.deinit(alloc, t.screens.active); try s.addImage(io, alloc, .{ .id = 1, .width = 1, .height = 1 }); - try s.addPlacement(io, alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .y = 0 }) } }); - try s.addPlacement(io, alloc, 1, 2, .{ .location = .{ .pin = try trackPin(&t, .{ .y = 1 }) } }); - try s.addPlacement(io, alloc, 1, 3, .{ .location = .{ .pin = try trackPin(&t, .{ .y = 2 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .y = 0 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 2, .{ .location = .{ .pin = try trackPin(&t, .{ .y = 1 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 3, .{ .location = .{ .pin = try trackPin(&t, .{ .y = 2 }) } }); s.delete(io, alloc, &t, .{ .row = .{ .delete = false, @@ -1431,8 +1555,8 @@ test "storage: delete images by range 1" { try s.addImage(io, alloc, .{ .id = 1 }); try s.addImage(io, alloc, .{ .id = 2 }); try s.addImage(io, alloc, .{ .id = 3 }); - try s.addPlacement(io, alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); - try s.addPlacement(io, alloc, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); try testing.expectEqual(@as(usize, 3), s.images.count()); try testing.expectEqual(@as(usize, 2), s.placements.count()); @@ -1457,8 +1581,8 @@ test "storage: delete images by range 2" { try s.addImage(io, alloc, .{ .id = 1 }); try s.addImage(io, alloc, .{ .id = 2 }); try s.addImage(io, alloc, .{ .id = 3 }); - try s.addPlacement(io, alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); - try s.addPlacement(io, alloc, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); try testing.expectEqual(@as(usize, 3), s.images.count()); try testing.expectEqual(@as(usize, 2), s.placements.count()); @@ -1483,17 +1607,26 @@ test "storage: delete images by range 3" { try s.addImage(io, alloc, .{ .id = 1 }); try s.addImage(io, alloc, .{ .id = 2 }); try s.addImage(io, alloc, .{ .id = 3 }); - try s.addPlacement(io, alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); - try s.addPlacement(io, alloc, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 3, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); try testing.expectEqual(@as(usize, 3), s.images.count()); - try testing.expectEqual(@as(usize, 2), s.placements.count()); + try testing.expectEqual(@as(usize, 3), s.placements.count()); s.dirty = false; - s.delete(io, alloc, &t, .{ .range = .{ .delete = false, .first = 1, .last = 1 } }); + s.delete(io, alloc, &t, .{ .range = .{ .delete = false, .first = 2, .last = 2 } }); try testing.expect(s.dirty); try testing.expectEqual(@as(usize, 3), s.images.count()); - try testing.expectEqual(@as(usize, 0), s.placements.count()); - try testing.expectEqual(tracked, t.screens.active.pages.countTrackedPins()); + try testing.expectEqual(@as(usize, 2), s.placements.count()); + try testing.expect(s.placements.get(.{ + .image_id = 1, + .placement_id = .{ .tag = .external, .id = 1 }, + }) != null); + try testing.expect(s.placements.get(.{ + .image_id = 3, + .placement_id = .{ .tag = .external, .id = 1 }, + }) != null); + try testing.expectEqual(tracked + 2, t.screens.active.pages.countTrackedPins()); } test "storage: delete images by range 4" { @@ -1509,17 +1642,26 @@ test "storage: delete images by range 4" { try s.addImage(io, alloc, .{ .id = 1 }); try s.addImage(io, alloc, .{ .id = 2 }); try s.addImage(io, alloc, .{ .id = 3 }); - try s.addPlacement(io, alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); - try s.addPlacement(io, alloc, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 3, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); try testing.expectEqual(@as(usize, 3), s.images.count()); - try testing.expectEqual(@as(usize, 2), s.placements.count()); + try testing.expectEqual(@as(usize, 3), s.placements.count()); s.dirty = false; - s.delete(io, alloc, &t, .{ .range = .{ .delete = true, .first = 1, .last = 1 } }); + s.delete(io, alloc, &t, .{ .range = .{ .delete = true, .first = 2, .last = 2 } }); try testing.expect(s.dirty); - try testing.expectEqual(@as(usize, 1), s.images.count()); - try testing.expectEqual(@as(usize, 0), s.placements.count()); - try testing.expectEqual(tracked, t.screens.active.pages.countTrackedPins()); + try testing.expectEqual(@as(usize, 2), s.images.count()); + try testing.expectEqual(@as(usize, 2), s.placements.count()); + try testing.expect(s.placements.get(.{ + .image_id = 1, + .placement_id = .{ .tag = .external, .id = 1 }, + }) != null); + try testing.expect(s.placements.get(.{ + .image_id = 3, + .placement_id = .{ .tag = .external, .id = 1 }, + }) != null); + try testing.expectEqual(tracked + 2, t.screens.active.pages.countTrackedPins()); } test "storage: aspect ratio calculation when only columns or rows specified" { @@ -1569,6 +1711,113 @@ 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); + } + + // A garbage pin represents content that has been pruned from retained + // history. Its fallback location must not make the placement visible. + pin.garbage = true; + { + const placement: ImageStorage.Placement = .{ + .location = .{ .pin = pin }, + .columns = 1, + .rows = 1, + }; + try testing.expect(placement.rect(.{ .width = 1, .height = 1 }, &t) == null); + } + pin.garbage = false; + + // 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; @@ -1619,7 +1868,7 @@ test "storage: generation bumps on placement and delete" { try s.addImage(io, alloc, .{ .id = 1 }); const gen_add = s.generation; - try s.addPlacement(io, alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); const gen_place = s.generation; try testing.expect(gen_place > gen_add); @@ -1713,7 +1962,7 @@ test "storage: no-op delete does not mark a mutation" { // Same for a delete that matches nothing. try s.addImage(io, alloc, .{ .id = 1 }); - try s.addPlacement(io, alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); const gen = s.generation; s.dirty = false; s.delete(io, alloc, &t, .{ .id = .{ .image_id = 42 } }); @@ -1754,6 +2003,7 @@ test "storage: evict unused transient image" { try s.addPlacement( io, alloc, + t.screens.active, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }, @@ -1792,7 +2042,7 @@ test "storage: pending image completes once and preserves age" { try testing.expectEqual(@as(u32, 1), s.imageByNumber(7).?.id); try testing.expect(s.imageById(1).?.data.isPending()); - try s.addPlacement(io, alloc, 1, 1, .{ + try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) }, }); try testing.expectEqual(@as(usize, 1), s.placements.count()); @@ -1884,7 +2134,7 @@ test "storage: replacement reuses pending reservation and preserves placements" .format = .rgba, .data = .{ .pending = 8 }, }); - try s.addPlacement(io, alloc, 1, 1, .{ + try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) }, }); try s.addImage(io, alloc, .{ @@ -1946,7 +2196,7 @@ test "storage: pending images share exact eviction ordering" { .data = .{ .complete = try alloc.dupe(u8, "*" ** 64) }, .usage = .{ .transient = true }, }); - try s.addPlacement(io, alloc, 2, 1, .{ + try s.addPlacement(io, alloc, t.screens.active, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) }, }); diff --git a/src/terminal/kitty/graphics_unicode.zig b/src/terminal/kitty/graphics_unicode.zig index dd1a6ce44..21fa0b3a5 100644 --- a/src/terminal/kitty/graphics_unicode.zig +++ b/src/terminal/kitty/graphics_unicode.zig @@ -1196,7 +1196,7 @@ test "unicode render placement: dog 4x2" { const image: Image = .{ .id = 1, .width = 500, .height = 306 }; try s.addImage(io, alloc, image); - try s.addPlacement(io, alloc, 1, 0, .{ + try s.addPlacement(io, alloc, t.screens.active, 1, 0, .{ .location = .{ .virtual = {} }, .columns = 4, .rows = 2, @@ -1264,7 +1264,7 @@ test "unicode render placement: dog 2x2 with blank cells" { const image: Image = .{ .id = 1, .width = 500, .height = 306 }; try s.addImage(io, alloc, image); - try s.addPlacement(io, alloc, 1, 0, .{ + try s.addPlacement(io, alloc, t.screens.active, 1, 0, .{ .location = .{ .virtual = {} }, .columns = 2, .rows = 2, @@ -1331,7 +1331,7 @@ test "unicode render placement: dog 1x1" { const image: Image = .{ .id = 1, .width = 500, .height = 306 }; try s.addImage(io, alloc, image); - try s.addPlacement(io, alloc, 1, 0, .{ + try s.addPlacement(io, alloc, t.screens.active, 1, 0, .{ .location = .{ .virtual = {} }, .columns = 1, .rows = 1, diff --git a/src/terminal/snapshot/snapshot.zig b/src/terminal/snapshot/snapshot.zig index f3adda8cb..dc8d7524d 100644 --- a/src/terminal/snapshot/snapshot.zig +++ b/src/terminal/snapshot/snapshot.zig @@ -1231,6 +1231,7 @@ test "complete snapshot preserves Kitty virtual placeholders" { try t.screens.active.kitty_images.addPlacement( testing.io, testing.allocator, + t.screens.active, 1, 0, .{