patch 9.1.2058: b_locked_split is not checked for :sbuffer

Problem:  b_locked_split is not checked for :sbuffer, which allows
          autocommands to leave windows open to freed buffers.
Solution: In do_buffer_ext, check just before possibly splitting, after
          handling 'switchbuf'. Leave win_split to handle the check for
          curbuf. (needed even if curbuf is not the target, as setting
          the buffer after splitting may fail) (Sean Dewar)

closes: #19096

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 1d624ae919817950b055c22d5e0b5029aad3942b..87b87272a801bd3aaba0369434d6a1865a15bb2f 100644 (file)
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -1434,19 +1434,10 @@ do_buffer_ext(
     if ((flags & DOBUF_NOPOPUP) && bt_popup(buf) && !bt_terminal(buf))
        return OK;
 #endif
-    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 && buf != curbuf
+           && !check_can_set_curbuf_forceit((flags & DOBUF_FORCEIT) != 0))
+       // disallow navigating to another buffer when 'winfixbuf' is applied
+       return FAIL;
 
     if ((action == DOBUF_GOTO || action == DOBUF_SPLIT)
                                                  && (buf->b_flags & BF_DUMMY))
@@ -1654,15 +1645,17 @@ do_buffer_ext(
     /*
      * make "buf" the current buffer
      */
-    if (action == DOBUF_SPLIT)     // split window first
+    // If 'switchbuf' is set jump to the window containing "buf".
+    if (action == DOBUF_SPLIT && swbuf_goto_win_with_buf(buf) != NULL)
+       return OK;
+    // Whether splitting or not, don't open a closing buffer in more windows.
+    if (buf != curbuf && buf->b_locked_split)
     {
-       // If 'switchbuf' is set jump to the window containing "buf".
-       if (swbuf_goto_win_with_buf(buf) != NULL)
-           return OK;
-
-       if (win_split(0, 0) == FAIL)
-           return FAIL;
+       emsg(_(e_cannot_switch_to_a_closing_buffer));
+       return FAIL;
     }
+    if (action == DOBUF_SPLIT && win_split(0, 0) == FAIL) // split window first
+       return FAIL;
 
     // go to current buffer - nothing to do
     if (buf == curbuf)

diff --git a/src/testdir/test_buffer.vim b/src/testdir/test_buffer.vim
index 2113feddfca070ea345db98b539e5210841fb0be..3fdb5fc4e1e49a8b2b63ef2092429f7e651c9f39 100644 (file)
--- a/src/testdir/test_buffer.vim
+++ b/src/testdir/test_buffer.vim
@@ -589,34 +589,89 @@ endfunc
 func Test_closed_buffer_still_in_window()
   %bw!
 
+  let s:fired = 0
   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:')
+    autocmd BufUnload * ++once let s:fired += 1 | call assert_fails(
+          \ 'call win_execute(s:w, "' .. s:b .. 'buffer")', 'E1546:')
   augroup END
   quit!
   " Previously resulted in s:b being curbuf while unloaded (no memfile).
+  call assert_equal(1, s:fired)
   call assert_equal(1, bufloaded(bufnr()))
   call assert_equal(0, bufexists(s:b))
+  %bw!
+
+  new flobby
+  let s:w = win_getid()
+  let s:b = bufnr()
+  setl bufhidden=wipe
+  augroup ViewClosedBuffer
+    autocmd!
+    autocmd BufUnload * ++once let s:fired += 1
+          \| wincmd p
+          \| call assert_notequal(s:w, win_getid())
+          \| call assert_notequal(s:b, bufnr())
+          \| execute s:b 'sbuffer'
+          \| call assert_equal(s:w, win_getid())
+          \| call assert_equal(s:b, bufnr())
+  augroup END
+  " Not a problem if 'switchbuf' switches to an existing window instead.
+  set switchbuf=useopen
+  quit!
+  call assert_equal(2, s:fired)
+  call assert_equal(0, bufexists(s:b))
+  set switchbuf&
+  %bw!
+
+  edit floob
+  let s:b = bufnr()
+  enew
+  augroup ViewClosedBuffer
+    autocmd!
+    autocmd BufWipeout * ++once let s:fired += 1
+          \| call assert_fails(s:b .. 'sbuffer | wincmd p', 'E1546:')
+          \| call assert_equal(1, winnr('$')) " :sbuffer shouldn't have split.
+  augroup END
+  " Used to be a heap UAF.
+  execute s:b 'bwipeout!'
+  call assert_equal(3, s:fired)
+  call assert_equal(0, bufexists(s:b))
+  %bw!
+
+  edit flarb
+  let s:b = bufnr()
+  enew
+  let b2 = bufnr()
+  augroup ViewClosedBuffer
+    autocmd!
+    autocmd BufWipeout * ++once let s:fired += 1
+          \| call assert_fails(s:b .. 'sbuffer', 'E1159:')
+  augroup END
+  " :sbuffer still should fail if curbuf is closing, even if it's not the target
+  " buffer (as switching buffers can fail after the split)
+  bwipeout!
+  call assert_equal(4, s:fired)
+  call assert_equal(0, bufexists(b2))
+  %bw!
 
   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
+    autocmd BufWipeout * ++once let s:fired += 1 | call win_gotoid(s:w)
+          \| call assert_fails(s:b .. 'buffer', 'E1546:') | wincmd p
   augroup END
   bw! " Close only this buffer first; used to be a heap UAF.
+  call assert_equal(5, s:fired)
 
-  unlet! s:w s:b
+  unlet! s:w s:b s:fired
   autocmd! ViewClosedBuffer
   %bw!
 endfunc

diff --git a/src/version.c b/src/version.c
index e4bee056d84b958af7f48d1ecc9ace7f28a4bc4e..22df517ce1113d7b3287089ee383b5305d03ca67 100644 (file)
--- 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 */
+/**/
+    2058,
 /**/
     2057,
 /**/