patch 9.1.1361: [security]: possible use-after-free when closing a buffer
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: #17246
Signed-off-by: Sean Dewar <6256228+seandewar@users.noreply.github.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
diff --git a/src/buffer.c b/src/buffer.c
index 697efa3..b4481b2 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -526,12 +526,6 @@
return can_unload;
}
- int
-buf_locked(buf_T *buf)
-{
- return buf->b_locked || buf->b_locked_split;
-}
-
/*
* Close the link to a buffer.
* "action" is used when there is no longer a window for the buffer.
@@ -1432,12 +1426,19 @@
if ((flags & DOBUF_NOPOPUP) && bt_popup(buf) && !bt_terminal(buf))
return OK;
#endif
- 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/errors.h b/src/errors.h
index 90cc2b1..d718507 100644
--- a/src/errors.h
+++ b/src/errors.h
@@ -3728,3 +3728,5 @@
EXTERN char e_no_quickfix_stack[]
INIT(= N_("E1545: Quickfix list stack unavailable"));
#endif
+EXTERN char e_cannot_switch_to_a_closing_buffer[]
+ INIT(= N_("E1546: Cannot switch to a closing buffer"));
diff --git a/src/ex_cmds.c b/src/ex_cmds.c
index e1e6c4e..4eb67e3 100644
--- a/src/ex_cmds.c
+++ b/src/ex_cmds.c
@@ -2743,9 +2743,9 @@
}
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
@@ -2753,6 +2753,7 @@
&& 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/src/proto/buffer.pro b/src/proto/buffer.pro
index 8384601..9044bae 100644
--- a/src/proto/buffer.pro
+++ b/src/proto/buffer.pro
@@ -5,7 +5,6 @@
void set_bufref(bufref_T *bufref, buf_T *buf);
int bufref_valid(bufref_T *bufref);
int buf_valid(buf_T *buf);
-int buf_locked(buf_T *buf);
int close_buffer(win_T *win, buf_T *buf, int action, int abort_if_last, int ignore_abort);
void buf_clear_file(buf_T *buf);
void buf_freeall(buf_T *buf, int flags);
diff --git a/src/structs.h b/src/structs.h
index 898f620..caf61ed 100644
--- a/src/structs.h
+++ b/src/structs.h
@@ -3072,7 +3072,7 @@
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.
/*
* b_ffname has the full path of the file (NULL for no name).
diff --git a/src/testdir/test_autocmd.vim b/src/testdir/test_autocmd.vim
index d5d516c..8d083db 100644
--- a/src/testdir/test_autocmd.vim
+++ b/src/testdir/test_autocmd.vim
@@ -5035,7 +5035,8 @@
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/src/testdir/test_buffer.vim b/src/testdir/test_buffer.vim
index 757ba05..36a6ef5 100644
--- a/src/testdir/test_buffer.vim
+++ b/src/testdir/test_buffer.vim
@@ -569,4 +569,39 @@
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
diff --git a/src/version.c b/src/version.c
index c0c98b4..7d29d70 100644
--- a/src/version.c
+++ b/src/version.c
@@ -705,6 +705,8 @@
static int included_patches[] =
{ /* Add new patch number below this line */
/**/
+ 1361,
+/**/
1360,
/**/
1359,