|
| 1 | +From 0a6e57b09bc8c76691b367a5babfb79b31b770e8 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Christian Brabandt <cb@256bit.org> |
| 3 | +Date: Thu, 15 Aug 2024 22:15:28 +0200 |
| 4 | +Subject: [PATCH] patch 9.1.0678: [security]: use-after-free in alist_add() |
| 5 | + |
| 6 | +Problem: [security]: use-after-free in alist_add() |
| 7 | + (SuyueGuo) |
| 8 | +Solution: Lock the current window, so that the reference to |
| 9 | + the argument list remains valid. |
| 10 | + |
| 11 | +This fixes CVE-2024-43374 |
| 12 | + |
| 13 | +Signed-off-by: Christian Brabandt <cb@256bit.org> |
| 14 | +--- |
| 15 | + src/arglist.c | 6 ++++++ |
| 16 | + src/buffer.c | 4 ++-- |
| 17 | + src/ex_cmds.c | 4 ++-- |
| 18 | + src/proto/window.pro | 1 + |
| 19 | + src/structs.h | 2 +- |
| 20 | + src/terminal.c | 4 ++-- |
| 21 | + src/testdir/test_arglist.vim | 23 +++++++++++++++++++++++ |
| 22 | + src/version.c | 2 ++ |
| 23 | + src/window.c | 29 +++++++++++++++++++---------- |
| 24 | + 9 files changed, 58 insertions(+), 17 deletions(-) |
| 25 | + |
| 26 | +diff --git a/src/arglist.c b/src/arglist.c |
| 27 | +index 187e16e8354b1..8825c8e252ccc 100644 |
| 28 | +--- a/src/arglist.c |
| 29 | ++++ b/src/arglist.c |
| 30 | +@@ -184,6 +184,8 @@ alist_set( |
| 31 | + /* |
| 32 | + * Add file "fname" to argument list "al". |
| 33 | + * "fname" must have been allocated and "al" must have been checked for room. |
| 34 | ++ * |
| 35 | ++ * May trigger Buf* autocommands |
| 36 | + */ |
| 37 | + void |
| 38 | + alist_add( |
| 39 | +@@ -196,6 +198,7 @@ alist_add( |
| 40 | + if (check_arglist_locked() == FAIL) |
| 41 | + return; |
| 42 | + arglist_locked = TRUE; |
| 43 | ++ curwin->w_locked = TRUE; |
| 44 | + |
| 45 | + #ifdef BACKSLASH_IN_FILENAME |
| 46 | + slash_adjust(fname); |
| 47 | +@@ -207,6 +210,7 @@ alist_add( |
| 48 | + ++al->al_ga.ga_len; |
| 49 | + |
| 50 | + arglist_locked = FALSE; |
| 51 | ++ curwin->w_locked = FALSE; |
| 52 | + } |
| 53 | + |
| 54 | + #if defined(BACKSLASH_IN_FILENAME) || defined(PROTO) |
| 55 | +@@ -365,6 +369,7 @@ alist_add_list( |
| 56 | + mch_memmove(&(ARGLIST[after + count]), &(ARGLIST[after]), |
| 57 | + (ARGCOUNT - after) * sizeof(aentry_T)); |
| 58 | + arglist_locked = TRUE; |
| 59 | ++ curwin->w_locked = TRUE; |
| 60 | + for (i = 0; i < count; ++i) |
| 61 | + { |
| 62 | + int flags = BLN_LISTED | (will_edit ? BLN_CURBUF : 0); |
| 63 | +@@ -373,6 +378,7 @@ alist_add_list( |
| 64 | + ARGLIST[after + i].ae_fnum = buflist_add(files[i], flags); |
| 65 | + } |
| 66 | + arglist_locked = FALSE; |
| 67 | ++ curwin->w_locked = FALSE; |
| 68 | + ALIST(curwin)->al_ga.ga_len += count; |
| 69 | + if (old_argcount > 0 && curwin->w_arg_idx >= after) |
| 70 | + curwin->w_arg_idx += count; |
| 71 | +diff --git a/src/buffer.c b/src/buffer.c |
| 72 | +index 447ce76d49a32..34500e4abc282 100644 |
| 73 | +--- a/src/buffer.c |
| 74 | ++++ b/src/buffer.c |
| 75 | +@@ -1484,7 +1484,7 @@ do_buffer_ext( |
| 76 | + // (unless it's the only window). Repeat this so long as we end up in |
| 77 | + // a window with this buffer. |
| 78 | + while (buf == curbuf |
| 79 | +- && !(curwin->w_closing || curwin->w_buffer->b_locked > 0) |
| 80 | ++ && !(win_locked(curwin) || curwin->w_buffer->b_locked > 0) |
| 81 | + && (!ONE_WINDOW || first_tabpage->tp_next != NULL)) |
| 82 | + { |
| 83 | + if (win_close(curwin, FALSE) == FAIL) |
| 84 | +@@ -5470,7 +5470,7 @@ ex_buffer_all(exarg_T *eap) |
| 85 | + : wp->w_width != Columns) |
| 86 | + || (had_tab > 0 && wp != firstwin)) |
| 87 | + && !ONE_WINDOW |
| 88 | +- && !(wp->w_closing || wp->w_buffer->b_locked > 0) |
| 89 | ++ && !(win_locked(wp) || wp->w_buffer->b_locked > 0) |
| 90 | + && !win_unlisted(wp)) |
| 91 | + { |
| 92 | + if (win_close(wp, FALSE) == FAIL) |
| 93 | +diff --git a/src/ex_cmds.c b/src/ex_cmds.c |
| 94 | +index 05778c8fd8b9c..349269a2bb8b6 100644 |
| 95 | +--- a/src/ex_cmds.c |
| 96 | ++++ b/src/ex_cmds.c |
| 97 | +@@ -2840,7 +2840,7 @@ do_ecmd( |
| 98 | + |
| 99 | + // Set the w_closing flag to avoid that autocommands close the |
| 100 | + // window. And set b_locked for the same reason. |
| 101 | +- the_curwin->w_closing = TRUE; |
| 102 | ++ the_curwin->w_locked = TRUE; |
| 103 | + ++buf->b_locked; |
| 104 | + |
| 105 | + if (curbuf == old_curbuf.br_buf) |
| 106 | +@@ -2854,7 +2854,7 @@ do_ecmd( |
| 107 | + |
| 108 | + // Autocommands may have closed the window. |
| 109 | + if (win_valid(the_curwin)) |
| 110 | +- the_curwin->w_closing = FALSE; |
| 111 | ++ the_curwin->w_locked = FALSE; |
| 112 | + --buf->b_locked; |
| 113 | + |
| 114 | + #ifdef FEAT_EVAL |
| 115 | +diff --git a/src/proto/window.pro b/src/proto/window.pro |
| 116 | +index 26c7040b8a1b4..441070ebfcb8e 100644 |
| 117 | +--- a/src/proto/window.pro |
| 118 | ++++ b/src/proto/window.pro |
| 119 | +@@ -93,3 +93,4 @@ int get_win_number(win_T *wp, win_T *first_win); |
| 120 | + int get_tab_number(tabpage_T *tp); |
| 121 | + char *check_colorcolumn(win_T *wp); |
| 122 | ++int win_locked(win_T *wp); |
| 123 | + /* vim: set ft=c : */ |
| 124 | +diff --git a/src/structs.h b/src/structs.h |
| 125 | +index fe4704a367949..abda3a0c38b4e 100644 |
| 126 | +--- a/src/structs.h |
| 127 | ++++ b/src/structs.h |
| 128 | +@@ -3785,7 +3785,7 @@ struct window_S |
| 129 | + synblock_T *w_s; // for :ownsyntax |
| 130 | + #endif |
| 131 | + |
| 132 | +- int w_closing; // window is being closed, don't let |
| 133 | ++ int w_locked; // window is being closed, don't let |
| 134 | + // autocommands close it too. |
| 135 | + |
| 136 | + frame_T *w_frame; // frame containing this window |
| 137 | +diff --git a/src/terminal.c b/src/terminal.c |
| 138 | +index 1fc0ef96881f9..f80196096df49 100644 |
| 139 | +--- a/src/terminal.c |
| 140 | ++++ b/src/terminal.c |
| 141 | +@@ -3680,10 +3680,10 @@ term_after_channel_closed(term_T *term) |
| 142 | + if (is_aucmd_win(curwin)) |
| 143 | + do_set_w_closing = TRUE; |
| 144 | + if (do_set_w_closing) |
| 145 | +- curwin->w_closing = TRUE; |
| 146 | ++ curwin->w_locked = TRUE; |
| 147 | + do_bufdel(DOBUF_WIPE, (char_u *)"", 1, fnum, fnum, FALSE); |
| 148 | + if (do_set_w_closing) |
| 149 | +- curwin->w_closing = FALSE; |
| 150 | ++ curwin->w_locked = FALSE; |
| 151 | + aucmd_restbuf(&aco); |
| 152 | + } |
| 153 | + #ifdef FEAT_PROP_POPUP |
| 154 | +diff --git a/src/testdir/test_arglist.vim b/src/testdir/test_arglist.vim |
| 155 | +index edc8b77429e20..8d81a828b3e03 100644 |
| 156 | +--- a/src/testdir/test_arglist.vim |
| 157 | ++++ b/src/testdir/test_arglist.vim |
| 158 | +@@ -359,6 +359,7 @@ func Test_argv() |
| 159 | + call assert_equal('', argv(1, 100)) |
| 160 | + call assert_equal([], argv(-1, 100)) |
| 161 | + call assert_equal('', argv(10, -1)) |
| 162 | ++ %argdelete |
| 163 | + endfunc |
| 164 | + |
| 165 | + " Test for the :argedit command |
| 166 | +@@ -744,4 +745,26 @@ func Test_all_command() |
| 167 | + %bw! |
| 168 | + endfunc |
| 169 | + |
| 170 | ++" Test for deleting buffer when creating an arglist. This was accessing freed |
| 171 | ++" memory |
| 172 | ++func Test_crash_arglist_uaf() |
| 173 | ++ "%argdelete |
| 174 | ++ new one |
| 175 | ++ au BufAdd XUAFlocal :bw |
| 176 | ++ "call assert_fails(':arglocal XUAFlocal', 'E163:') |
| 177 | ++ arglocal XUAFlocal |
| 178 | ++ au! BufAdd |
| 179 | ++ bw! XUAFlocal |
| 180 | ++ |
| 181 | ++ au BufAdd XUAFlocal2 :bw |
| 182 | ++ new two |
| 183 | ++ new three |
| 184 | ++ arglocal |
| 185 | ++ argadd XUAFlocal2 Xfoobar |
| 186 | ++ bw! XUAFlocal2 |
| 187 | ++ bw! two |
| 188 | ++ |
| 189 | ++ au! BufAdd |
| 190 | ++endfunc |
| 191 | ++ |
| 192 | + " vim: shiftwidth=2 sts=2 expandtab |
| 193 | +diff --git a/src/window.c b/src/window.c |
| 194 | +index 43a15e0561f2c..b2c90c7d64114 100644 |
| 195 | +--- a/src/window.c |
| 196 | ++++ b/src/window.c |
| 197 | +@@ -2511,7 +2511,7 @@ close_windows( |
| 198 | + for (wp = firstwin; wp != NULL && !ONE_WINDOW; ) |
| 199 | + { |
| 200 | + if (wp->w_buffer == buf && (!keep_curwin || wp != curwin) |
| 201 | +- && !(wp->w_closing || wp->w_buffer->b_locked > 0)) |
| 202 | ++ && !(win_locked(wp) || wp->w_buffer->b_locked > 0)) |
| 203 | + { |
| 204 | + if (win_close(wp, FALSE) == FAIL) |
| 205 | + // If closing the window fails give up, to avoid looping |
| 206 | +@@ -2532,7 +2532,7 @@ close_windows( |
| 207 | + if (tp != curtab) |
| 208 | + FOR_ALL_WINDOWS_IN_TAB(tp, wp) |
| 209 | + if (wp->w_buffer == buf |
| 210 | +- && !(wp->w_closing || wp->w_buffer->b_locked > 0)) |
| 211 | ++ && !(win_locked(wp) || wp->w_buffer->b_locked > 0)) |
| 212 | + { |
| 213 | + win_close_othertab(wp, FALSE, tp); |
| 214 | + |
| 215 | +@@ -2654,10 +2654,10 @@ win_close_buffer(win_T *win, int action, int abort_if_last) |
| 216 | + bufref_T bufref; |
| 217 | + |
| 218 | + set_bufref(&bufref, curbuf); |
| 219 | +- win->w_closing = TRUE; |
| 220 | ++ win->w_locked = TRUE; |
| 221 | + close_buffer(win, win->w_buffer, action, abort_if_last, TRUE); |
| 222 | + if (win_valid_any_tab(win)) |
| 223 | +- win->w_closing = FALSE; |
| 224 | ++ win->w_locked = FALSE; |
| 225 | + // Make sure curbuf is valid. It can become invalid if 'bufhidden' is |
| 226 | + // "wipe". |
| 227 | + if (!bufref_valid(&bufref)) |
| 228 | +@@ -2705,7 +2705,7 @@ win_close(win_T *win, int free_buf) |
| 229 | + if (window_layout_locked(CMD_close)) |
| 230 | + return FAIL; |
| 231 | + |
| 232 | +- if (win->w_closing || (win->w_buffer != NULL |
| 233 | ++ if (win_locked(win) || (win->w_buffer != NULL |
| 234 | + && win->w_buffer->b_locked > 0)) |
| 235 | + return FAIL; // window is already being closed |
| 236 | + if (win_unlisted(win)) |
| 237 | +@@ -2754,19 +2754,19 @@ win_close(win_T *win, int free_buf) |
| 238 | + other_buffer = TRUE; |
| 239 | + if (!win_valid(win)) |
| 240 | + return FAIL; |
| 241 | +- win->w_closing = TRUE; |
| 242 | ++ win->w_locked = TRUE; |
| 243 | + apply_autocmds(EVENT_BUFLEAVE, NULL, NULL, FALSE, curbuf); |
| 244 | + if (!win_valid(win)) |
| 245 | + return FAIL; |
| 246 | +- win->w_closing = FALSE; |
| 247 | ++ win->w_locked = FALSE; |
| 248 | + if (last_window()) |
| 249 | + return FAIL; |
| 250 | + } |
| 251 | +- win->w_closing = TRUE; |
| 252 | ++ win->w_locked = TRUE; |
| 253 | + apply_autocmds(EVENT_WINLEAVE, NULL, NULL, FALSE, curbuf); |
| 254 | + if (!win_valid(win)) |
| 255 | + return FAIL; |
| 256 | +- win->w_closing = FALSE; |
| 257 | ++ win->w_locked = FALSE; |
| 258 | + if (last_window()) |
| 259 | + return FAIL; |
| 260 | + #ifdef FEAT_EVAL |
| 261 | +@@ -3346,7 +3346,7 @@ win_close_othertab(win_T *win, int free_buf, tabpage_T *tp) |
| 262 | + |
| 263 | + // Get here with win->w_buffer == NULL when win_close() detects the tab |
| 264 | + // page changed. |
| 265 | +- if (win->w_closing || (win->w_buffer != NULL |
| 266 | ++ if (win_locked(win) || (win->w_buffer != NULL |
| 267 | + && win->w_buffer->b_locked > 0)) |
| 268 | + return; // window is already being closed |
| 269 | + |
| 270 | +@@ -7808,3 +7808,12 @@ skip: |
| 271 | + return NULL; // no error |
| 272 | + } |
| 273 | + #endif |
| 274 | ++ |
| 275 | ++/* |
| 276 | ++ * Don't let autocommands close the given window |
| 277 | ++ */ |
| 278 | ++ int |
| 279 | ++win_locked(win_T *wp) |
| 280 | ++{ |
| 281 | ++ return wp->w_locked; |
| 282 | ++} |
0 commit comments