patch 9.1.0143: [security]: autocmd causes use-after-free in set_curbuf()

Problem:  [security]: autocmd cause use-after-free in set_curbuf()
          (kawarimidoll)
Solution: check side-effect of BufLeave autocommand, when the number
          of windows changed, close windows containing buffers that will
          be wiped, if curbuf changed unexpectedly make sure b_nwindows
          is decremented otherwise it cannot be wiped

set_curbuf() already makes some efforts to ensure the BufLeave
autocommands do not cause issues.  However there are still 2 issues
that are not taken care of:

1) If a BufLeave autocommand opens a new window containing the same
buffer as that is going got be closed in close_buffer() a bit later,
we suddenly have another window open, containing a free'd buffer.  So we
must check if the number of windows changed and if it does (and the
current buffer is going to be wiped (according to the 'bufhidden'
setting), let's immediately close all windows containing the current
buffer using close_windows()

2) If a BufLeave autocommand changes our current buffer (displays it in
the current window), buf->b_nwindow will be incremented. As part of
set_curbuf() we will however enter another buffer soon, which means, the
newly created curbuf will have b_nwindows still have set, even so the
buffer is no longer displayed in a window. This causes later problems,
because it will no longer be possible to wipe such a buffer. So just
before entering the final buffer, check if the curbuf changed when
calling the BufLeave autocommand and if it does (and curbuf is still
valid), decrement curbuf->b_nwindows.

Both issues can be verified using the provided test (however the second
issue only because such an impacted buffer won't be wiped, causing
futher issues in later tests).

fixes: #13839
closes: #14104

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

diff --git a/src/buffer.c b/src/buffer.c
index b17ae1652191fdac4b9b9385dc620a41e8d9ebb1..8d62e643686f392aa0a5f6b03069677478560c4b 100644 (file)
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -1790,6 +1790,7 @@ set_curbuf(buf_T *buf, int action)
     bufref_T   newbufref;
     bufref_T   prevbufref;
     int                valid;
+    int                last_winid = get_last_winid();
 
     setpcmark();
     if ((cmdmod.cmod_flags & CMOD_KEEPALT) == 0)
@@ -1818,7 +1819,11 @@ set_curbuf(buf_T *buf, int action)
        if (prevbuf == curwin->w_buffer)
            reset_synblock(curwin);
 #endif
-       if (unload)
+       // autocommands may have opened a new window
+       // with prevbuf, grr
+       if (unload ||
+           (last_winid != get_last_winid() &&
+            strchr((char *)"wdu", prevbuf->b_p_bh[0]) != NULL))
            close_windows(prevbuf, FALSE);
 #if defined(FEAT_EVAL)
        if (bufref_valid(&prevbufref) && !aborting())
@@ -1854,6 +1859,10 @@ set_curbuf(buf_T *buf, int action)
 #endif
        ) || curwin->w_buffer == NULL)
     {
+       // autocommands changed curbuf and we will move to another
+       // buffer soon, so decrement curbuf->b_nwindows
+       if (curbuf != NULL && prevbuf != curbuf)
+           curbuf->b_nwindows--;
        // If the buffer is not valid but curwin->w_buffer is NULL we must
        // enter some buffer.  Using the last one is hopefully OK.
        if (!valid)

diff --git a/src/proto/window.pro b/src/proto/window.pro
index a91a51190bbd7f87d0cf6d428ce91bfdfb9456f8..e5c03969fb4ebefb4fd141458d44beae4b72a67e 100644 (file)
--- a/src/proto/window.pro
+++ b/src/proto/window.pro
@@ -96,4 +96,5 @@ int win_hasvertsplit(void);
 int get_win_number(win_T *wp, win_T *first_win);
 int get_tab_number(tabpage_T *tp);
 char *check_colorcolumn(win_T *wp);
+int get_last_winid(void);
 /* vim: set ft=c : */

diff --git a/src/testdir/test_autocmd.vim b/src/testdir/test_autocmd.vim
index 90dc3c1d32d66c67ae254fb6d27914d70a62c50f..f4e3c15a61cf35ec565a2a835af5849982640663 100644 (file)
--- a/src/testdir/test_autocmd.vim
+++ b/src/testdir/test_autocmd.vim
@@ -4441,4 +4441,16 @@ func Test_autocmd_invalidates_undo_on_textchanged()
   call StopVimInTerminal(buf)
 endfunc
 
+func Test_autocmd_creates_new_buffer_on_bufleave()
+  e a.txt
+  e b.txt
+  setlocal bufhidden=wipe
+  autocmd BufLeave <buffer> diffsplit c.txt
+  bn
+  call assert_equal(1, winnr('$'))
+  call assert_equal('a.txt', bufname('%'))
+  bw a.txt
+  bw c.txt
+endfunc
+
 " vim: shiftwidth=2 sts=2 expandtab

diff --git a/src/version.c b/src/version.c
index c6c536ab197a238b1ed64cd4e17f644acaa3abeb..cc2d41247124b896b759f2c30dfd0f38b12710b5 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -704,6 +704,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    143,
 /**/
     142,
 /**/

diff --git a/src/window.c b/src/window.c
index 7123780c5239cdf525c43b03b0e1226d41d48f59..bda24fc4e517ad92c18c52fcbff5e11f353989a2 100644 (file)
--- a/src/window.c
+++ b/src/window.c
@@ -7941,3 +7941,9 @@ skip:
     return NULL;  // no error
 }
 #endif
+
+    int
+get_last_winid(void)
+{
+    return last_win_id;
+}