patch 9.0.2149: [security]: use-after-free in exec_instructions()

Problem:  [security]: use-after-free in exec_instructions()
Solution: get tv pointer again

[security]: use-after-free in exec_instructions()

exec_instructions may access freed memory, if the GA_GROWS_FAILS()
re-allocates memory. When this happens, the typval tv may still point to
now already freed memory. So let's get that pointer again and compare it
with tv. If those two pointers differ, tv is now invalid and we have to
refresh the tv pointer.

closes: #13621

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

diff --git a/src/testdir/crash/poc_uaf_exec_instructions b/src/testdir/crash/poc_uaf_exec_instructions
new file mode 100644 (file)
index 0000000..49ae857
Binary files /dev/null and b/src/testdir/crash/poc_uaf_exec_instructions differ

diff --git a/src/testdir/test_crash.vim b/src/testdir/test_crash.vim
index 49e712a901d8b5cabd964e1c0a52c4d7e70f48bd..242da8e5db457be333f7a17cb708689a34bb5711 100644 (file)
--- a/src/testdir/test_crash.vim
+++ b/src/testdir/test_crash.vim
@@ -113,6 +113,7 @@ endfunc
 func Test_crash1_2()
   CheckNotBSD
   CheckExecutable dash
+  let g:test_is_flaky = 1
 
   " The following used to crash Vim
   let opts = #{cmd: 'sh'}
@@ -149,22 +150,9 @@ func Test_crash1_2()
     \ ' ; echo "crash 4: [OK]" >> '.. result .. "\<cr>")
   call TermWait(buf, 150)
 
-  let file = 'crash/poc_ex_substitute'
-  let cmn_args = "%s -u NONE -i NONE -n -e -s -S %s -c ':qa!'"
-  let args = printf(cmn_args, vim, file)
-  " just make sure it runs, we don't care about the resulting echo
-  call term_sendkeys(buf, args .. "\<cr>")
-  " There is no output generated in Github CI for the asan clang build.
-  " so just skip generating the ouput.
-  " call term_sendkeys(buf, args ..
-  "   \ ' &&  echo "crash 5: [OK]" >> '.. result .. "\<cr>")
-  call TermWait(buf, 150)
-
   " clean up
   exe buf .. "bw!"
-
   exe "sp " .. result
-
   let expected = [
       \ 'crash 1: [OK]',
       \ 'crash 2: [OK]',
@@ -174,10 +162,33 @@ func Test_crash1_2()
 
   call assert_equal(expected, getline(1, '$'))
   bw!
-
   call delete(result)
 endfunc
 
+" This test just runs various scripts, that caused issues before.
+" We are not really asserting anything here, it's just important
+" that ASAN does not detect any issues.
+func Test_crash1_3()
+  let vim  = GetVimProg()
+  let buf = RunVimInTerminal('sh', #{cmd: 'sh'})
+
+  let file = 'crash/poc_ex_substitute'
+  let cmn_args = "%s -u NONE -i NONE -n -e -s -S %s -c ':qa!'\<cr>"
+  let args = printf(cmn_args, vim, file)
+  call term_sendkeys(buf, args)
+  call TermWait(buf, 150)
+
+  let file = 'crash/poc_uaf_exec_instructions'
+  let cmn_args = "%s -u NONE -i NONE -n -e -s -S %s -c ':qa!'\<cr>"
+  let args = printf(cmn_args, vim, file)
+  call term_sendkeys(buf, args)
+  call TermWait(buf, 150)
+
+  " clean up
+  exe buf .. "bw!"
+  bw!
+endfunc
+
 func Test_crash2()
   " The following used to crash Vim
   let opts = #{wait_for_ruler: 0, rows: 20}

diff --git a/src/version.c b/src/version.c
index 2c092279011877d7cbbcdd300cb73e2e757e1e0b..064b8137f5c5dd213b12360147faecf674db8fae 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 */
+/**/
+    2149,
 /**/
     2148,
 /**/

diff --git a/src/vim9execute.c b/src/vim9execute.c
index e329559eabe1fdcb299cb9a092a1d01671168d16..44cdb09e30f55a732aa5ae8d189984d5cf062f97 100644 (file)
--- a/src/vim9execute.c
+++ b/src/vim9execute.c
@@ -4123,8 +4123,22 @@ exec_instructions(ectx_T *ectx)
                                      + iptr->isn_arg.outer.outer_idx;
                    if (iptr->isn_type == ISN_LOADOUTER)
                    {
+                       typval_T *copy;
                        if (GA_GROW_FAILS(&ectx->ec_stack, 1))
                            goto theend;
+                       // careful: ga_grow_inner may re-alloc the stack
+                       if (depth < 0)
+                           copy = ((typval_T *)outer->out_loop[-depth - 1]
+                                                                  .stack->ga_data)
+                                             + outer->out_loop[-depth - 1].var_idx
+                                             + iptr->isn_arg.outer.outer_idx;
+                       else
+                           copy = ((typval_T *)outer->out_stack->ga_data)
+                                         + outer->out_frame_idx + STACK_FRAME_SIZE
+                                         + iptr->isn_arg.outer.outer_idx;
+                       // memory was freed, get tv again
+                       if (copy != tv)
+                           tv = copy;
                        copy_tv(tv, STACK_TV_BOT(0));
                        ++ectx->ec_stack.ga_len;
                    }