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

6cb1c82840
(cherry picked from commit 627c648252)

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)) {

src/nvim/buffer_defs.h

@@ -366,7 +366,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
 

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"));
 

src/nvim/ex_cmds.c

@@ -2270,14 +2270,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) {

test/functional/api/window_spec.lua

@@ -1769,10 +1769,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
           .. '})'
       )

test/old/testdir/test_autocmd.vim

@@ -4174,7 +4174,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()

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