patch 9.1.0678: [security]: use-after-free in alist_add()
Problem: [security]: use-after-free in alist_add()
(SuyueGuo)
Solution: Lock the current window, so that the reference to
the argument list remains valid.
This fixes CVE-2024-43374
Signed-off-by: Christian Brabandt <cb@256bit.org>
diff --git a/src/arglist.c b/src/arglist.c
index 187e16e..8825c8e 100644
--- a/src/arglist.c
+++ b/src/arglist.c
@@ -184,6 +184,8 @@
/*
* Add file "fname" to argument list "al".
* "fname" must have been allocated and "al" must have been checked for room.
+ *
+ * May trigger Buf* autocommands
*/
void
alist_add(
@@ -196,6 +198,7 @@
if (check_arglist_locked() == FAIL)
return;
arglist_locked = TRUE;
+ curwin->w_locked = TRUE;
#ifdef BACKSLASH_IN_FILENAME
slash_adjust(fname);
@@ -207,6 +210,7 @@
++al->al_ga.ga_len;
arglist_locked = FALSE;
+ curwin->w_locked = FALSE;
}
#if defined(BACKSLASH_IN_FILENAME) || defined(PROTO)
@@ -365,6 +369,7 @@
mch_memmove(&(ARGLIST[after + count]), &(ARGLIST[after]),
(ARGCOUNT - after) * sizeof(aentry_T));
arglist_locked = TRUE;
+ curwin->w_locked = TRUE;
for (i = 0; i < count; ++i)
{
int flags = BLN_LISTED | (will_edit ? BLN_CURBUF : 0);
@@ -373,6 +378,7 @@
ARGLIST[after + i].ae_fnum = buflist_add(files[i], flags);
}
arglist_locked = FALSE;
+ curwin->w_locked = FALSE;
ALIST(curwin)->al_ga.ga_len += count;
if (old_argcount > 0 && curwin->w_arg_idx >= after)
curwin->w_arg_idx += count;
diff --git a/src/buffer.c b/src/buffer.c
index 447ce76..34500e4 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -1484,7 +1484,7 @@
// (unless it's the only window). Repeat this so long as we end up in
// a window with this buffer.
while (buf == curbuf
- && !(curwin->w_closing || curwin->w_buffer->b_locked > 0)
+ && !(win_locked(curwin) || curwin->w_buffer->b_locked > 0)
&& (!ONE_WINDOW || first_tabpage->tp_next != NULL))
{
if (win_close(curwin, FALSE) == FAIL)
@@ -5470,7 +5470,7 @@
: wp->w_width != Columns)
|| (had_tab > 0 && wp != firstwin))
&& !ONE_WINDOW
- && !(wp->w_closing || wp->w_buffer->b_locked > 0)
+ && !(win_locked(wp) || wp->w_buffer->b_locked > 0)
&& !win_unlisted(wp))
{
if (win_close(wp, FALSE) == FAIL)
diff --git a/src/ex_cmds.c b/src/ex_cmds.c
index 05778c8..349269a 100644
--- a/src/ex_cmds.c
+++ b/src/ex_cmds.c
@@ -2840,7 +2840,7 @@
// Set the w_closing flag to avoid that autocommands close the
// window. And set b_locked for the same reason.
- the_curwin->w_closing = TRUE;
+ the_curwin->w_locked = TRUE;
++buf->b_locked;
if (curbuf == old_curbuf.br_buf)
@@ -2854,7 +2854,7 @@
// Autocommands may have closed the window.
if (win_valid(the_curwin))
- the_curwin->w_closing = FALSE;
+ the_curwin->w_locked = FALSE;
--buf->b_locked;
#ifdef FEAT_EVAL
diff --git a/src/proto/window.pro b/src/proto/window.pro
index 26c7040..441070e 100644
--- a/src/proto/window.pro
+++ b/src/proto/window.pro
@@ -100,4 +100,5 @@
int get_tab_number(tabpage_T *tp);
char *check_colorcolumn(win_T *wp);
int get_last_winid(void);
+int win_locked(win_T *wp);
/* vim: set ft=c : */
diff --git a/src/structs.h b/src/structs.h
index fe4704a..abda3a0 100644
--- a/src/structs.h
+++ b/src/structs.h
@@ -3785,7 +3785,7 @@
synblock_T *w_s; // for :ownsyntax
#endif
- int w_closing; // window is being closed, don't let
+ int w_locked; // window is being closed, don't let
// autocommands close it too.
frame_T *w_frame; // frame containing this window
diff --git a/src/terminal.c b/src/terminal.c
index 1fc0ef9..f801960 100644
--- a/src/terminal.c
+++ b/src/terminal.c
@@ -3680,10 +3680,10 @@
if (is_aucmd_win(curwin))
do_set_w_closing = TRUE;
if (do_set_w_closing)
- curwin->w_closing = TRUE;
+ curwin->w_locked = TRUE;
do_bufdel(DOBUF_WIPE, (char_u *)"", 1, fnum, fnum, FALSE);
if (do_set_w_closing)
- curwin->w_closing = FALSE;
+ curwin->w_locked = FALSE;
aucmd_restbuf(&aco);
}
#ifdef FEAT_PROP_POPUP
diff --git a/src/testdir/test_arglist.vim b/src/testdir/test_arglist.vim
index edc8b77..8d81a82 100644
--- a/src/testdir/test_arglist.vim
+++ b/src/testdir/test_arglist.vim
@@ -359,6 +359,7 @@
call assert_equal('', argv(1, 100))
call assert_equal([], argv(-1, 100))
call assert_equal('', argv(10, -1))
+ %argdelete
endfunc
" Test for the :argedit command
@@ -744,4 +745,26 @@
%bw!
endfunc
+" Test for deleting buffer when creating an arglist. This was accessing freed
+" memory
+func Test_crash_arglist_uaf()
+ "%argdelete
+ new one
+ au BufAdd XUAFlocal :bw
+ "call assert_fails(':arglocal XUAFlocal', 'E163:')
+ arglocal XUAFlocal
+ au! BufAdd
+ bw! XUAFlocal
+
+ au BufAdd XUAFlocal2 :bw
+ new two
+ new three
+ arglocal
+ argadd XUAFlocal2 Xfoobar
+ bw! XUAFlocal2
+ bw! two
+
+ au! BufAdd
+endfunc
+
" vim: shiftwidth=2 sts=2 expandtab
diff --git a/src/version.c b/src/version.c
index 78ccd20..08b3faf 100644
--- a/src/version.c
+++ b/src/version.c
@@ -705,6 +705,8 @@
static int included_patches[] =
{ /* Add new patch number below this line */
/**/
+ 678,
+/**/
677,
/**/
676,
diff --git a/src/window.c b/src/window.c
index 43a15e0..b2c90c7 100644
--- a/src/window.c
+++ b/src/window.c
@@ -2511,7 +2511,7 @@
for (wp = firstwin; wp != NULL && !ONE_WINDOW; )
{
if (wp->w_buffer == buf && (!keep_curwin || wp != curwin)
- && !(wp->w_closing || wp->w_buffer->b_locked > 0))
+ && !(win_locked(wp) || wp->w_buffer->b_locked > 0))
{
if (win_close(wp, FALSE) == FAIL)
// If closing the window fails give up, to avoid looping
@@ -2532,7 +2532,7 @@
if (tp != curtab)
FOR_ALL_WINDOWS_IN_TAB(tp, wp)
if (wp->w_buffer == buf
- && !(wp->w_closing || wp->w_buffer->b_locked > 0))
+ && !(win_locked(wp) || wp->w_buffer->b_locked > 0))
{
win_close_othertab(wp, FALSE, tp);
@@ -2654,10 +2654,10 @@
bufref_T bufref;
set_bufref(&bufref, curbuf);
- win->w_closing = TRUE;
+ win->w_locked = TRUE;
close_buffer(win, win->w_buffer, action, abort_if_last, TRUE);
if (win_valid_any_tab(win))
- win->w_closing = FALSE;
+ win->w_locked = FALSE;
// Make sure curbuf is valid. It can become invalid if 'bufhidden' is
// "wipe".
if (!bufref_valid(&bufref))
@@ -2705,7 +2705,7 @@
if (window_layout_locked(CMD_close))
return FAIL;
- if (win->w_closing || (win->w_buffer != NULL
+ if (win_locked(win) || (win->w_buffer != NULL
&& win->w_buffer->b_locked > 0))
return FAIL; // window is already being closed
if (win_unlisted(win))
@@ -2754,19 +2754,19 @@
other_buffer = TRUE;
if (!win_valid(win))
return FAIL;
- win->w_closing = TRUE;
+ win->w_locked = TRUE;
apply_autocmds(EVENT_BUFLEAVE, NULL, NULL, FALSE, curbuf);
if (!win_valid(win))
return FAIL;
- win->w_closing = FALSE;
+ win->w_locked = FALSE;
if (last_window())
return FAIL;
}
- win->w_closing = TRUE;
+ win->w_locked = TRUE;
apply_autocmds(EVENT_WINLEAVE, NULL, NULL, FALSE, curbuf);
if (!win_valid(win))
return FAIL;
- win->w_closing = FALSE;
+ win->w_locked = FALSE;
if (last_window())
return FAIL;
#ifdef FEAT_EVAL
@@ -3346,7 +3346,7 @@
// Get here with win->w_buffer == NULL when win_close() detects the tab
// page changed.
- if (win->w_closing || (win->w_buffer != NULL
+ if (win_locked(win) || (win->w_buffer != NULL
&& win->w_buffer->b_locked > 0))
return; // window is already being closed
@@ -8000,3 +8000,12 @@
{
return last_win_id;
}
+
+/*
+ * Don't let autocommands close the given window
+ */
+ int
+win_locked(win_T *wp)
+{
+ return wp->w_locked;
+}