From 42f0c05d7e4b99f3d101c4d834c83f679e6d1398 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 20 Aug 2025 13:04:42 -0700 Subject: [PATCH] terminal: fix undefined memory access in OSC parser Fixes #8007 Verified with `test-valgrind -Dtest-filter="OSC"` which had cond access errors before, and none after this. Basically a copy of #8008. --- src/terminal/osc.zig | 31 +++++++++++++------------------ src/terminal/stream.zig | 6 ++++++ 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/terminal/osc.zig b/src/terminal/osc.zig index 00ede49ce..6090166da 100644 --- a/src/terminal/osc.zig +++ b/src/terminal/osc.zig @@ -16,6 +16,10 @@ const kitty = @import("kitty.zig"); const log = std.log.scoped(.osc); pub const Command = union(enum) { + /// This generally shouldn't ever be set except as an initial zero value. + /// Ignore it. + invalid, + /// Set the window title of the terminal /// /// If title mode 0 is set text is expect to be hex encoded (i.e. utf-8 @@ -433,6 +437,7 @@ pub const Parser = struct { var result: Parser = .{ .alloc = null, .state = .empty, + .command = .invalid, .buf_start = 0, .buf_idx = 0, .buf_dynamic = null, @@ -440,14 +445,12 @@ pub const Parser = struct { // Keeping all our undefined values together so we can // visually easily duplicate them in the Valgrind check below. - .command = undefined, .buf = undefined, .temp_state = undefined, }; if (std.valgrind.runningOnValgrind() > 0) { // Initialize our undefined fields so Valgrind can catch it. // https://github.com/ziglang/zig/issues/19148 - result.command = undefined; result.buf = undefined; result.temp_state = undefined; } @@ -478,9 +481,17 @@ pub const Parser = struct { return; } + // Some commands have their own memory management we need to clear. + switch (self.command) { + .kitty_color_protocol => |*v| v.list.deinit(), + .color_operation => |*v| v.operations.deinit(self.alloc.?), + else => {}, + } + self.state = .empty; self.buf_start = 0; self.buf_idx = 0; + self.command = .invalid; self.complete = false; if (self.buf_dynamic) |ptr| { const alloc = self.alloc.?; @@ -488,22 +499,6 @@ pub const Parser = struct { alloc.destroy(ptr); self.buf_dynamic = null; } - - // Some commands have their own memory management we need to clear. - // After cleaning up these command, we reset the command to - // some nonsense (but valid) command so we don't double free. - const default: Command = .{ .hyperlink_end = {} }; - switch (self.command) { - .kitty_color_protocol => |*v| { - v.list.deinit(); - self.command = default; - }, - .color_operation => |*v| { - v.operations.deinit(self.alloc.?); - self.command = default; - }, - else => {}, - } } /// Consume the next character c and advance the parser state. diff --git a/src/terminal/stream.zig b/src/terminal/stream.zig index 5af446ebb..f40fc4c94 100644 --- a/src/terminal/stream.zig +++ b/src/terminal/stream.zig @@ -1608,6 +1608,12 @@ pub fn Stream(comptime Handler: type) type { .sleep, .show_message_box, .change_conemu_tab_title, .wait_input => { log.warn("unimplemented OSC callback: {}", .{cmd}); }, + + .invalid => { + // This is an invalid internal state, not an invalid OSC + // string being parsed. We shouldn't see this. + log.warn("invalid OSC, should never happen", .{}); + }, } // Fall through for when we don't have a handler.