CVE-2022-3491: Heap-based Buffer Overflow (CWE-122)
value: Heap-based Buffer Overflow in GitHub repository vim/vim prior to 9.0.0742.
CWE description: A heap overflow condition is a buffer overflow, where the buffer that can be overwritten is allocated in the heap portion of memory, generally meaning that the buffer was allocated using a routine such as malloc().
CVSS v3 base score: 7.8
Published: 2022-12-03T13:15Z
Repository: https://github.com/vim/vim
Language: C
Files changed: src/testdir/test_vim9_func.vim, src/testdir/test_vim9_script.vim, src/version.c, src/vim9cmds.c, src/vim9compile.c
Commit date: 2022-10-13 16:12:57 +0100
Commit message: patch 9.0.0742: reading past end of the line when compiling a function

Problem:    Reading past end of the line when compiling a function with
            errors.
Solution:   Do not return an invalid pointer.  Fix skipping redirection.

Diff: diff --git a/src/testdir/test_vim9_func.vim b/src/testdir/test_vim9_func.vim
index 38766e33015..bb563562648 100644
--- a/src/testdir/test_vim9_func.vim
+++ b/src/testdir/test_vim9_func.vim
@@ -4337,10 +4337,37 @@ def Test_defer()
   assert_equal(['in One', 'in Two', 'end Two', 'two2', 'two1', 'two0', 'end One', 'one'], g:deferred)
   unlet g:deferred
   assert_equal('', glob('XdeferFile'))
 enddef
 
+def Test_invalid_redir()
+  var lines =<< trim END
+      def Tone()
+        if 1
+          redi =>@0
+          redi END
+        endif
+      enddef
+      defcompile
+  END
+  v9.CheckScriptFailure(lines, 'E354:')
+  delfunc g:Tone
+
+  # this was reading past the end of the line
+  lines =<< trim END
+      def Ttwo()
+        if 0
+          redi =>@0
+          redi END
+        endif
+      enddef
+      defcompile
+  END
+  v9.CheckScriptFailure(lines, 'E354:')
+  delfunc g:Ttwo
+enddef
+
 " The following messes up syntax highlight, keep near the end.
 if has('python3')
   def Test_python3_command()
     py3 import vim
     py3 vim.command("g:done = 'yes'")
diff --git a/src/testdir/test_vim9_script.vim b/src/testdir/test_vim9_script.vim
index a0b8352f3f7..6c8f1f0ce0b 100644
--- a/src/testdir/test_vim9_script.vim
+++ b/src/testdir/test_vim9_script.vim
@@ -2134,19 +2134,70 @@ def Test_for_skipped_block()
   v9.CheckDefAndScriptSuccess(lines)
 enddef
 
 def Test_skipped_redir()
   var lines =<< trim END
-      def T()
+      def Tredir()
         if 0
-          redir =>l[0]
+          redir => l[0]
           redir END
         endif
       enddef
       defcompile
   END
   v9.CheckScriptSuccess(lines)
+  delfunc g:Tredir
+
+  lines =<< trim END
+      def Tredir()
+        if 0
+          redir => l[0]
+        endif
+        echo 'executed'
+        if 0
+          redir END
+        endif
+      enddef
+      defcompile
+  END
+  v9.CheckScriptSuccess(lines)
+  delfunc g:Tredir
+
+  lines =<< trim END
+      def Tredir()
+        var l = ['']
+        if 1
+          redir => l[0]
+        endif
+        echo 'executed'
+        if 0
+          redir END
+        else
+          redir END
+        endif
+      enddef
+      defcompile
+  END
+  v9.CheckScriptSuccess(lines)
+  delfunc g:Tredir
+
+  lines =<< trim END
+      let doit = 1
+      def Tredir()
+        var l = ['']
+        if g:doit
+          redir => l[0]
+        endif
+        echo 'executed'
+        if g:doit
+          redir END
+        endif
+      enddef
+      defcompile
+  END
+  v9.CheckScriptSuccess(lines)
+  delfunc g:Tredir
 enddef
 
 def Test_for_loop()
   var lines =<< trim END
       var result = ''
diff --git a/src/version.c b/src/version.c
index 6570ac52a8a..467f8e08369 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 */
+/**/
+    742,
 /**/
     741,
 /**/
     740,
 /**/
diff --git a/src/vim9cmds.c b/src/vim9cmds.c
index 73f95b26dfe..e5abf895e6a 100644
--- a/src/vim9cmds.c
+++ b/src/vim9cmds.c
@@ -2410,38 +2410,41 @@ compile_redir(char_u *line, exarg_T *eap, cctx_T *cctx)
 
     if (lhs->lhs_name != NULL)
     {
 	if (STRNCMP(arg, "END", 3) == 0)
 	{
-	    if (lhs->lhs_append)
+	    if (cctx->ctx_skip != SKIP_YES)
 	    {
-		// First load the current variable value.
-		if (compile_load_lhs_with_index(lhs, lhs->lhs_whole,
+		if (lhs->lhs_append)
+		{
+		    // First load the current variable value.
+		    if (compile_load_lhs_with_index(lhs, lhs->lhs_whole,
 								 cctx) == FAIL)
-		    return NULL;
-	    }
+			return NULL;
+		}
 
-	    // Gets the redirected text and put it on the stack, then store it
-	    // in the variable.
-	    generate_instr_type(cctx, ISN_REDIREND, &t_string);
+		// Gets the redirected text and put it on the stack, then store
+		// it in the variable.
+		generate_instr_type(cctx, ISN_REDIREND, &t_string);
 
-	    if (lhs->lhs_append)
-		generate_CONCAT(cctx, 2);
+		if (lhs->lhs_append)
+		    generate_CONCAT(cctx, 2);
 
-	    if (lhs->lhs_has_index)
-	    {
-		// Use the info in "lhs" to store the value at the index in the
-		// list or dict.
-		if (compile_assign_unlet(lhs->lhs_whole, lhs, TRUE,
+		if (lhs->lhs_has_index)
+		{
+		    // Use the info in "lhs" to store the value at the index in
+		    // the list or dict.
+		    if (compile_assign_unlet(lhs->lhs_whole, lhs, TRUE,
 						      &t_string, cctx) == FAIL)
+			return NULL;
+		}
+		else if (generate_store_lhs(cctx, lhs, -1, FALSE) == FAIL)
 		    return NULL;
-	    }
-	    else if (generate_store_lhs(cctx, lhs, -1, FALSE) == FAIL)
-		return NULL;
 
-	    VIM_CLEAR(lhs->lhs_name);
-	    VIM_CLEAR(lhs->lhs_whole);
+		VIM_CLEAR(lhs->lhs_name);
+		VIM_CLEAR(lhs->lhs_whole);
+	    }
 	    return arg + 3;
 	}
 	emsg(_(e_cannot_nest_redir));
 	return NULL;
     }
@@ -2463,17 +2466,24 @@ compile_redir(char_u *line, exarg_T *eap, cctx_T *cctx)
 					 FALSE, FALSE, FALSE, 1, cctx) == FAIL)
 	    return NULL;
 	if (need_type(&t_string, lhs->lhs_member_type,
 					    -1, 0, cctx, FALSE, FALSE) == FAIL)
 	    return NULL;
-	generate_instr(cctx, ISN_REDIRSTART);
-	lhs->lhs_append = append;
-	if (lhs->lhs_has_index)
+	if (cctx->ctx_skip == SKIP_YES)
 	{
-	    lhs->lhs_whole = vim_strnsave(arg, lhs->lhs_varlen_total);
-	    if (lhs->lhs_whole == NULL)
-		return NULL;
+	    VIM_CLEAR(lhs->lhs_name);
+	}
+	else
+	{
+	    generate_instr(cctx, ISN_REDIRSTART);
+	    lhs->lhs_append = append;
+	    if (lhs->lhs_has_index)
+	    {
+		lhs->lhs_whole = vim_strnsave(arg, lhs->lhs_varlen_total);
+		if (lhs->lhs_whole == NULL)
+		    return NULL;
+	    }
 	}
 
 	return arg + lhs->lhs_varlen_total;
     }
 
diff --git a/src/vim9compile.c b/src/vim9compile.c
index b3e1b83ea4e..73bfa6c6af1 100644
--- a/src/vim9compile.c
+++ b/src/vim9compile.c
@@ -1281,10 +1281,23 @@ vim9_declare_error(char_u *name)
 	default: return;
     }
     semsg(_(e_cannot_declare_a_scope_variable), scope, name);
 }
 
+/*
+ * Return TRUE if "name" is a valid register to use.
+ * Return FALSE and give an error message if not.
+ */
+    static int
+valid_dest_reg(int name)
+{
+    if ((name == '@' || valid_yank_reg(name, FALSE)) && name != '.')
+	return TRUE;
+    emsg_invreg(name);
+    return FAIL;
+}
+
 /*
  * For one assignment figure out the type of destination.  Return it in "dest".
  * When not recognized "dest" is not set.
  * For an option "option_scope" is set.
  * For a v:var "vimvaridx" is set.
@@ -1362,16 +1375,12 @@ get_var_dest(
 	*dest = dest_env;
 	*type = &t_string;
     }
     else if (*name == '@')
     {
-	if (name[1] != '@'
-			&& (!valid_yank_reg(name[1], FALSE) || name[1] == '.'))
-	{
-	    emsg_invreg(name[1]);
+	if (!valid_dest_reg(name[1]))
 	    return FAIL;
-	}
 	*dest = dest_reg;
 	*type = name[1] == '#' ? &t_number_or_string : &t_string;
     }
     else if (STRNCMP(name, "g:", 2) == 0)
     {
@@ -1443,11 +1452,15 @@ compile_lhs(
     // "dest_end" is the end of the destination, including "[expr]" or
     // ".name".
     // "var_end" is the end of the variable/option/etc. name.
     lhs->lhs_dest_end = skip_var_one(var_start, FALSE);
     if (*var_start == '@')
+    {
+	if (!valid_dest_reg(var_start[1]))
+	    return FAIL;
 	var_end = var_start + 2;
+    }
     else
     {
 	// skip over the leading "&", "&l:", "&g:" and "$"
 	var_end = skip_option_env_lead(var_start);
 	var_end = to_name_end(var_end, TRUE);

Vulnerable code:       def T()
          redir =>l[0]
	    if (lhs->lhs_append)
		// First load the current variable value.
		if (compile_load_lhs_with_index(lhs, lhs->lhs_whole,
		    return NULL;
	    }
	    // Gets the redirected text and put it on the stack, then store it
	    // in the variable.
	    generate_instr_type(cctx, ISN_REDIREND, &t_string);
	    if (lhs->lhs_append)
		generate_CONCAT(cctx, 2);
	    if (lhs->lhs_has_index)
	    {
		// Use the info in "lhs" to store the value at the index in the
		// list or dict.
		if (compile_assign_unlet(lhs->lhs_whole, lhs, TRUE,
	    }
	    else if (generate_store_lhs(cctx, lhs, -1, FALSE) == FAIL)
		return NULL;
	    VIM_CLEAR(lhs->lhs_name);
	    VIM_CLEAR(lhs->lhs_whole);
	generate_instr(cctx, ISN_REDIRSTART);
	lhs->lhs_append = append;
	if (lhs->lhs_has_index)
	    lhs->lhs_whole = vim_strnsave(arg, lhs->lhs_varlen_total);
	    if (lhs->lhs_whole == NULL)
		return NULL;
	if (name[1] != '@'
			&& (!valid_yank_reg(name[1], FALSE) || name[1] == '.'))
	{
	    emsg_invreg(name[1]);
	}
Fixed code: def Test_invalid_redir()
  var lines =<< trim END
      def Tone()
        if 1
          redi =>@0
          redi END
        endif
      enddef
      defcompile
  END
  v9.CheckScriptFailure(lines, 'E354:')
  delfunc g:Tone

  # this was reading past the end of the line
  lines =<< trim END
      def Ttwo()
        if 0
          redi =>@0
          redi END
        endif
      enddef
      defcompile
  END
  v9.CheckScriptFailure(lines, 'E354:')
  delfunc g:Ttwo
enddef

      def Tredir()
          redir => l[0]
  delfunc g:Tredir

  lines =<< trim END
      def Tredir()
        if 0
          redir => l[0]
        endif
        echo 'executed'
        if 0
          redir END
        endif
      enddef
      defcompile
  END
  v9.CheckScriptSuccess(lines)
  delfunc g:Tredir

  lines =<< trim END
      def Tredir()
        var l = ['']
        if 1
          redir => l[0]
        endif
        echo 'executed'
        if 0
          redir END
        else
          redir END
        endif
      enddef
      defcompile
  END
  v9.CheckScriptSuccess(lines)
  delfunc g:Tredir

  lines =<< trim END
      let doit = 1
      def Tredir()
        var l = ['']
        if g:doit
          redir => l[0]
        endif
        echo 'executed'
        if g:doit
          redir END
        endif
      enddef
      defcompile
  END
  v9.CheckScriptSuccess(lines)
  delfunc g:Tredir
/**/
    742,
	    if (cctx->ctx_skip != SKIP_YES)
		if (lhs->lhs_append)
		{
		    // First load the current variable value.
		    if (compile_load_lhs_with_index(lhs, lhs->lhs_whole,
			return NULL;
		}
		// Gets the redirected text and put it on the stack, then store
		// it in the variable.
		generate_instr_type(cctx, ISN_REDIREND, &t_string);
		if (lhs->lhs_append)
		    generate_CONCAT(cctx, 2);
		if (lhs->lhs_has_index)
		{
		    // Use the info in "lhs" to store the value at the index in
		    // the list or dict.
		    if (compile_assign_unlet(lhs->lhs_whole, lhs, TRUE,
			return NULL;
		}
		else if (generate_store_lhs(cctx, lhs, -1, FALSE) == FAIL)
		VIM_CLEAR(lhs->lhs_name);
		VIM_CLEAR(lhs->lhs_whole);
	    }
	if (cctx->ctx_skip == SKIP_YES)
	    VIM_CLEAR(lhs->lhs_name);
	}
	else
	{
	    generate_instr(cctx, ISN_REDIRSTART);
	    lhs->lhs_append = append;
	    if (lhs->lhs_has_index)
	    {
		lhs->lhs_whole = vim_strnsave(arg, lhs->lhs_varlen_total);
		if (lhs->lhs_whole == NULL)
		    return NULL;
	    }
/*
 * Return TRUE if "name" is a valid register to use.
 * Return FALSE and give an error message if not.
 */
    static int
valid_dest_reg(int name)
{
    if ((name == '@' || valid_yank_reg(name, FALSE)) && name != '.')
	return TRUE;
    emsg_invreg(name);
    return FAIL;
}

	if (!valid_dest_reg(name[1]))
    {
	if (!valid_dest_reg(var_start[1]))
	    return FAIL;
    }