patch 9.2.0331: spellfile: stack buffer overflows in spell file generation

Problem:  spell_read_aff() uses sprintf() into a fixed-size stack buffer
          without bounds checking. store_aff_word() uses STRCAT() to
          append attacker-controlled strings into newword[MAXWLEN] without
          checking remaining space. Both are reachable via :mkspell with
          crafted .aff/.dic files (xinyi234)
Solution: Replace sprintf() with vim_snprintf() in spell_read_aff().
          Replace STRCAT() with STRNCAT() with explicit remaining-space
          calculation in store_aff_word().

closes: #19944

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

diff --git a/src/spellfile.c b/src/spellfile.c
index c570920795032f759a2e98ddafcd2162bd200eea..a9a347a89a97856ade981267a0f5e71bd67ba46a 100644 (file)
--- a/src/spellfile.c
+++ b/src/spellfile.c
@@ -2736,10 +2736,12 @@ spell_read_aff(spellinfo_T *spin, char_u *fname)
                        char_u  buf[MAXLINELEN];
 
                        aff_entry->ae_cond = getroom_save(spin, items[4]);
+                       // Note: this silently truncates the buffer, but this should
+                       // not happen in practice
                        if (*items[0] == 'P')
-                           sprintf((char *)buf, "^%s", items[4]);
+                           vim_snprintf((char *)buf, sizeof(buf), "^%s", items[4]);
                        else
-                           sprintf((char *)buf, "%s$", items[4]);
+                           vim_snprintf((char *)buf, sizeof(buf), "%s$", items[4]);
                        aff_entry->ae_prog = vim_regcomp(buf,
                                            RE_MAGIC + RE_STRING + RE_STRICT);
                        if (aff_entry->ae_prog == NULL)
@@ -3906,7 +3908,9 @@ store_aff_word(
                                else
                                    p += STRLEN(ae->ae_chop);
                            }
-                           STRCAT(newword, p);
+                           // Note: this silently truncates the buffer, but this should
+                           // not happen in practice
+                           STRNCAT(newword, p, MAXWLEN - STRLEN(newword) - 1);
                        }
                        else
                        {
@@ -3922,7 +3926,9 @@ store_aff_word(
                                *p = NUL;
                            }
                            if (ae->ae_add != NULL)
-                               STRCAT(newword, ae->ae_add);
+                               // Note: this silently truncates the buffer, but this should
+                               // not happen in practice
+                               STRNCAT(newword, ae->ae_add, MAXWLEN - STRLEN(newword) - 1);
                        }
 
                        use_flags = flags;

diff --git a/src/testdir/test_spellfile.vim b/src/testdir/test_spellfile.vim
index b72974ed07e8d8b3c25ea3c13a80f59722dc685c..f46a25d99e1c43843223e8f3781d5f6c8c2ab1da 100644 (file)
--- a/src/testdir/test_spellfile.vim
+++ b/src/testdir/test_spellfile.vim
@@ -1165,5 +1165,32 @@ func Test_mkspell_empty_dic()
   call delete('XtestEmpty.spl')
 endfunc
 
+" This used to cause a buffer overflow
+func Test_mkspell_no_buffer_overflow()
+  CheckNotMSWindows
+
+  let aff_lines = ['SET ISO8859-1', 'SFX A Y 1',
+        \ 'SFX A 0 s ' .. repeat(nr2char(0xff), 491)]
+  call writefile(aff_lines, 'Xbof.aff', 'D')
+  call writefile(['1', 'word/A'], 'Xbof.dic', 'D')
+  " Must not crash; ignore any conversion/regex errors.
+  try
+    mkspell! Xbof.spl Xbof
+  catch
+  endtry
+  defer delete('Xbof.spl')
+
+  let long = repeat(nr2char(0xff), 200)
+  let aff2_lines = ['SET ISO8859-1', 'SFX A Y 1',
+        \ 'SFX A 0 ' .. long .. ' .']
+  call writefile(aff2_lines, 'Xbof2.aff', 'D')
+  call writefile(['1', long .. '/A'], 'Xbof2.dic', 'D')
+  try
+    mkspell! Xbof2.spl Xbof2
+  catch
+  endtry
+  defer delete('Xbof2.spl')
+endfunc
+
 
 " vim: shiftwidth=2 sts=2 expandtab

diff --git a/src/version.c b/src/version.c
index 535e85ef85ca6aec3d78894222458027656509b2..ccffba42d0c02ea621792b5ae0efee78f37ae239 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -734,6 +734,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    331,
 /**/
     330,
 /**/