Commit 1fc6306
Ian Tay
·
2026-03-08 12:58:59 -0400 EDT
parent df38b13
fix(ipc): validate wire data before arithmetic and indexing - expectedLength: header.len is u32 off the wire; adding sizeof(Header) at u32 width could wrap (panic in safe mode, UB slice bounds in release). Widen to usize first. - get_session_entries: cmd_len/cwd_len are u16 (max 65535) but index into [256]u8 arrays. Clamp before slicing.
3 files changed,
+17,
-7
+7,
-1
| ... | ... | @@ -15,6 +15,10 @@ pub const Tag = enum(u8) { | |
| 15 | 15 | History = 8, | |
| 16 | 16 | Run = 9, | |
| 17 | 17 | Ack = 10, | |
| 18 | + | // Non-exhaustive: this enum comes off the wire via bytesToValue and | |
| 19 | + | // @enumFromInt, so out-of-range values (11-255) are representable | |
| 20 | + | // rather than UB. Switches must handle `_` (unknown tag). | |
| 21 | + | _, | |
| 18 | 22 | }; | |
| 19 | 23 | ||
| 20 | 24 | pub const Header = packed struct { |
| ... | ... | @@ -53,7 +57,9 @@ pub const Info = extern struct { | |
| 53 | 57 | pub fn expectedLength(data: []const u8) ?usize { | |
| 54 | 58 | if (data.len < @sizeOf(Header)) return null; | |
| 55 | 59 | const header = std.mem.bytesToValue(Header, data[0..@sizeOf(Header)]); | |
| 56 | - | return @sizeOf(Header) + header.len; | |
| 60 | + | // header.len comes off the wire; widen to usize before adding so a | |
| 61 | + | // near-u32-max value can't wrap (panic in safe mode, UB in release). | |
| 62 | + | return @as(usize, @sizeOf(Header)) + @as(usize, header.len); | |
| 57 | 63 | } | |
| 58 | 64 | ||
| 59 | 65 | pub fn send(fd: i32, tag: Tag, data: []const u8) !void { |
+2,
-1
| ... | ... | @@ -624,7 +624,7 @@ const Daemon = struct { | |
| 624 | 624 | ||
| 625 | 625 | pub fn handleHistory(self: *Daemon, client: *Client, term: *ghostty_vt.Terminal, payload: []const u8) !void { | |
| 626 | 626 | const format: util.HistoryFormat = if (payload.len > 0) | |
| 627 | - | @enumFromInt(payload[0]) | |
| 627 | + | std.meta.intToEnum(util.HistoryFormat, payload[0]) catch .plain | |
| 628 | 628 | else | |
| 629 | 629 | .plain; | |
| 630 | 630 | if (util.serializeTerminal(self.alloc, term, format)) |output| { |
| ... | ... | @@ -1489,6 +1489,7 @@ fn daemonLoop(daemon: *Daemon, server_sock_fd: i32, pty_fd: i32) !void { | |
| 1489 | 1489 | .History => try daemon.handleHistory(client, &term, msg.payload), | |
| 1490 | 1490 | .Run => try daemon.handleRun(client, pty_fd, msg.payload), | |
| 1491 | 1491 | .Output, .Ack => {}, | |
| 1492 | + | _ => std.log.warn("ignoring unknown IPC tag={d}", .{@intFromEnum(msg.header.tag)}), | |
| 1492 | 1493 | } | |
| 1493 | 1494 | } | |
| 1494 | 1495 | } |
+8,
-5
| ... | ... | @@ -64,13 +64,16 @@ pub fn get_session_entries(alloc: std.mem.Allocator, socket_dir: []const u8) !st | |
| 64 | 64 | }; | |
| 65 | 65 | posix.close(result.fd); | |
| 66 | 66 | ||
| 67 | - | // Extract cmd and cwd from the fixed-size arrays | |
| 68 | - | const cmd: ?[]const u8 = if (result.info.cmd_len > 0) | |
| 69 | - | alloc.dupe(u8, result.info.cmd[0..result.info.cmd_len]) catch null | |
| 67 | + | // Extract cmd and cwd from the fixed-size arrays. Lengths come | |
| 68 | + | // off the wire (u16 range), so clamp to the actual array size. | |
| 69 | + | const cmd_len = @min(result.info.cmd_len, ipc.MAX_CMD_LEN); | |
| 70 | + | const cwd_len = @min(result.info.cwd_len, ipc.MAX_CWD_LEN); | |
| 71 | + | const cmd: ?[]const u8 = if (cmd_len > 0) | |
| 72 | + | alloc.dupe(u8, result.info.cmd[0..cmd_len]) catch null | |
| 70 | 73 | else | |
| 71 | 74 | null; | |
| 72 | - | const cwd: ?[]const u8 = if (result.info.cwd_len > 0) | |
| 73 | - | alloc.dupe(u8, result.info.cwd[0..result.info.cwd_len]) catch null | |
| 75 | + | const cwd: ?[]const u8 = if (cwd_len > 0) | |
| 76 | + | alloc.dupe(u8, result.info.cwd[0..cwd_len]) catch null | |
| 74 | 77 | else | |
| 75 | 78 | null; | |
| 76 | 79 |