From a589156b4d3ea2dc72908b8773c42ad012929c64 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Fri, 17 Nov 2023 08:56:41 +0800 Subject: vim-patch:9.0.1857: [security] heap-use-after-free in is_qf_win() Problem: heap-use-after-free in is_qf_win() Solution: Check buffer is valid before accessing it https://github.com/vim/vim/commit/fc68299d436cf87453e432daa77b6d545df4d7ed Co-authored-by: Christian Brabandt --- src/nvim/main.c | 2 +- src/nvim/quickfix.c | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) (limited to 'src/nvim') diff --git a/src/nvim/main.c b/src/nvim/main.c index 818a1313d7..5c0fc758b9 100644 --- a/src/nvim/main.c +++ b/src/nvim/main.c @@ -698,7 +698,7 @@ void getout(int exitval) for (const tabpage_T *tp = first_tabpage; tp != NULL; tp = next_tp) { next_tp = tp->tp_next; FOR_ALL_WINDOWS_IN_TAB(wp, tp) { - if (wp->w_buffer == NULL) { + if (wp->w_buffer == NULL || !buf_valid(wp->w_buffer)) { // Autocmd must have close the buffer already, skip. continue; } diff --git a/src/nvim/quickfix.c b/src/nvim/quickfix.c index 19b34b52b4..2ddee313a3 100644 --- a/src/nvim/quickfix.c +++ b/src/nvim/quickfix.c @@ -262,10 +262,8 @@ static const char *e_current_location_list_was_changed = #define IS_QF_LIST(qfl) ((qfl)->qfl_type == QFLT_QUICKFIX) #define IS_LL_LIST(qfl) ((qfl)->qfl_type == QFLT_LOCATION) -// // Return location list for window 'wp' // For location list window, return the referenced location list -// #define GET_LOC_LIST(wp) (IS_LL_WINDOW(wp) ? (wp)->w_llist_ref : (wp)->w_llist) // Macro to loop through all the items in a quickfix list @@ -3863,13 +3861,11 @@ static bool qf_win_pos_update(qf_info_T *qi, int old_qf_index) static int is_qf_win(const win_T *win, const qf_info_T *qi) FUNC_ATTR_NONNULL_ARG(2) FUNC_ATTR_PURE FUNC_ATTR_WARN_UNUSED_RESULT { - // // A window displaying the quickfix buffer will have the w_llist_ref field // set to NULL. // A window displaying a location list buffer will have the w_llist_ref // pointing to the location list. - // - if (bt_quickfix(win->w_buffer)) { + if (buf_valid(win->w_buffer) && bt_quickfix(win->w_buffer)) { if ((IS_QF_STACK(qi) && win->w_llist_ref == NULL) || (IS_LL_STACK(qi) && win->w_llist_ref == qi)) { return true; -- cgit From 8dc72789cfad630c2f2da572916490a32d5155e6 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Fri, 17 Nov 2023 09:12:01 +0800 Subject: vim-patch:9.0.1858: [security] heap use after free in ins_compl_get_exp() Problem: heap use after free in ins_compl_get_exp() Solution: validate buffer before accessing it https://github.com/vim/vim/commit/ee9166eb3b41846661a39b662dc7ebe8b5e15139 Co-authored-by: Christian Brabandt --- src/nvim/insexpand.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/nvim') diff --git a/src/nvim/insexpand.c b/src/nvim/insexpand.c index 28d1c8216e..f565d5b9e8 100644 --- a/src/nvim/insexpand.c +++ b/src/nvim/insexpand.c @@ -3435,7 +3435,7 @@ static int ins_compl_get_exp(pos_T *ini) compl_started = true; } else { // Mark a buffer scanned when it has been scanned completely - if (type == 0 || type == CTRL_X_PATH_PATTERNS) { + if (buf_valid(st.ins_buf) && (type == 0 || type == CTRL_X_PATH_PATTERNS)) { assert(st.ins_buf); st.ins_buf->b_scanned = true; } -- cgit From 1274380029313d820c9a0c28d10d606ddb27aacd Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Fri, 17 Nov 2023 09:19:01 +0800 Subject: vim-patch:9.0.1864: still crash with bt_quickfix1_poc Problem: crash with bt_quickfix1_poc when cleaning up and EXITFREE is defined Solution: Test if buffer is valid in a window, else close window directly, don't try to access buffer properties While at it, increase the crash timeout slightly, so that CI has a chance to finish processing the test_crash() test. https://github.com/vim/vim/commit/623ba31821a41acee7e948794e84867680b97885 Co-authored-by: Christian Brabandt --- src/nvim/window.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'src/nvim') diff --git a/src/nvim/window.c b/src/nvim/window.c index 8ff8053118..89bdd7f5e0 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -3776,6 +3776,12 @@ void close_others(int message, int forceit) continue; } + // autoccommands messed this one up + if (!buf_valid(wp->w_buffer) && win_valid(wp)) { + wp->w_buffer = NULL; + win_close(wp, false, false); + continue; + } // Check if it's allowed to abandon this window int r = can_abandon(wp->w_buffer, forceit); if (!win_valid(wp)) { // autocommands messed wp up -- cgit From 3ab0e296c674a6846512df24a0a70199bba8c59a Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Fri, 17 Nov 2023 09:33:54 +0800 Subject: vim-patch:9.0.1969: [security] buffer-overflow in trunc_string() Problem: buffer-overflow in trunc_string() Solution: Add NULL at end of buffer Currently trunc_string() assumes that when the string is too long, buf[e-1] will always be writeable. But that assumption may not always be true. The condition currently looks like this else if (e + 3 < buflen) [...] else { // can't fit in the "...", just truncate it buf[e - 1] = NUL; } but this means, we may run into the last else clause with e still being larger than buflen. So a buffer overflow occurs. So instead of using `buf[e - 1]`, let's just always truncate at `buf[buflen - 1]` which should always be writable. https://github.com/vim/vim/commit/3bd7fa12e146c6051490d048a4acbfba974eeb04 vim-patch:9.0.2004: Missing test file Problem: Missing test file Solution: git-add the file to the repo closes: vim/vim#13305 https://github.com/vim/vim/commit/d4afbdd0715c722cfc73d3a8ab9e578667615faa Co-authored-by: Christian Brabandt --- src/nvim/message.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/nvim') diff --git a/src/nvim/message.c b/src/nvim/message.c index 8be8581537..ee1a9e60b0 100644 --- a/src/nvim/message.c +++ b/src/nvim/message.c @@ -468,7 +468,7 @@ void trunc_string(const char *s, char *buf, int room_in, int buflen) buf[e + 3 + len - 1] = NUL; } else { // can't fit in the "...", just truncate it - buf[e - 1] = NUL; + buf[buflen - 1] = NUL; } } -- cgit From d49be1cd2893ad583361ac058279a471ad7877e5 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Fri, 17 Nov 2023 09:42:59 +0800 Subject: vim-patch:9.0.2010: [security] use-after-free from buf_contents_changed() Problem: [security] use-after-free from buf_contents_changed() Solution: block autocommands https://github.com/vim/vim/commit/41e6f7d6ba67b61d911f9b1d76325cd79224753d Co-authored-by: Christian Brabandt --- src/nvim/buffer.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'src/nvim') diff --git a/src/nvim/buffer.c b/src/nvim/buffer.c index 6617907f8f..6d5c7a1766 100644 --- a/src/nvim/buffer.c +++ b/src/nvim/buffer.c @@ -4223,6 +4223,10 @@ bool buf_contents_changed(buf_T *buf) aco_save_T aco; aucmd_prepbuf(&aco, newbuf); + // We don't want to trigger autocommands now, they may have nasty + // side-effects like wiping buffers + block_autocmds(); + if (ml_open(curbuf) == OK && readfile(buf->b_ffname, buf->b_fname, 0, 0, (linenr_T)MAXLNUM, @@ -4247,6 +4251,8 @@ bool buf_contents_changed(buf_T *buf) wipe_buffer(newbuf, false); } + unblock_autocmds(); + return differ; } -- cgit From 790bd4d5858713e8503825892c7d08340d189370 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Fri, 17 Nov 2023 09:47:04 +0800 Subject: vim-patch:9.0.2106: [security]: Use-after-free in win_close() Problem: [security]: Use-after-free in win_close() Solution: Check window is valid, before accessing it If the current window structure is no longer valid (because a previous autocommand has already freed this window), fail and return before attempting to set win->w_closing variable. Add a test to trigger ASAN in CI https://github.com/vim/vim/commit/25aabc2b8ee1e19ced6f4da9d866cf9378fc4c5a Co-authored-by: Christian Brabandt --- src/nvim/window.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src/nvim') diff --git a/src/nvim/window.c b/src/nvim/window.c index 89bdd7f5e0..00524b2f56 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -2661,6 +2661,9 @@ int win_close(win_T *win, bool free_buf, bool force) reset_VIsual_and_resel(); // stop Visual mode other_buffer = true; + if (!win_valid(win)) { + return FAIL; + } win->w_closing = true; apply_autocmds(EVENT_BUFLEAVE, NULL, NULL, false, curbuf); if (!win_valid(win)) { -- cgit