patch 9.1.1410: out-of-bounds access with 'completefunc'

Problem:  out-of-bounds access with 'completefunc' (csetc)
Solution: check if it is safe to advance cpt_sources_index
          (Girish Palya)

fixes: #17363
closes: #17374

Co-authored-by: @csetc
Signed-off-by: Girish Palya <girishji@gmail.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>

diff --git a/src/insexpand.c b/src/insexpand.c
index 700f734cba1df14dfd8b5c99dfe64428c15bdb49..3592fdcaf2b4ee08b04c4667234dd73f76c0aa9f 100644 (file)
--- a/src/insexpand.c
+++ b/src/insexpand.c
@@ -227,7 +227,7 @@ typedef struct cpt_source_T
 
 static cpt_source_T *cpt_sources_array; // Pointer to the array of completion sources
 static int         cpt_sources_count;  // Total number of completion sources specified in the 'cpt' option
-static int         cpt_sources_index;  // Index of the current completion source being expanded
+static int         cpt_sources_index = -1;  // Index of the current completion source being expanded
 
 // "compl_match_array" points the currently displayed list of entries in the
 // popup menu.  It is NULL when there is no popup menu.
@@ -3949,6 +3949,19 @@ thesaurus_func_complete(int type UNUSED)
 #endif
 }
 
+/*
+ * Check if 'cpt' list index can be advanced to the next completion source.
+ */
+    static int
+may_advance_cpt_index(char_u *cpt)
+{
+    char_u  *p = cpt;
+
+    while (*p == ',' || *p == ' ') // Skip delimiters
+       p++;
+    return (*p != NUL);
+}
+
 /*
  * Return value of process_next_cpt_value()
  */
@@ -4004,12 +4017,14 @@ process_next_cpt_value(
        ins_compl_next_state_T *st,
        int             *compl_type_arg,
        pos_T           *start_match_pos,
-       int             fuzzy_collect)
+       int             fuzzy_collect,
+       int             *advance_cpt_idx)
 {
     int            compl_type = -1;
     int            status = INS_COMPL_CPT_OK;
 
     st->found_all = FALSE;
+    *advance_cpt_idx = FALSE;
 
     while (*st->e_cpt == ',' || *st->e_cpt == ' ')
        st->e_cpt++;
@@ -4124,6 +4139,7 @@ process_next_cpt_value(
 
        // in any case e_cpt is advanced to the next entry
        (void)copy_option_part(&st->e_cpt, IObuff, IOSIZE, ",");
+       *advance_cpt_idx = may_advance_cpt_index(st->e_cpt);
 
        st->found_all = TRUE;
        if (compl_type == -1)
@@ -4976,6 +4992,23 @@ strip_caret_numbers_in_place(char_u *str)
     *write = '\0';
 }
 
+/*
+ * Safely advance the cpt_sources_index by one.
+ */
+    static int
+advance_cpt_sources_index_safe(void)
+{
+    if (cpt_sources_index < cpt_sources_count - 1)
+    {
+       cpt_sources_index++;
+       return OK;
+    }
+#ifdef FEAT_EVAL
+    semsg(_(e_list_index_out_of_range_nr), cpt_sources_index + 1);
+#endif
+    return FAIL;
+}
+
 /*
  * Get the next expansion(s), using "compl_pattern".
  * The search starts at position "ini" in curbuf and in the direction
@@ -4993,6 +5026,7 @@ ins_compl_get_exp(pos_T *ini)
     int                i;
     int                found_new_match;
     int                type = ctrl_x_mode;
+    int                may_advance_cpt_idx = FALSE;
 
     if (!compl_started)
     {
@@ -5027,7 +5061,8 @@ ins_compl_get_exp(pos_T *ini)
                                    ? &st.last_match_pos : &st.first_match_pos;
 
     // For ^N/^P loop over all the flags/windows/buffers in 'complete'.
-    for (cpt_sources_index = 0;;)
+    cpt_sources_index = 0;
+    for (;;)
     {
        found_new_match = FAIL;
        st.set_match_pos = FALSE;
@@ -5038,13 +5073,15 @@ ins_compl_get_exp(pos_T *ini)
        if ((ctrl_x_mode_normal() || ctrl_x_mode_line_or_eval())
                                        && (!compl_started || st.found_all))
        {
-           int status = process_next_cpt_value(&st, &type, ini, cfc_has_mode());
+           int status = process_next_cpt_value(&st, &type, ini,
+                   cfc_has_mode(), &may_advance_cpt_idx);
 
            if (status == INS_COMPL_CPT_END)
                break;
            if (status == INS_COMPL_CPT_CONT)
            {
-               cpt_sources_index++;
+               if (may_advance_cpt_idx && !advance_cpt_sources_index_safe())
+                   break;
                continue;
            }
        }
@@ -5057,8 +5094,8 @@ ins_compl_get_exp(pos_T *ini)
        // get the next set of completion matches
        found_new_match = get_next_completion_match(type, &st, ini);
 
-       if (type > 0)
-           cpt_sources_index++;
+       if (may_advance_cpt_idx && !advance_cpt_sources_index_safe())
+           break;
 
        // break the loop for specialized modes (use 'complete' just for the
        // generic ctrl_x_mode == CTRL_X_NORMAL) or when we've found a new
@@ -6907,7 +6944,7 @@ cpt_compl_refresh(void)
     strip_caret_numbers_in_place(cpt);
 
     cpt_sources_index = 0;
-    for (p = cpt; *p; cpt_sources_index++)
+    for (p = cpt; *p;)
     {
        while (*p == ',' || *p == ' ') // Skip delimiters
            p++;
@@ -6926,7 +6963,9 @@ cpt_compl_refresh(void)
            }
        }
 
-       copy_option_part(&p, IObuff, IOSIZE, ","); // Advance p
+       (void)copy_option_part(&p, IObuff, IOSIZE, ","); // Advance p
+       if (may_advance_cpt_index(p))
+           (void)advance_cpt_sources_index_safe();
     }
     cpt_sources_index = -1;
 

diff --git a/src/testdir/test_ins_complete.vim b/src/testdir/test_ins_complete.vim
index 06ab6cfa9a820248022968a3c0512a61a72acaba..ffe549ab5c806e1f2549ea94b80b1e96c0d1dd0b 100644 (file)
--- a/src/testdir/test_ins_complete.vim
+++ b/src/testdir/test_ins_complete.vim
@@ -1037,7 +1037,7 @@ func Test_completefunc_invalid_data()
   exe "normal i\<C-N>"
   call assert_equal('moon', getline(1))
   set completefunc& complete&
-  close!
+  bw!
 endfunc
 
 " Test for errors in using complete() function
@@ -4126,6 +4126,11 @@ func Test_complete_match_count()
   exe "normal! Gof\<c-n>\<c-r>=PrintMenuWords()\<cr>"
   call assert_equal('fo{''matches'': [''fo'', ''foo1'', ''foo2''], ''selected'': 0}', getline(5))
   5d
+  set cpt=.^1,,,F^2,,,
+  call setline(1, ["fo", "foo", "foobar", "fobarbaz"])
+  exe "normal! Gof\<c-n>\<c-r>=PrintMenuWords()\<cr>"
+  call assert_equal('fo{''matches'': [''fo'', ''foo1'', ''foo2''], ''selected'': 0}', getline(5))
+  5d
   exe "normal! Gof\<c-n>\<c-n>\<c-r>=PrintMenuWords()\<cr>"
   call assert_equal('foo1{''matches'': [''fo'', ''foo1'', ''foo2''], ''selected'': 1}', getline(5))
   5d
@@ -4602,4 +4607,41 @@ func Test_register_completion()
   set ignorecase&
 endfunc
 
+" Test refresh:always with unloaded buffers (issue #17363)
+func Test_complete_unloaded_buf_refresh_always()
+  func TestComplete(findstart, base)
+    if a:findstart
+      let line = getline('.')
+      let start = col('.') - 1
+      while start > 0 && line[start - 1] =~ '\a'
+        let start -= 1
+      endwhile
+      return start
+    else
+      let g:CallCount += 1
+      let res = ["update1", "update12", "update123"]
+      return #{words: res, refresh: 'always'}
+    endif
+  endfunc
+
+  let g:CallCount = 0
+  set completeopt=menu,longest
+  set completefunc=TestComplete
+  set complete=b,u,t,i,F
+  badd foo1
+  badd foo2
+  new
+  exe "normal! iup\<C-N>\<BS>\<BS>\<BS>\<BS>\<BS>"
+  call assert_equal('up', getline(1))
+  call assert_equal(6, g:CallCount)
+
+  bd! foo1
+  bd! foo2
+  bw!
+  set completeopt&
+  set complete&
+  set completefunc&
+  delfunc TestComplete
+endfunc
+
 " vim: shiftwidth=2 sts=2 expandtab nofoldenable

diff --git a/src/version.c b/src/version.c
index a9d95e3fe4f2419b3f25a0bfcd84a6e95bb6d0ab..33b08bb3aaf72a823f5162ffa94b5dc43538c2d8 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -709,6 +709,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1410,
 /**/
     1409,
 /**/