CVE-2023-4733: Use After Free (CWE-416)
value: Use After Free in GitHub repository vim/vim prior to 9.0.1840.
CWE description: The product reuses or references memory after it has been freed. At some point afterward, the memory may be allocated again and saved in another pointer, while the original pointer references a location somewhere within the new allocation. Any operations using the original pointer are no longer valid because the memory "belongs" to the code that operates on the new pointer.
CVSS v3 base score: 7.8
Published: 2023-09-04T14:15Z
Repository: https://github.com/vim/vim
Language: C
Files changed: src/ex_cmds.c, src/testdir/Make_all.mak, src/testdir/dumps/Test_crash_01.dump, src/testdir/test_crash.vim, src/version.c
Commit date: 2023-09-02 14:44:47 +0200
Commit message: patch 9.0.1840: [security] use-after-free in do_ecmd

Problem:  use-after-free in do_ecmd
Solution: Verify oldwin pointer after reset_VIsual()

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

Diff: diff --git a/src/ex_cmds.c b/src/ex_cmds.c
index 20d4d9a2eaad..9348b4edda15 100644
--- a/src/ex_cmds.c
+++ b/src/ex_cmds.c
@@ -2644,16 +2644,22 @@ do_ecmd(
 	if (fnum == 0 && other_file && ffname != NULL)
 	    (void)setaltfname(ffname, sfname, newlnum < 0 ? 0 : newlnum);
 	goto theend;
     }
 
-    /*
-     * End Visual mode before switching to another buffer, so the text can be
-     * copied into the GUI selection buffer.
-     */
+    
+     // End Visual mode before switching to another buffer, so the text can be
+     // copied into the GUI selection buffer.
+     // Careful: may trigger ModeChanged() autocommand
+     
+    // Should we block autocommands here?
     reset_VIsual();
 
+    // autocommands freed window :(
+    if (oldwin != NULL && !win_valid(oldwin))
+	oldwin = NULL;
+
 #if defined(FEAT_EVAL)
     if ((command != NULL || newlnum > (linenr_T)0)
 	    && *get_vim_var_str(VV_SWAPCOMMAND) == NUL)
     {
 	int	len;
diff --git a/src/testdir/Make_all.mak b/src/testdir/Make_all.mak
index e2e29f12d2b3..961718fd699c 100644
--- a/src/testdir/Make_all.mak
+++ b/src/testdir/Make_all.mak
@@ -103,10 +103,11 @@ NEW_TESTS = \
 	test_comparators \
 	test_compiler \
 	test_conceal \
 	test_const \
 	test_cpoptions \
+	test_crash \
 	test_crypt \
 	test_cscope \
 	test_cursor_func \
 	test_cursorline \
 	test_curswant \
@@ -367,10 +368,11 @@ NEW_TESTS_RES = \
 	test_comments.res \
 	test_comparators.res \
 	test_conceal.res \
 	test_const.res \
 	test_cpoptions.res \
+	test_crash.res \
 	test_crypt.res \
 	test_cscope.res \
 	test_cursor_func.res \
 	test_cursorline.res \
 	test_curswant.res \
diff --git a/src/testdir/crash/poc_huaf1 b/src/testdir/crash/poc_huaf1
new file mode 100644
index 000000000000..0d0ea475c106
Binary files /dev/null and b/src/testdir/crash/poc_huaf1 differ
diff --git a/src/testdir/crash/poc_huaf2 b/src/testdir/crash/poc_huaf2
new file mode 100644
index 000000000000..4867e0f956bb
Binary files /dev/null and b/src/testdir/crash/poc_huaf2 differ
diff --git a/src/testdir/crash/poc_huaf3 b/src/testdir/crash/poc_huaf3
new file mode 100644
index 000000000000..7e38a9a17c26
Binary files /dev/null and b/src/testdir/crash/poc_huaf3 differ
diff --git a/src/testdir/dumps/Test_crash_01.dump b/src/testdir/dumps/Test_crash_01.dump
new file mode 100644
index 000000000000..1f0fd44c058b
--- /dev/null
+++ b/src/testdir/dumps/Test_crash_01.dump
@@ -0,0 +1,20 @@
+> +0&#ffffff0@74
+@75
+@75
+@75
+@75
+@75
+@75
+@75
+@75
+@75
+@75
+@75
+@75
+@75
+@75
+@75
+@75
+@75
+@75
+@75
diff --git a/src/testdir/test_crash.vim b/src/testdir/test_crash.vim
new file mode 100644
index 000000000000..0dea3c2cb1cb
--- /dev/null
+++ b/src/testdir/test_crash.vim
@@ -0,0 +1,25 @@
+" Some tests, that used to crash Vim
+source check.vim
+source screendump.vim
+
+CheckScreendump
+
+func Test_crash1()
+  " The following used to crash Vim
+  let opts = #{wait_for_ruler: 0}
+  let args = ' -u NONE -i NONE -n -e -s -S '
+  let buf = RunVimInTerminal(args .. ' crash/poc_huaf1', opts)
+  call VerifyScreenDump(buf, 'Test_crash_01', {})
+  exe buf .. "bw!"
+
+  let buf = RunVimInTerminal(args .. ' crash/poc_huaf2', opts)
+  call VerifyScreenDump(buf, 'Test_crash_01', {})
+  exe buf .. "bw!"
+
+  let buf = RunVimInTerminal(args .. ' crash/poc_huaf3', opts)
+  call VerifyScreenDump(buf, 'Test_crash_01', {})
+  exe buf .. "bw!"
+
+endfunc
+
+" vim: shiftwidth=2 sts=2 expandtab
diff --git a/src/version.c b/src/version.c
index 5f6995cc61e0..34878d74929c 100644
--- a/src/version.c
+++ b/src/version.c
@@ -697,10 +697,12 @@ static char *(features[]) =
 	NULL
 };
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1840,
 /**/
     1839,
 /**/
     1838,
 /**/

Vulnerable code:     /*
     * End Visual mode before switching to another buffer, so the text can be
     * copied into the GUI selection buffer.
     */
Fixed code:     
     // End Visual mode before switching to another buffer, so the text can be
     // copied into the GUI selection buffer.
     // Careful: may trigger ModeChanged() autocommand
     
    // Should we block autocommands here?
    // autocommands freed window :(
    if (oldwin != NULL && !win_valid(oldwin))
	oldwin = NULL;

	test_crash \
	test_crash.res \
> +0&#ffffff0@74
@75
@75
@75
@75
@75
@75
@75
@75
@75
@75
@75
@75
@75
@75
@75
@75
@75
@75
@75
" Some tests, that used to crash Vim
source check.vim
source screendump.vim

CheckScreendump

func Test_crash1()
  " The following used to crash Vim
  let opts = #{wait_for_ruler: 0}
  let args = ' -u NONE -i NONE -n -e -s -S '
  let buf = RunVimInTerminal(args .. ' crash/poc_huaf1', opts)
  call VerifyScreenDump(buf, 'Test_crash_01', {})
  exe buf .. "bw!"

  let buf = RunVimInTerminal(args .. ' crash/poc_huaf2', opts)
  call VerifyScreenDump(buf, 'Test_crash_01', {})
  exe buf .. "bw!"

  let buf = RunVimInTerminal(args .. ' crash/poc_huaf3', opts)
  call VerifyScreenDump(buf, 'Test_crash_01', {})
  exe buf .. "bw!"

endfunc

" vim: shiftwidth=2 sts=2 expandtab
/**/
    1840,