Commit: patch 9.0.1857: [security] heap-use-after-free in is_qf_win()

[Christian Brabandt]

patch 9.0.1857: [security] heap-use-after-free in is_qf_win()

Commit: https://github.com/vim/vim/commit/fc68299d436cf87453e432daa77b6d545df4d7ed
Author: Christian Brabandt <c...@256bit.org>
Date: Sun Sep 3 20:20:52 2023 +0200

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

Signed-off-by: Christian Brabandt <c...@256bit.org>

diff --git a/src/main.c b/src/main.c
index cca53fe25..a40c3a666 100644
--- a/src/main.c
+++ b/src/main.c
@@ -1646,7 +1646,7 @@ getout(int exitval)
next_tp = tp->tp_next;
FOR_ALL_WINDOWS_IN_TAB(tp, wp)
{
- if (wp->w_buffer == NULL)
+ if (wp->w_buffer == NULL || !buf_valid(wp->w_buffer))
// Autocmd must have close the buffer already, skip.
continue;
buf = wp->w_buffer;
diff --git a/src/quickfix.c b/src/quickfix.c
index aa431ea10..a3d3e8fb7 100644
--- a/src/quickfix.c
+++ b/src/quickfix.c
@@ -4492,7 +4492,7 @@ is_qf_win(win_T *win, qf_info_T *qi)
// 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;
diff --git a/src/testdir/crash/bt_quickfix_poc b/src/testdir/crash/bt_quickfix_poc
new file mode 100644
index 000000000..bf02b4dcb
--- /dev/null
+++ b/src/testdir/crash/bt_quickfix_poc
@@ -0,0 +1,9 @@
+comman!-narg=* Xexpr <mods>lex<args>
+auto BufReadPre * exe"sn" ..expand("<abuf>")
+fu Xautocmd_changelist()
+cal writefile(['Xtestfile2:4:4'],'Xerr')
+ sil! edi Xerr
+Xexpr 'Xtestfile:4:4'
+endf
+call Xautocmd_changelist()
+call Xautocmd_changelist()
\ No newline at end of file
diff --git a/src/testdir/test_crash.vim b/src/testdir/test_crash.vim
index eb3c0a37f..516d99193 100644
--- a/src/testdir/test_crash.vim
+++ b/src/testdir/test_crash.vim
@@ -5,38 +5,58 @@ source screendump.vim
CheckScreendump

func Test_crash1()
+ if !executable('sh')
+ throw 'Skipped: sh not executable!'
+ endif
" The following used to crash Vim
- " let opts = #{wait_for_ruler: 0, rows: 20, cmd: 'sh'}
let opts = #{cmd: 'sh'}
- let args = 'bash'
let vim = GetVimProg()

- let buf = RunVimInTerminal(args, opts)
+ let buf = RunVimInTerminal('sh', opts)

let file = 'crash/poc_huaf1'
let cmn_args = "%s -u NONE -i NONE -n -e -s -S %s -c ':qa!'"
let args = printf(cmn_args, vim, file)
call term_sendkeys(buf, args ..
- \ ' && echo "crash 1: [OK]" >> X_crash1_result.txt' .. "\<cr>")
+ \ ' && echo "crash 1: [OK]" > X_crash1_result.txt' .. "\<cr>")
+ call TermWait(buf, 50)

let file = 'crash/poc_huaf2'
let args = printf(cmn_args, vim, file)
call term_sendkeys(buf, args ..
\ ' && echo "crash 2: [OK]" >> X_crash1_result.txt' .. "\<cr>")
+ call TermWait(buf, 50)

let file = 'crash/poc_huaf3'
let args = printf(cmn_args, vim, file)
call term_sendkeys(buf, args ..
\ ' && echo "crash 3: [OK]" >> X_crash1_result.txt' .. "\<cr>")
+ call TermWait(buf, 100)

- call TermWait(buf, 50)
+ let file = 'crash/bt_quickfix_poc'
+ let args = printf(cmn_args, vim, file)
+ call term_sendkeys(buf, args ..
+ \ ' && echo "crash 4: [OK]" >> X_crash1_result.txt' .. "\<cr>")
+ " clean up
+ call delete('Xerr')
+
+ " This test takes a bit longer
+ call TermWait(buf, 200)

" clean up
+ call delete('Xerr')
exe buf .. "bw!"

sp X_crash1_result.txt
- call assert_equal(['crash 1: [OK]', 'crash 2: [OK]', 'crash 3: [OK]'],
- \ getline(1, '$'))
+
+ let expected = [
+ \ 'crash 1: [OK]',
+ \ 'crash 2: [OK]',
+ \ 'crash 3: [OK]',
+ \ 'crash 4: [OK]',
+ \ ]
+
+ call assert_equal(expected, getline(1, '$'))
bw!

call delete('X_crash1_result.txt')
diff --git a/src/version.c b/src/version.c
index 98964fbbd..13d5d695a 100644
--- a/src/version.c
+++ b/src/version.c
@@ -699,6 +699,8 @@ static char *(features[]) =

static int included_patches[] =
{ /* Add new patch number below this line */
+/**/
+ 1857,
/**/
1856,
/**/

[Yegappan Lakshmanan]

Hi Christian,

On Sun, Sep 3, 2023 at 11:30 AM Christian Brabandt <cbl...@256bit.org> wrote:
>
> patch 9.0.1857: [security] heap-use-after-free in is_qf_win()
>
> Commit: https://github.com/vim/vim/commit/fc68299d436cf87453e432daa77b6d545df4d7ed
> Author: Christian Brabandt <c...@256bit.org>
> Date: Sun Sep 3 20:20:52 2023 +0200
>
> 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
>
> Signed-off-by: Christian Brabandt <c...@256bit.org>
>

> diff --git a/src/testdir/crash/bt_quickfix_poc b/src/testdir/crash/bt_quickfix_poc
> new file mode 100644
> index 000000000..bf02b4dcb
> --- /dev/null
> +++ b/src/testdir/crash/bt_quickfix_poc
> @@ -0,0 +1,9 @@
> +comman!-narg=* Xexpr <mods>lex<args>
> +auto BufReadPre * exe"sn" ..expand("<abuf>")
> +fu Xautocmd_changelist()
> +cal writefile(['Xtestfile2:4:4'],'Xerr')
> + sil! edi Xerr
> +Xexpr 'Xtestfile:4:4'
> +endf
> +call Xautocmd_changelist()
> +call Xautocmd_changelist()
> \ No newline at end of file
>

Is there supposed to be a newline at the end of this file?

- Yegappan

[Christian Brabandt]

I guess not...


Thanks,
Christian
--
I pledge allegiance to the flag
of the United States of America
and to the republic for which it stands,
one nation,
indivisible,
with liberty
and justice for all.
-- Francis Bellamy, 1892