commit 627c648252e151266928bea501349af5eb24e134
parent 2c1f5a6aa587e02748ffe4f153d96703d861828b
Author: Sean Dewar <6256228+seandewar@users.noreply.github.com>
Date: Sun, 4 May 2025 03:15:51 +0100
vim-patch:9.1.1361: [security]: possible use-after-free when closing a buffer (#33820)
Problem: [security]: Possible to open more windows into a closing
buffer without splitting, bypassing existing "b_locked_split"
checks and triggering use-after-free
Solution: Disallow switching to a closing buffer. Editing a closing
buffer (via ":edit", etc.) was fixed in v9.1.0764, but add an
error message and check just "b_locked_split", as "b_locked"
is necessary only when the buffer shouldn't be wiped, and may
be set for buffers that are in-use but not actually closing.
(Sean Dewar)
closes: vim/vim#17246
https://github.com/vim/vim/commit/6cb1c828406dcbb9b67ee788501b94f3a0bac88a
Diffstat:
7 files changed, 56 insertions(+), 16 deletions(-)
diff --git a/src/nvim/buffer.c b/src/nvim/buffer.c
@@ -480,11 +480,6 @@ static bool can_unload_buffer(buf_T *buf)
return can_unload;
}
-bool buf_locked(buf_T *buf)
-{
- return buf->b_locked || buf->b_locked_split;
-}
-
/// Close the link to a buffer.
///
/// @param win If not NULL, set b_last_cursor.
@@ -1300,11 +1295,17 @@ static int do_buffer_ext(int action, int start, int dir, int count, int flags)
return FAIL;
}
- if (action == DOBUF_GOTO
- && buf != curbuf
- && !check_can_set_curbuf_forceit((flags & DOBUF_FORCEIT) ? true : false)) {
- // disallow navigating to another buffer when 'winfixbuf' is applied
- return FAIL;
+ if (action == DOBUF_GOTO && buf != curbuf) {
+ if (!check_can_set_curbuf_forceit((flags & DOBUF_FORCEIT) != 0)) {
+ // disallow navigating to another buffer when 'winfixbuf' is applied
+ return FAIL;
+ }
+ if (buf->b_locked_split) {
+ // disallow navigating to a closing buffer, which like splitting,
+ // can result in more windows displaying it
+ emsg(_(e_cannot_switch_to_a_closing_buffer));
+ return FAIL;
+ }
}
if ((action == DOBUF_GOTO || action == DOBUF_SPLIT) && (buf->b_flags & BF_DUMMY)) {
diff --git a/src/nvim/buffer_defs.h b/src/nvim/buffer_defs.h
@@ -368,7 +368,7 @@ struct file_buffer {
int b_locked; // Buffer is being closed or referenced, don't
// let autocommands wipe it out.
int b_locked_split; // Buffer is being closed, don't allow opening
- // a new window with it.
+ // it in more windows.
int b_ro_locked; // Non-zero when the buffer can't be changed.
// Used for FileChangedRO
diff --git a/src/nvim/errors.h b/src/nvim/errors.h
@@ -188,6 +188,7 @@ INIT(= N_("E5767: Cannot use :undo! to redo or move to a different undo branch")
EXTERN const char e_winfixbuf_cannot_go_to_buffer[]
INIT(= N_("E1513: Cannot switch buffer. 'winfixbuf' is enabled"));
EXTERN const char e_invalid_return_type_from_findfunc[] INIT( = N_("E1514: 'findfunc' did not return a List type"));
+EXTERN const char e_cannot_switch_to_a_closing_buffer[] INIT( = N_("E1546: Cannot switch to a closing buffer"));
EXTERN const char e_trustfile[] INIT(= N_("E5570: Cannot update trust file: %s"));
diff --git a/src/nvim/ex_cmds.c b/src/nvim/ex_cmds.c
@@ -2279,14 +2279,16 @@ int do_ecmd(int fnum, char *ffname, char *sfname, exarg_T *eap, linenr_T newlnum
if (buf == NULL) {
goto theend;
}
- // autocommands try to edit a file that is going to be removed, abort
- if (buf_locked(buf)) {
+ // autocommands try to edit a closing buffer, which like splitting, can
+ // result in more windows displaying it; abort
+ if (buf->b_locked_split) {
// window was split, but not editing the new buffer, reset b_nwindows again
if (oldwin == NULL
&& curwin->w_buffer != NULL
&& curwin->w_buffer->b_nwindows > 1) {
curwin->w_buffer->b_nwindows--;
}
+ emsg(_(e_cannot_switch_to_a_closing_buffer));
goto theend;
}
if (curwin->w_alt_fnum == buf->b_fnum && prev_alt_fnum != 0) {
diff --git a/test/functional/api/window_spec.lua b/test/functional/api/window_spec.lua
@@ -1992,10 +1992,10 @@ describe('API/win', function()
pcall_err(api.nvim_win_close, w, true)
)
- -- OK when using window to different buffer than `win`s.
+ -- OK when using a buffer that isn't closing.
w = api.nvim_get_current_win()
command(
- 'only | autocmd BufHidden * ++once call nvim_open_win(0, 0, #{split: "left", win: '
+ 'only | autocmd BufHidden * ++once call nvim_open_win(bufnr("#"), 0, #{split: "left", win: '
.. w
.. '})'
)
diff --git a/test/old/testdir/test_autocmd.vim b/test/old/testdir/test_autocmd.vim
@@ -4314,7 +4314,8 @@ func Test_autocmd_BufWinLeave_with_vsp()
exe "e " fname
vsp
augroup testing
- exe "au BufWinLeave " .. fname .. " :e " dummy .. "| vsp " .. fname
+ exe 'au BufWinLeave' fname 'e' dummy
+ \ '| call assert_fails(''vsp' fname ''', ''E1546:'')'
augroup END
bw
call CleanUpTestAuGroup()
diff --git a/test/old/testdir/test_buffer.vim b/test/old/testdir/test_buffer.vim
@@ -563,4 +563,39 @@ func Test_buflist_alloc_failure()
call assert_fails('cexpr "XallocFail6:10:Line10"', 'E342:')
endfunc
+func Test_closed_buffer_still_in_window()
+ %bw!
+
+ let s:w = win_getid()
+ new
+ let s:b = bufnr()
+ setl bufhidden=wipe
+
+ augroup ViewClosedBuffer
+ autocmd!
+ autocmd BufUnload * ++once call assert_fails(
+ \ 'call win_execute(s:w, "' .. s:b .. 'b")', 'E1546:')
+ augroup END
+ quit!
+ " Previously resulted in s:b being curbuf while unloaded (no memfile).
+ call assert_equal(1, bufloaded(bufnr()))
+ call assert_equal(0, bufexists(s:b))
+
+ let s:w = win_getid()
+ split
+ new
+ let s:b = bufnr()
+
+ augroup ViewClosedBuffer
+ autocmd!
+ autocmd BufWipeout * ++once call win_gotoid(s:w)
+ \| call assert_fails(s:b .. 'b', 'E1546:') | wincmd p
+ augroup END
+ bw! " Close only this buffer first; used to be a heap UAF.
+
+ unlet! s:w s:b
+ autocmd! ViewClosedBuffer
+ %bw!
+endfunc
+
" vim: shiftwidth=2 sts=2 expandtab
