CVE-2021-4173: Use After Free (CWE-416)
value: vim is vulnerable to Use After Free
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: 2021-12-27T13:15Z
Repository: https://github.com/vim/vim
Language: C
Files changed: src/proto/userfunc.pro, src/testdir/test_vim9_func.vim, src/userfunc.c, src/version.c, src/vim9compile.c, src/vim9execute.c
Commit date: 2021-12-26 14:23:22 +0000
Commit message: patch 8.2.3902: Vim9: double free with nested :def function

Problem:    Vim9: double free with nested :def function.
Solution:   Pass "line_to_free" from compile_def_function() and make sure
            cmdlinep is valid.

Diff: diff --git a/src/proto/userfunc.pro b/src/proto/userfunc.pro
index 0320f09d367..311cee2e128 100644
--- a/src/proto/userfunc.pro
+++ b/src/proto/userfunc.pro
@@ -36,11 +36,11 @@ char_u *printable_func_name(ufunc_T *fp);
 char_u *trans_function_name(char_u **pp, int *is_global, int skip, int flags, funcdict_T *fdp, partial_T **partial, type_T **type);
 char_u *untrans_function_name(char_u *name);
 char_u *get_scriptlocal_funcname(char_u *funcname);
 char_u *save_function_name(char_u **name, int *is_global, int skip, int flags, funcdict_T *fudi);
 void list_functions(regmatch_T *regmatch);
-ufunc_T *define_function(exarg_T *eap, char_u *name_arg);
+ufunc_T *define_function(exarg_T *eap, char_u *name_arg, char_u **line_to_free);
 void ex_function(exarg_T *eap);
 void ex_defcompile(exarg_T *eap);
 int eval_fname_script(char_u *p);
 int translated_function_exists(char_u *name, int is_global);
 int has_varargs(ufunc_T *ufunc);
diff --git a/src/testdir/test_vim9_func.vim b/src/testdir/test_vim9_func.vim
index 5b8ad17c780..1a14c101679 100644
--- a/src/testdir/test_vim9_func.vim
+++ b/src/testdir/test_vim9_func.vim
@@ -1667,10 +1667,30 @@ enddef
 def Test_error_in_nested_function()
   # Error in called function requires unwinding the call stack.
   assert_fails('FuncWithForwardCall()', 'E1096:', '', 1, 'FuncWithForwardCall')
 enddef
 
+def Test_nested_functin_with_nextcmd()
+  var lines =<< trim END
+      vim9script
+      # Define an outer function
+      def FirstFunction()
+        # Define an inner function
+        def SecondFunction()
+          # the function has a body, a double free is detected.
+          AAAAA
+
+         # enddef followed by | or } followed by # one or more characters
+         enddef|BBBB
+      enddef
+
+      # Compile all functions
+      defcompile
+  END
+  CheckScriptFailure(lines, 'E476: Invalid command: AAAAA')
+enddef
+
 def Test_return_type_wrong()
   CheckScriptFailure([
         'def Func(): number',
         'return "a"',
         'enddef',
diff --git a/src/userfunc.c b/src/userfunc.c
index 8971740922b..80c64d0deb9 100644
--- a/src/userfunc.c
+++ b/src/userfunc.c
@@ -718,16 +718,18 @@ get_function_body(
 		line_arg = p + 1;
 	    }
 	}
 	else
 	{
-	    vim_free(*line_to_free);
 	    if (eap->getline == NULL)
 		theline = getcmdline(':', 0L, indent, getline_options);
 	    else
 		theline = eap->getline(':', eap->cookie, indent,
 							      getline_options);
+	    if (*eap->cmdlinep == *line_to_free)
+		*eap->cmdlinep = theline;
+	    vim_free(*line_to_free);
 	    *line_to_free = theline;
 	}
 	if (KeyTyped)
 	    lines_left = Rows - 1;
 	if (theline == NULL)
@@ -835,11 +837,12 @@ get_function_body(
 		    {
 			// Another command follows. If the line came from "eap"
 			// we can simply point into it, otherwise we need to
 			// change "eap->cmdlinep".
 			eap->nextcmd = nextcmd;
-			if (*line_to_free != NULL)
+			if (*line_to_free != NULL
+					    && *eap->cmdlinep != *line_to_free)
 			{
 			    vim_free(*eap->cmdlinep);
 			    *eap->cmdlinep = *line_to_free;
 			    *line_to_free = NULL;
 			}
@@ -1159,11 +1162,11 @@ lambda_function_body(
 	    ((char_u **)gap->ga_data)[gap->ga_len++] = pnl;
 	    ((char_u **)freegap->ga_data)[freegap->ga_len++] = pnl;
 	}
 	if (ga_grow(gap, 1) == FAIL || ga_grow(freegap, 1) == FAIL)
 	    goto erret;
-	if (cmdline != NULL)
+	if (eap.nextcmd != NULL)
 	    // more is following after the "}", which was skipped
 	    last = cmdline;
 	else
 	    // nothing is following the "}"
 	    last = (char_u *)"}";
@@ -1173,11 +1176,11 @@ lambda_function_body(
 	    mch_memmove(pnl + 1, last, plen + 1);
 	((char_u **)gap->ga_data)[gap->ga_len++] = pnl;
 	((char_u **)freegap->ga_data)[freegap->ga_len++] = pnl;
     }
 
-    if (cmdline != NULL)
+    if (eap.nextcmd != NULL)
     {
 	garray_T *tfgap = &evalarg->eval_tofree_ga;
 
 	// Something comes after the "}".
 	*arg = eap.nextcmd;
@@ -1185,10 +1188,12 @@ lambda_function_body(
 	// "arg" points into cmdline, need to keep the line and free it later.
 	if (ga_grow(tfgap, 1) == OK)
 	{
 	    ((char_u **)(tfgap->ga_data))[tfgap->ga_len++] = cmdline;
 	    evalarg->eval_using_cmdline = TRUE;
+	    if (cmdline == line_to_free)
+		line_to_free = NULL;
 	}
     }
     else
 	*arg = (char_u *)"";
 
@@ -3986,13 +3991,12 @@ list_functions(regmatch_T *regmatch)
  * When "name_arg" is not NULL this is a nested function, using "name_arg" for
  * the function name.
  * Returns a pointer to the function or NULL if no function defined.
  */
     ufunc_T *
-define_function(exarg_T *eap, char_u *name_arg)
+define_function(exarg_T *eap, char_u *name_arg, char_u **line_to_free)
 {
-    char_u	*line_to_free = NULL;
     int		j;
     int		c;
     int		saved_did_emsg;
     char_u	*name = name_arg;
     int		is_global = FALSE;
@@ -4256,11 +4260,11 @@ define_function(exarg_T *eap, char_u *name_arg)
     // invalid.
     ++p;
     if (get_function_args(&p, ')', &newargs,
 			eap->cmdidx == CMD_def ? &argtypes : NULL, FALSE,
 			 NULL, &varargs, &default_args, eap->skip,
-			 eap, &line_to_free) == FAIL)
+			 eap, line_to_free) == FAIL)
 	goto errret_2;
     whitep = p;
 
     if (eap->cmdidx == CMD_def)
     {
@@ -4366,11 +4370,11 @@ define_function(exarg_T *eap, char_u *name_arg)
     // Save the starting line number.
     sourcing_lnum_top = SOURCING_LNUM;
 
     // Do not define the function when getting the body fails and when
     // skipping.
-    if (get_function_body(eap, &newlines, line_arg, &line_to_free) == FAIL
+    if (get_function_body(eap, &newlines, line_arg, line_to_free) == FAIL
 	    || eap->skip)
 	goto erret;
 
     /*
      * If there are no errors, add the function
@@ -4658,11 +4662,10 @@ errret_2:
 	vim_free(fp);
 	fp = NULL;
     }
 ret_free:
     ga_clear_strings(&argtypes);
-    vim_free(line_to_free);
     vim_free(fudi.fd_newkey);
     if (name != name_arg)
 	vim_free(name);
     vim_free(ret_type);
     did_emsg |= saved_did_emsg;
@@ -4674,11 +4677,14 @@ ret_free:
  * ":function"
  */
     void
 ex_function(exarg_T *eap)
 {
-    (void)define_function(eap, NULL);
+    char_u *line_to_free = NULL;
+
+    (void)define_function(eap, NULL, &line_to_free);
+    vim_free(line_to_free);
 }
 
 /*
  * :defcompile - compile all :def functions in the current script that need to
  * be compiled.  Except dead functions.  Doesn't do profiling.
diff --git a/src/version.c b/src/version.c
index 9a5ef2cc993..55ddb069c87 100644
--- a/src/version.c
+++ b/src/version.c
@@ -747,10 +747,12 @@ static char *(features[]) =
 	NULL
 };
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    3902,
 /**/
     3901,
 /**/
     3900,
 /**/
diff --git a/src/vim9compile.c b/src/vim9compile.c
index e9d6089b10a..b23145ebcdf 100644
--- a/src/vim9compile.c
+++ b/src/vim9compile.c
@@ -810,15 +810,17 @@ func_needs_compiling(ufunc_T *ufunc, compiletype_T compile_type)
 
 /*
  * Compile a nested :def command.
  */
     static char_u *
-compile_nested_function(exarg_T *eap, cctx_T *cctx)
+compile_nested_function(exarg_T *eap, cctx_T *cctx, char_u **line_to_free)
 {
     int		is_global = *eap->arg == 'g' && eap->arg[1] == ':';
     char_u	*name_start = eap->arg;
     char_u	*name_end = to_name_end(eap->arg, TRUE);
+    int		off;
+    char_u	*func_name;
     char_u	*lambda_name;
     ufunc_T	*ufunc;
     int		r = FAIL;
     compiletype_T   compile_type;
 
@@ -864,11 +866,21 @@ compile_nested_function(exarg_T *eap, cctx_T *cctx)
     eap->forceit = FALSE;
     // We use the special <Lamba>99 name, but it's not really a lambda.
     lambda_name = vim_strsave(get_lambda_name());
     if (lambda_name == NULL)
 	return NULL;
-    ufunc = define_function(eap, lambda_name);
+
+    // This may free the current line, make a copy of the name.
+    off = is_global ? 2 : 0;
+    func_name = vim_strnsave(name_start + off, name_end - name_start - off);
+    if (func_name == NULL)
+    {
+	r = FAIL;
+	goto theend;
+    }
+
+    ufunc = define_function(eap, lambda_name, line_to_free);
 
     if (ufunc == NULL)
     {
 	r = eap->skip ? OK : FAIL;
 	goto theend;
@@ -909,25 +921,18 @@ compile_nested_function(exarg_T *eap, cctx_T *cctx)
 	compile_def_function(ufunc, FALSE, CT_NONE, cctx);
 #endif
 
     if (is_global)
     {
-	char_u *func_name = vim_strnsave(name_start + 2,
-						    name_end - name_start - 2);
-
-	if (func_name == NULL)
-	    r = FAIL;
-	else
-	{
-	    r = generate_NEWFUNC(cctx, lambda_name, func_name);
-	    lambda_name = NULL;
-	}
+	r = generate_NEWFUNC(cctx, lambda_name, func_name);
+	func_name = NULL;
+	lambda_name = NULL;
     }
     else
     {
 	// Define a local variable for the function reference.
-	lvar_T	*lvar = reserve_local(cctx, name_start, name_end - name_start,
+	lvar_T	*lvar = reserve_local(cctx, func_name, name_end - name_start,
 						    TRUE, ufunc->uf_func_type);
 
 	if (lvar == NULL)
 	    goto theend;
 	if (generate_FUNCREF(cctx, ufunc) == FAIL)
@@ -935,10 +940,11 @@ compile_nested_function(exarg_T *eap, cctx_T *cctx)
 	r = generate_STORE(cctx, ISN_STORE, lvar->lv_idx, NULL);
     }
 
 theend:
     vim_free(lambda_name);
+    vim_free(func_name);
     return r == FAIL ? NULL : (char_u *)"";
 }
 
 /*
  * Return the length of an assignment operator, or zero if there isn't one.
@@ -2859,11 +2865,11 @@ compile_def_function(
 	switch (ea.cmdidx)
 	{
 	    case CMD_def:
 	    case CMD_function:
 		    ea.arg = p;
-		    line = compile_nested_function(&ea, &cctx);
+		    line = compile_nested_function(&ea, &cctx, &line_to_free);
 		    break;
 
 	    case CMD_return:
 		    line = compile_return(p, check_return_type,
 				 local_cmdmod.cmod_flags & CMOD_LEGACY, &cctx);
diff --git a/src/vim9execute.c b/src/vim9execute.c
index 49336c0992d..9814f2514ba 100644
--- a/src/vim9execute.c
+++ b/src/vim9execute.c
@@ -3343,14 +3343,16 @@ exec_instructions(ectx_T *ectx)
 		if (iptr->isn_arg.string == NULL)
 		    list_functions(NULL);
 		else
 		{
 		    exarg_T ea;
+		    char_u  *line_to_free = NULL;
 
 		    CLEAR_FIELD(ea);
 		    ea.cmd = ea.arg = iptr->isn_arg.string;
-		    define_function(&ea, NULL);
+		    define_function(&ea, NULL, &line_to_free);
+		    vim_free(line_to_free);
 		}
 		break;
 
 	    // jump if a condition is met
 	    case ISN_JUMP:

Vulnerable code: ufunc_T *define_function(exarg_T *eap, char_u *name_arg);
	    vim_free(*line_to_free);
			if (*line_to_free != NULL)
	if (cmdline != NULL)
    if (cmdline != NULL)
define_function(exarg_T *eap, char_u *name_arg)
    char_u	*line_to_free = NULL;
			 eap, &line_to_free) == FAIL)
    if (get_function_body(eap, &newlines, line_arg, &line_to_free) == FAIL
    vim_free(line_to_free);
    (void)define_function(eap, NULL);
compile_nested_function(exarg_T *eap, cctx_T *cctx)
    ufunc = define_function(eap, lambda_name);
	char_u *func_name = vim_strnsave(name_start + 2,
						    name_end - name_start - 2);

	if (func_name == NULL)
	    r = FAIL;
	else
	{
	    r = generate_NEWFUNC(cctx, lambda_name, func_name);
	    lambda_name = NULL;
	}
	lvar_T	*lvar = reserve_local(cctx, name_start, name_end - name_start,
		    line = compile_nested_function(&ea, &cctx);
		    define_function(&ea, NULL);
Fixed code: ufunc_T *define_function(exarg_T *eap, char_u *name_arg, char_u **line_to_free);
def Test_nested_functin_with_nextcmd()
  var lines =<< trim END
      vim9script
      # Define an outer function
      def FirstFunction()
        # Define an inner function
        def SecondFunction()
          # the function has a body, a double free is detected.
          AAAAA

         # enddef followed by | or } followed by # one or more characters
         enddef|BBBB
      enddef

      # Compile all functions
      defcompile
  END
  CheckScriptFailure(lines, 'E476: Invalid command: AAAAA')
enddef

	    if (*eap->cmdlinep == *line_to_free)
		*eap->cmdlinep = theline;
	    vim_free(*line_to_free);
			if (*line_to_free != NULL
					    && *eap->cmdlinep != *line_to_free)
	if (eap.nextcmd != NULL)
    if (eap.nextcmd != NULL)
	    if (cmdline == line_to_free)
		line_to_free = NULL;
define_function(exarg_T *eap, char_u *name_arg, char_u **line_to_free)
			 eap, line_to_free) == FAIL)
    if (get_function_body(eap, &newlines, line_arg, line_to_free) == FAIL
    char_u *line_to_free = NULL;

    (void)define_function(eap, NULL, &line_to_free);
    vim_free(line_to_free);
/**/
    3902,
compile_nested_function(exarg_T *eap, cctx_T *cctx, char_u **line_to_free)
    int		off;
    char_u	*func_name;

    // This may free the current line, make a copy of the name.
    off = is_global ? 2 : 0;
    func_name = vim_strnsave(name_start + off, name_end - name_start - off);
    if (func_name == NULL)
    {
	r = FAIL;
	goto theend;
    }

    ufunc = define_function(eap, lambda_name, line_to_free);
	r = generate_NEWFUNC(cctx, lambda_name, func_name);
	func_name = NULL;
	lambda_name = NULL;
	lvar_T	*lvar = reserve_local(cctx, func_name, name_end - name_start,
    vim_free(func_name);
		    line = compile_nested_function(&ea, &cctx, &line_to_free);
		    char_u  *line_to_free = NULL;
		    define_function(&ea, NULL, &line_to_free);
		    vim_free(line_to_free);