vim-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

0a6e57b09b

Co-authored-by: Christian Brabandt <cb@256bit.org>
This commit is contained in:
zeertzjq 2024-08-16 09:00:50 +08:00
parent 8b8096500d
commit b3d291c565
6 changed files with 50 additions and 18 deletions

View File

@ -203,6 +203,8 @@ void alist_set(alist_T *al, int count, char **files, int use_curbuf, int *fnum_l
/// Add file "fname" to argument list "al".
/// "fname" must have been allocated and "al" must have been checked for room.
///
/// May trigger Buf* autocommands
///
/// @param set_fnum 1: set buffer number; 2: re-use curbuf
void alist_add(alist_T *al, char *fname, int set_fnum)
{
@ -213,6 +215,7 @@ void alist_add(alist_T *al, char *fname, int set_fnum)
return;
}
arglist_locked = true;
curwin->w_locked = true;
#ifdef BACKSLASH_IN_FILENAME
slash_adjust(fname);
@ -225,6 +228,7 @@ void alist_add(alist_T *al, char *fname, int set_fnum)
al->al_ga.ga_len++;
arglist_locked = false;
curwin->w_locked = false;
}
#if defined(BACKSLASH_IN_FILENAME)
@ -352,12 +356,14 @@ static void alist_add_list(int count, char **files, int after, bool will_edit)
(size_t)(ARGCOUNT - after) * sizeof(aentry_T));
}
arglist_locked = true;
curwin->w_locked = true;
for (int i = 0; i < count; i++) {
const int flags = BLN_LISTED | (will_edit ? BLN_CURBUF : 0);
ARGLIST[after + i].ae_fname = files[i];
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;

View File

@ -1379,7 +1379,7 @@ static int do_buffer_ext(int action, int start, int dir, int count, int flags)
// When the autocommand window is involved win_close() may need to print an error message.
// 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)
&& (is_aucmd_win(lastwin) || !last_window(curwin))) {
if (win_close(curwin, false, false) == FAIL) {
break;
@ -3644,7 +3644,7 @@ void ex_buffer_all(exarg_T *eap)
: wp->w_width != Columns)
|| (had_tab > 0 && wp != firstwin))
&& !ONE_WINDOW
&& !(wp->w_closing || wp->w_buffer->b_locked > 0)
&& !(win_locked(curwin) || wp->w_buffer->b_locked > 0)
&& !is_aucmd_win(wp)) {
if (win_close(wp, false, false) == FAIL) {
break;

View File

@ -1045,7 +1045,7 @@ struct window_S {
win_T *w_prev; ///< link to previous window
win_T *w_next; ///< link to next window
bool w_closing; ///< window is being closed, don't let
bool w_locked; ///< window is being closed, don't let
///< autocommands close it too.
frame_T *w_frame; ///< frame containing this window

View File

@ -2349,7 +2349,7 @@ int do_ecmd(int fnum, char *ffname, char *sfname, exarg_T *eap, linenr_T newlnum
// Set w_closing to avoid that autocommands close the window.
// 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) {
@ -2365,7 +2365,7 @@ int do_ecmd(int fnum, char *ffname, char *sfname, exarg_T *eap, linenr_T newlnum
// Autocommands may have closed the window.
if (win_valid(the_curwin)) {
the_curwin->w_closing = false;
the_curwin->w_locked = false;
}
buf->b_locked--;

View File

@ -2471,7 +2471,7 @@ void close_windows(buf_T *buf, bool keep_curwin)
// When the autocommand window is involved win_close() may need to print an error message.
for (win_T *wp = lastwin; wp != NULL && (is_aucmd_win(lastwin) || !one_window(wp));) {
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, false) == FAIL) {
// If closing the window fails give up, to avoid looping forever.
break;
@ -2493,7 +2493,7 @@ void close_windows(buf_T *buf, bool keep_curwin)
// Start from tp_lastwin to close floating windows with the same buffer first.
for (win_T *wp = tp->tp_lastwin; wp != NULL; wp = wp->w_prev) {
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);
// Start all over, the tab page may be closed and
@ -2630,10 +2630,10 @@ static void win_close_buffer(win_T *win, int action, bool abort_if_last)
if (win->w_buffer != NULL) {
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
@ -2661,7 +2661,7 @@ int win_close(win_T *win, bool free_buf, bool force)
return FAIL;
}
if (win->w_closing
if (win_locked(win)
|| (win->w_buffer != NULL && win->w_buffer->b_locked > 0)) {
return FAIL; // window is already being closed
}
@ -2736,22 +2736,22 @@ int win_close(win_T *win, bool free_buf, bool force)
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(win)) {
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(win)) {
return FAIL;
}
@ -2798,9 +2798,6 @@ int win_close(win_T *win, bool free_buf, bool force)
// to split a window to avoid trouble.
split_disallowed++;
// let terminal buffers know that this window dimensions may be ignored
win->w_closing = true;
bool was_floating = win->w_floating;
if (ui_has(kUIMultigrid)) {
ui_call_win_close(win->w_grid_alloc.handle);
@ -2949,7 +2946,7 @@ void win_close_othertab(win_T *win, int free_buf, tabpage_T *tp)
{
// Get here with win->w_buffer == NULL when win_close() detects the tab page
// changed.
if (win->w_closing
if (win_locked(win)
|| (win->w_buffer != NULL && win->w_buffer->b_locked > 0)) {
return; // window is already being closed
}
@ -7439,6 +7436,12 @@ int get_last_winid(void)
return last_win_id;
}
/// Don't let autocommands close the given window
int win_locked(win_T *wp)
{
return wp->w_locked;
}
void win_get_tabwin(handle_T id, int *tabnr, int *winnr)
{
*tabnr = 0;

View File

@ -360,6 +360,7 @@ func Test_argv()
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 @@ func Test_all_command()
%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