fix(tmux): use-after-free in receivedListWindows action slice
receivedListWindows stores a slice of Window structs into the action list so callers can process GUI window changes. Previously, this slice pointed directly into a local ArrayList's backing memory, which was freed by defer on function return. The action then held a dangling pointer. This is currently latent because consumers only read the slice length (inline in the fat pointer), not the pointed-to data. It becomes a live crash the moment anyone iterates the window contents. Fix: dupe the slice into the arena allocator so it shares the same lifetime as the action list. Add a regression test that dereferences action.windows[0].id/width/ height after the function returns, which would read freed memory without the fix.pull/11213/head
parent
472b926a4d
commit
9524f311d0
|
|
@ -894,8 +894,11 @@ pub const Viewer = struct {
|
|||
}
|
||||
|
||||
// Setup our windows action so the caller can process GUI
|
||||
// window changes.
|
||||
try actions.append(arena_alloc, .{ .windows = windows.items });
|
||||
// window changes. We must dupe into the arena because `windows`
|
||||
// is a local ArrayList whose backing memory is freed by the
|
||||
// defer above when this function returns.
|
||||
const arena_windows = try arena_alloc.dupe(Window, windows.items);
|
||||
try actions.append(arena_alloc, .{ .windows = arena_windows });
|
||||
|
||||
// Sync up our layouts. This will populate unknown panes, prune, etc.
|
||||
try self.syncLayouts(windows.items);
|
||||
|
|
@ -2290,3 +2293,59 @@ test "two pane flow with pane state" {
|
|||
},
|
||||
});
|
||||
}
|
||||
|
||||
test "list-windows action data survives function return" {
|
||||
// Regression test: receivedListWindows used to store windows.items
|
||||
// (a slice into a local ArrayList) directly into the action. The
|
||||
// ArrayList backing memory was freed by defer on function return,
|
||||
// leaving the action holding a dangling pointer. The fix dupes the
|
||||
// slice into the arena so it survives.
|
||||
var viewer = try Viewer.init(testing.allocator);
|
||||
defer viewer.deinit();
|
||||
|
||||
try testViewer(&viewer, &.{
|
||||
// Startup sequence: block_end -> session_changed -> version -> list-windows
|
||||
.{ .input = .{ .tmux = .{ .block_end = "" } } },
|
||||
.{
|
||||
.input = .{ .tmux = .{ .session_changed = .{
|
||||
.id = 1,
|
||||
.name = "test",
|
||||
} } },
|
||||
.contains_command = "display-message",
|
||||
},
|
||||
.{
|
||||
.input = .{ .tmux = .{ .block_end = "3.5a" } },
|
||||
.contains_command = "list-windows",
|
||||
},
|
||||
// Receive list-windows response with a single window.
|
||||
// Format: session_id window_id width height layout
|
||||
.{
|
||||
.input = .{ .tmux = .{
|
||||
.block_end =
|
||||
\\$1 @5 120 40 ab00,120x40,0,0,3
|
||||
,
|
||||
} },
|
||||
.contains_tags = &.{ .windows, .command },
|
||||
.check = (struct {
|
||||
fn check(_: *Viewer, actions: []const Viewer.Action) anyerror!void {
|
||||
// Find the .windows action and dereference the slice
|
||||
// contents. Before the fix, this read freed memory.
|
||||
for (actions) |action| {
|
||||
if (action == .windows) {
|
||||
try testing.expectEqual(1, action.windows.len);
|
||||
try testing.expectEqual(5, action.windows[0].id);
|
||||
try testing.expectEqual(120, action.windows[0].width);
|
||||
try testing.expectEqual(40, action.windows[0].height);
|
||||
return;
|
||||
}
|
||||
}
|
||||
return error.TestExpectedEqual;
|
||||
}
|
||||
}).check,
|
||||
},
|
||||
.{
|
||||
.input = .{ .tmux = .exit },
|
||||
.contains_tags = &.{.exit},
|
||||
},
|
||||
});
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue