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 187e16e8354b14805f0217dfed4a3351097c6905..8825c8e252ccc574271de1dc1e8900eb163ac28f 100644 (file)
--- a/src/arglist.c
+++ b/src/arglist.c
@@ -184,6 +184,8 @@ alist_set(
 /*
  * 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 @@ alist_add(
     if (check_arglist_locked() == FAIL)
        return;
     arglist_locked = TRUE;
+    curwin->w_locked = TRUE;
 
 #ifdef BACKSLASH_IN_FILENAME
     slash_adjust(fname);
@@ -207,6 +210,7 @@ alist_add(
     ++al->al_ga.ga_len;
 
     arglist_locked = FALSE;
+    curwin->w_locked = FALSE;
 }
 
 #if defined(BACKSLASH_IN_FILENAME) || defined(PROTO)
@@ -365,6 +369,7 @@ alist_add_list(
            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 @@ alist_add_list(
            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 447ce76d49a328fd843e8408a48fa5a78e99d5c0..34500e4abc28215cb85574147ba34b7dceac6bed 100644 (file)
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -1484,7 +1484,7 @@ do_buffer_ext(
        // (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 @@ 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(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 05778c8fd8b9c6f38ff3225cd9eb221a6a96c31c..349269a2bb8b69635aeddb4e97aa8779cf33ad30 100644 (file)
--- a/src/ex_cmds.c
+++ b/src/ex_cmds.c
@@ -2840,7 +2840,7 @@ do_ecmd(
 
                // 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 @@ do_ecmd(
 
                // 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 26c7040b8a1b4d357333f9cfd6d2a441a616b693..441070ebfcb8e8dcf1df27d64fe95231dd9ab916 100644 (file)
--- a/src/proto/window.pro
+++ b/src/proto/window.pro
@@ -100,4 +100,5 @@ 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);
+int win_locked(win_T *wp);
 /* vim: set ft=c : */

diff --git a/src/structs.h b/src/structs.h
index fe4704a367949ca7c94aff18630a6c1f2e5667b9..abda3a0c38b4eaf5cf3f9d0dc77d44ea0fc53db4 100644 (file)
--- a/src/structs.h
+++ b/src/structs.h
@@ -3785,7 +3785,7 @@ struct window_S
     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 1fc0ef96881f9516fe67b6b7e46efea94b3f8d83..f80196096df49e29b08250c376ad3fbbfc599d93 100644 (file)
--- a/src/terminal.c
+++ b/src/terminal.c
@@ -3680,10 +3680,10 @@ term_after_channel_closed(term_T *term)
                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 edc8b77429e20ac6c68c5434bb35a621b9888484..8d81a828b3e03dcc94ea45bab5e55e81ae0a668d 100644 (file)
--- a/src/testdir/test_arglist.vim
+++ b/src/testdir/test_arglist.vim
@@ -359,6 +359,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

diff --git a/src/version.c b/src/version.c
index 78ccd20e5a1aa602f9e9878b68cc90c60f7df4b1..08b3fafd7b4527bc15bf3083b4bb7055a46bb8f8 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 */
+/**/
+    678,
 /**/
     677,
 /**/

diff --git a/src/window.c b/src/window.c
index 43a15e0561f2cbd7cfc0d6526f038bfabeb1fc39..b2c90c7d64114ded5ed41631b23eb45d9a95b066 100644 (file)
--- a/src/window.c
+++ b/src/window.c
@@ -2511,7 +2511,7 @@ close_windows(
     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 @@ close_windows(
        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 @@ win_close_buffer(win_T *win, int action, int abort_if_last)
        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 @@ win_close(win_T *win, int free_buf)
     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 @@ win_close(win_T *win, int free_buf)
            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 @@ 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 || (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 @@ 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;
+}