Commit: patch 9.1.2085: Use-after-free in winframe_remove()

[Christian Brabandt]

patch 9.1.2085: Use-after-free in winframe_remove()

Commit: https://github.com/vim/vim/commit/ead1dda74a485ef0470e7252d07c1a36b8cde517
Author: Christian Brabandt <c...@256bit.org>
Date: Tue Jan 13 21:40:40 2026 +0000

patch 9.1.2085: Use-after-free in winframe_remove()

Problem: Use-after-free in winframe_remove() (henices)
Solution: Set window_layout_locked() inside winframe_remove()
and check that writing diff files is disallowed when the
window layout is locked.

It can happen with a custom diff expression when removing a window:

1. Buffer was removed, so win_frame_remove() is called to remove the
window.
2. win_frame_remove() → frame_new_height() → scroll_to_fraction()
→ diff_check_fill() (checks for filler lines)
3. diff_check_fill() ends up causing a diff_try_update, and because we
are not using internal diff, it has to first write the file to a
buffer using buf_write()
4. buf_write() is called for a buffer that is not contained within a
window, so it first calls aucmd_prepbuf() to create a new temporary
window before writing the buffer and then later calls
aucmd_restbuf(), which restores the previous window layout, calling
winframe_remove() again, which will free the window/frame structure,
eventually freeing stuff that will still be accessed at step 2.

closes: #19064

Signed-off-by: Christian Brabandt <c...@256bit.org>

diff --git a/src/diff.c b/src/diff.c
index 46325d64c..8b2e83e79 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -898,6 +898,13 @@ diff_write(buf_T *buf, diffin_T *din, linenr_T start, linenr_T end)
if (din->din_fname == NULL)
return diff_write_buffer(buf, din, start, end);

+ // Writing the diff buffers may trigger changes in the window structure
+ // via aucmd_prepbuf()/aucmd_restbuf() commands.
+ // This may cause recursively calling winframe_remove() which is not safe and causes
+ // use after free, so let's stop it here.
+ if (frames_locked())
+ return FAIL;
+
if (end < 0)
end = buf->b_ml.ml_line_count;

diff --git a/src/proto/window.pro b/src/proto/window.pro
index 44e4201fa..939c22076 100644
--- a/src/proto/window.pro
+++ b/src/proto/window.pro
@@ -1,6 +1,7 @@
/* window.c */
void window_layout_lock(void);
void window_layout_unlock(void);
+int frames_locked(void);
int window_layout_locked(enum CMD_index cmd);
int check_can_set_curbuf_disabled(void);
int check_can_set_curbuf_forceit(int forceit);
diff --git a/src/testdir/test_diffmode.vim b/src/testdir/test_diffmode.vim
index 340397a27..7e93ea363 100644
--- a/src/testdir/test_diffmode.vim
+++ b/src/testdir/test_diffmode.vim
@@ -3566,4 +3566,36 @@ func Test_diff_add_prop_in_autocmd()
call StopVimInTerminal(buf)
endfunc

+" this was causing a use-after-free by callig winframe_remove() rerursively
+func Test_diffexpr_wipe_buffers()
+ CheckRunVimInTerminal
+
+ let lines =<< trim END
+ def DiffFuncExpr()
+ var in: list<string> = readfile(v:fname_in)
+ var new = readfile(v:fname_new)
+ var out: string = diff(in, new)
+ writefile(split(out, "n"), v:fname_out)
+ enddef
+
+ new
+ vnew
+ set diffexpr=DiffFuncExpr()
+ wincmd l
+ new
+ cal setline(1,range(20))
+ wind difft
+ wincm w
+ hid
+ %bw!
+ END
+ call writefile(lines, 'Xtest_diffexpr_wipe', 'D')
+
+ let buf = RunVimInTerminal('Xtest_diffexpr_wipe', {})
+ call term_sendkeys(buf, ":so\<CR>")
+ call WaitForAssert({-> assert_match('4 buffers wiped out', term_getline(buf, 20))})
+
+ call StopVimInTerminal(buf)
+endfunc
+
" vim: shiftwidth=2 sts=2 expandtab
diff --git a/src/version.c b/src/version.c
index 0d09cef03..3e3bae84b 100644
--- a/src/version.c
+++ b/src/version.c
@@ -734,6 +734,8 @@ static char *(features[]) =

static int included_patches[] =
{ /* Add new patch number below this line */
+/**/
+ 2085,
/**/
2084,
/**/
diff --git a/src/window.c b/src/window.c
index 491b74e23..f4909b7f6 100644
--- a/src/window.c
+++ b/src/window.c
@@ -90,6 +90,10 @@ static int split_disallowed = 0;
// autocommands mess up the window structure.
static int close_disallowed = 0;

+// When non-zero changing the window frame structure is forbidden. Used
+// to avoid that winframe_remove() is called recursively
+static int frame_locked = 0;
+
/*
* Disallow changing the window layout (split window, close window, move
* window). Resizing is still allowed.
@@ -111,6 +115,12 @@ window_layout_unlock(void)
--close_disallowed;
}

+ int
+frames_locked(void)
+{
+ return frame_locked;
+}
+
/*
* When the window layout cannot be changed give an error and return TRUE.
* "cmd" indicates the action being performed and is used to pick the relevant
@@ -3577,6 +3587,8 @@ winframe_remove(
if (tp == NULL ? ONE_WINDOW : tp->tp_firstwin == tp->tp_lastwin)
return NULL;

+ frame_locked++;
+
// Save the position of the containing frame (which will also contain the
// altframe) before we remove anything, to recompute window positions later.
wp = frame2win(frp_close->fr_parent);
@@ -3682,6 +3694,8 @@ winframe_remove(
else
*unflat_altfr = frp2;

+ frame_locked--;
+
return wp;
}