Commit: patch 9.2.0936: stringifying a list or dict can free the item being iterated

patch 9.2.0936: stringifying a list or dict can free the item being iterated

Commit: https://github.com/vim/vim/commit/5f54b1dc4cc87e7825f7585dc73a488e67985915
Author: Samuel Schlesinger <[email protected]>
Date:   Tue Aug 11 18:28:47 2026 +0000

    patch 9.2.0936: stringifying a list or dict can free the item being iterated
    
    Problem:  match(), matchstr(), matchend(), matchlist() and matchstrpos()
              over a list, join() of a list, and string() or :echo of a
              list or dict stringify each item while iterating over the
              container.  For an object item this runs the user-defined
              string() method, which can remove the item the loop is
              standing on, or grow a dict so its hash table is
              reallocated, leaving the loop reading freed memory.
    Solution: Lock the container while iterating, so a change from the
              string() method fails with E741 instead of corrupting the
              iterator, the same way filter(), map(), sort() and reduce()
              already do.  For a dict also hash_lock() it, so growing it
              cannot reallocate the hash table, mirroring the dict path of
              filter()/map() (Samuel Schlesinger).
    
    find_some_match() in evalfunc.c, list_join_inner() in list.c and
    dict2string() in dict.c each iterated a container while calling
    echo_string(); list_reduce() was the existing pattern they were
    missing.
    
    Add tests: match() and matchstr() over a list, join() and string() of
    a list, and string() of a dict, each with an object whose string()
    method mutates the container; every one crashes an unpatched Vim.
    
    closes: #21001
    
    Co-Authored-By: Claude <[email protected]>
    Signed-off-by: Samuel Schlesinger <[email protected]>
    Signed-off-by: Christian Brabandt <[email protected]>

diff --git a/src/dict.c b/src/dict.c
index 5531cc72a..029fcfe78 100644
--- a/src/dict.c
+++ b/src/dict.c
@@ -809,12 +809,22 @@ dict2string(typval_T *tv, int copyID, int restore_copyID)
     char_u	*s;
     dict_T	*d;
     int		todo;
+    int		prev_lock;
 
     if ((d = tv->vval.v_dict) == NULL)
 	return NULL;
     ga_init2(&ga, sizeof(char), 80);
     ga_append(&ga, '{');
 
+    // Lock the dictionary, so that user code that echo_string_core() below
+    // may invoke, such as the string() method of an object, cannot remove
+    // an item or add one and cause the hash table to be reallocated while
+    // we are iterating over it.
+    prev_lock = d->dv_lock;
+    if (d->dv_lock == 0)
+	d->dv_lock = VAR_LOCKED;
+    hash_lock(&d->dv_hashtab);
+
     todo = (int)d->dv_hashtab.ht_used;
     FOR_ALL_HASHTAB_ITEMS(&d->dv_hashtab, hi, todo)
     {
@@ -845,6 +855,8 @@ dict2string(typval_T *tv, int copyID, int restore_copyID)
 
 	}
     }
+    hash_unlock(&d->dv_hashtab);
+    d->dv_lock = prev_lock;
     if (todo > 0)
     {
 	vim_free(ga.ga_data);
diff --git a/src/evalfunc.c b/src/evalfunc.c
index 23dc21e95..472b2fe02 100644
--- a/src/evalfunc.c
+++ b/src/evalfunc.c
@@ -9256,6 +9256,7 @@ find_some_match(typval_T *argvars, typval_T *rettv, matchtype_T type)
     list_T	*l = NULL;
     listitem_T	*li = NULL;
     long	idx = 0;
+    int		prev_lock = 0;
     char_u	*tofree = NULL;
 
     // Make 'cpoptions' empty, the 'l' flag should not be used here.
@@ -9358,6 +9359,16 @@ find_some_match(typval_T *argvars, typval_T *rettv, matchtype_T type)
     {
 	regmatch.rm_ic = p_ic;
 
+	// Lock the list, so that the item the loop is standing on cannot
+	// be freed by user code that echo_string() below may invoke: the
+	// string() method of an object could remove the item.
+	if (l != NULL)
+	{
+	    prev_lock = l->lv_lock;
+	    if (l->lv_lock == 0)
+		l->lv_lock = VAR_LOCKED;
+	}
+
 	for (;;)
 	{
 	    if (l != NULL)
@@ -9460,6 +9471,8 @@ find_some_match(typval_T *argvars, typval_T *rettv, matchtype_T type)
 		rettv->vval.v_number += (varnumber_T)(str - expr);
 	    }
 	}
+	if (l != NULL)
+	    l->lv_lock = prev_lock;
 	vim_regfree(regmatch.regprog);
     }
 
diff --git a/src/list.c b/src/list.c
index e400cb096..82243c82b 100644
--- a/src/list.c
+++ b/src/list.c
@@ -1528,6 +1528,7 @@ list_join_inner(
     join_T	*p;
     long	sumlen = 0;
     int		first = TRUE;
+    int		prev_lock;
     char_u	*tofree;
     char_u	numbuf[NUMBUFLEN];
     listitem_T	*item;
@@ -1536,12 +1537,21 @@ list_join_inner(
 
     // Stringify each item in the list.
     CHECK_LIST_MATERIALIZE(l);
+    // Lock the list, so that the item the loop is standing on cannot be
+    // freed by user code that echo_string_core() below may invoke: the
+    // string() method of an object could remove the item.
+    prev_lock = l->lv_lock;
+    if (l->lv_lock == 0)
+	l->lv_lock = VAR_LOCKED;
     for (item = l->lv_first; item != NULL && !got_int; item = item->li_next)
     {
 	s.string = echo_string_core(&item->li_tv, &tofree, numbuf, copyID,
 				      echo_style, restore_copyID, !echo_style);
 	if (s.string == NULL)
+	{
+	    l->lv_lock = prev_lock;
 	    return FAIL;
+	}
 
 	s.length = STRLEN(s.string);
 	sumlen += (long)s.length;
@@ -1565,6 +1575,7 @@ list_join_inner(
 	if (did_echo_string_emsg)  // recursion error, bail out
 	    break;
     }
+    l->lv_lock = prev_lock;
 
     // Allocate result buffer with its total size, avoid re-allocation and
     // multiple copy operations.  Add 2 for a tailing ']' and NUL.
diff --git a/src/testdir/test_functions.vim b/src/testdir/test_functions.vim
index 2f2a7fc5f..3aec6c524 100644
--- a/src/testdir/test_functions.vim
+++ b/src/testdir/test_functions.vim
@@ -1253,6 +1253,89 @@ func Test_matchstrpos()
   call assert_equal(['', -1, -1], matchstrpos(test_null_list(), ' '))
 endfunc
 
+" While match() iterates over a list, stringifying an item can run the
+" string() method of an object, which must not be able to free the item
+" the loop is standing on.
+func Test_match_list_changed_while_matching()
+  let lines =<< trim END
+    vim9script
+    class C
+      def string(): string
+        if !g:removed
+          g:removed = true
+          remove(g:mlist, 0)
+        endif
+        return 'nostring'
+      enddef
+    endclass
+    g:mlist = [C.new(), C.new(), C.new()]
+  END
+  call writefile(lines, 'Xmatchmutate.vim', 'D')
+  let g:removed = v:false
+  source Xmatchmutate.vim
+  call assert_fails('call match(g:mlist, "xyz")', 'E741:')
+  call assert_equal(3, len(g:mlist))
+  let g:removed = v:false
+  call assert_fails('call matchstr(g:mlist, "xyz")', 'E741:')
+  call assert_equal(3, len(g:mlist))
+  unlet g:mlist g:removed
+endfunc
+
+" Same for join() and string(): stringifying an item can run the string()
+" method of an object, which must not be able to free the item the loop is
+" standing on.
+func Test_join_list_changed_while_stringified()
+  let lines =<< trim END
+    vim9script
+    class C
+      def string(): string
+        if !g:removed
+          g:removed = true
+          remove(g:jlist, 0)
+        endif
+        return 'nostring'
+      enddef
+    endclass
+    g:jlist = [C.new(), C.new(), C.new()]
+  END
+  call writefile(lines, 'Xjoinmutate.vim', 'D')
+  let g:removed = v:false
+  source Xjoinmutate.vim
+  call assert_fails('call join(g:jlist, ",")', 'E741:')
+  call assert_equal(3, len(g:jlist))
+  let g:removed = v:false
+  call assert_fails('call string(g:jlist)', 'E741:')
+  call assert_equal(3, len(g:jlist))
+  unlet g:jlist g:removed
+endfunc
+
+" Same for a dict: stringifying a value can run the string() method of an
+" object, which must not be able to remove an item and free it, or grow the
+" dict and reallocate the hash table, while it is being iterated over.
+func Test_dict_changed_while_stringified()
+  let lines =<< trim END
+    vim9script
+    class C
+      def string(): string
+        if !g:removed
+          g:removed = true
+          for k in keys(g:d)
+            remove(g:d, k)
+          endfor
+        endif
+        return 'nostring'
+      enddef
+    endclass
+    g:d = {'a': C.new(), 'b': C.new(), 'c': C.new()}
+  END
+  call writefile(lines, 'Xdictmutate.vim', 'D')
+  let g:removed = v:false
+  source Xdictmutate.vim
+  call assert_fails('call string(g:d)', 'E741:')
+  call assert_equal(3, len(g:d))
+  unlet g:d g:removed
+endfunc
+
 " Test for matchstrlist()
 func Test_matchstrlist()
   let lines =<< trim END
diff --git a/src/version.c b/src/version.c
index 45ddeda96..2cbf92a2e 100644
--- a/src/version.c
+++ b/src/version.c
@@ -763,6 +763,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    936,
 /**/
     935,
 /**/

-- 
-- 
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

--- 
You received this message because you are subscribed to the Google Groups "vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To view this discussion visit https://groups.google.com/d/msgid/vim_dev/E1wtrTE-007Wn3-4d%40256bit.org.