From ab82b8ab720ce46183a58a55554e4a4a7423e3f5 Mon Sep 17 00:00:00 2001
From: Jon Parise <jon@indelible.org>
Date: Mon, 1 Jun 2026 19:58:20 -0400
Subject: [PATCH 1/2] core: fix use-after-free in Surface.setSelection

setSelection captured the previous selection, then called Screen.select
(which deinits the previous selection's tracked pins), then compared the
new selection against the now-freed previous pin via `sel.eql(prev)`.
That read freed pin memory (use-after-free).

The comparison was a copy-on-select optimization ("only re-copy if the
selection changed"). Remove it rather than repair it because:

- It never fired correctly. It compared against freed memory, so the
  shipped behavior was already "always copy".

- It can't be repaired by copying `prev`'s pin before Screen.select.
  That fixes the use-after-free but not the logic: the call sites (e.g.
  mouse drag release) pass a selection equal to the one already set, so
  a working `eql` skip would suppress the very copy those sites exist to
  perform. A correct optimization would have to compare against the
  last-copied selection (before the mouse event mutated the live one),
  which would require extra state.

- It isn't worth tracking that additional state. The copy runs once per
  selection gesture (mouse up, double-click), which isn't in a hot path,
  so skipping a redundant re-copy only saves a single clipboard write.

Removing the skip eliminates the use-after-free and keeps the behavior
consistent with what we've already been doing.
---
 src/Surface.zig | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/src/Surface.zig b/src/Surface.zig
index 410f717b0..99c740c89 100644
--- a/src/Surface.zig
+++ b/src/Surface.zig
@@ -2330,18 +2330,14 @@ fn copySelectionToClipboards(
 ///
 /// This must be called with the renderer mutex held.
 fn setSelection(self: *Surface, sel_: ?terminal.Selection) !void {
-    const prev_ = self.io.terminal.screens.active.selection;
     try self.io.terminal.screens.active.select(sel_);
 
     // If copy on select is false then exit early.
     if (self.config.copy_on_select == .false) return;
 
     // Set our selection clipboard. If the selection is cleared we do not
-    // clear the clipboard. If the selection is set, we only set the clipboard
-    // again if it changed, since setting the clipboard can be an expensive
-    // operation.
+    // clear the clipboard.
     const sel = sel_ orelse return;
-    if (prev_) |prev| if (sel.eql(prev)) return;
 
     switch (self.config.copy_on_select) {
         .false => unreachable, // handled above with an early exit

From 76b9bdb1999398fa1b64d000f9a77088af232b62 Mon Sep 17 00:00:00 2001
From: Jon Parise <jon@indelible.org>
Date: Mon, 1 Jun 2026 19:58:25 -0400
Subject: [PATCH 2/2] terminal: test Screen.select frees existing pins

---
 src/terminal/Screen.zig | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig
index becda78b7..8ee700252 100644
--- a/src/terminal/Screen.zig
+++ b/src/terminal/Screen.zig
@@ -7652,6 +7652,32 @@ test "Screen: select untracked" {
     try testing.expectEqual(tracked, s.pages.countTrackedPins());
 }
 
+test "Screen: select replaces existing pins" {
+    const testing = std.testing;
+    const alloc = testing.allocator;
+
+    var s = try init(alloc, .{ .cols = 10, .rows = 10, .max_scrollback = 0 });
+    defer s.deinit();
+    try s.testWriteString("ABC  DEF\n 123\n456");
+
+    const tracked = s.pages.countTrackedPins();
+    try s.select(Selection.init(
+        s.pages.pin(.{ .active = .{ .x = 0, .y = 0 } }).?,
+        s.pages.pin(.{ .active = .{ .x = 3, .y = 0 } }).?,
+        false,
+    ));
+    try testing.expectEqual(tracked + 2, s.pages.countTrackedPins());
+
+    // Replacing the selection must untrack the prior selection's pins
+    // rather than leak them.
+    try s.select(Selection.init(
+        s.pages.pin(.{ .active = .{ .x = 0, .y = 1 } }).?,
+        s.pages.pin(.{ .active = .{ .x = 2, .y = 1 } }).?,
+        false,
+    ));
+    try testing.expectEqual(tracked + 2, s.pages.countTrackedPins());
+}
+
 test "Screen: selectAll" {
     const testing = std.testing;
     const alloc = testing.allocator;