patch 9.2.0513: [security]: memory safety issues in spellfile.c
Commit:
https://github.com/vim/vim/commit/25e4e46c584840806b45da20edf8219cf19801a2
Author: Christian Brabandt <[email protected]>
Date: Fri May 22 21:46:57 2026 +0000
patch 9.2.0513: [security]: memory safety issues in spellfile.c
Problem: [security]: memory safety issues in spellfile.c
(tacdm)
Solution: Add recursion limit to read_tree_node(), add length limit
check in tree_count_words(), use alloc_clear() in
spell_read_tree().
Github Security Advisory:
https://github.com/vim/vim/security/advisories/GHSA-3h95-3962-mmvf
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Signed-off-by: Christian Brabandt <[email protected]>
diff --git a/src/spell.h b/src/spell.h
index 1f473732a..4cef842fa 100644
--- a/src/spell.h
+++ b/src/spell.h
@@ -119,6 +119,7 @@ struct slang_S
// Info from the .sug file. Loaded on demand.
time_t sl_sugtime; // timestamp for .sug file
char_u *sl_sbyts; // soundfolded word bytes
+ long sl_sbyts_len; // length of sl_sbyts
idx_T *sl_sidxs; // soundfolded word indexes
buf_T *sl_sugbuf; // buffer with word number table
int sl_sugloaded; // TRUE when .sug file was loaded or
failed to
diff --git a/src/spellfile.c b/src/spellfile.c
index 5102dad5b..f65bb642b 100644
--- a/src/spellfile.c
+++ b/src/spellfile.c
@@ -313,7 +313,7 @@ static int set_sofo(slang_T *lp, char_u *from, char_u *to);
static void set_sal_first(slang_T *lp);
static int *mb_str2wide(char_u *s);
static int spell_read_tree(FILE *fd, char_u **bytsp, long *bytsp_len, idx_T
**idxsp, int prefixtree, int prefixcnt);
-static idx_T read_tree_node(FILE *fd, char_u *byts, idx_T *idxs, int maxidx,
idx_T startidx, int prefixtree, int maxprefcondnr);
+static idx_T read_tree_node(FILE *fd, char_u *byts, idx_T *idxs, int maxidx,
idx_T startidx, int prefixtree, int maxprefcondnr, int depth);
static void set_spell_charflags(char_u *flags, int cnt, char_u *upp);
static int set_spell_chartab(char_u *fol, char_u *low, char_u *upp);
static void set_map_str(slang_T *lp, char_u *map);
@@ -597,7 +597,7 @@ endOK:
* Returns the total number of words.
*/
static void
-tree_count_words(char_u *byts, idx_T *idxs)
+tree_count_words(char_u *byts, long byts_len, idx_T *idxs)
{
int depth;
idx_T arridx[MAXWLEN];
@@ -635,8 +635,8 @@ tree_count_words(char_u *byts, idx_T *idxs)
++wordcount[depth];
// Skip over any other NUL bytes (same word with different
- // flags).
- while (byts[n + 1] == 0)
+ // flags). But don't go over the end
+ while (n + 1 < byts_len && byts[n + 1] == 0)
{
++n;
++curi[depth];
@@ -732,8 +732,8 @@ suggest_load_files(void)
* <SUGWORDTREE>: <wordtree>
* Read the trie with the soundfolded words.
*/
- if (spell_read_tree(fd, &slang->sl_sbyts, NULL, &slang->sl_sidxs,
- FALSE, 0) != 0)
+ if (spell_read_tree(fd, &slang->sl_sbyts, &slang->sl_sbyts_len,
+ &slang->sl_sidxs, FALSE, 0) != 0)
{
someerror:
semsg(_(e_error_while_reading_sug_file_str),
@@ -782,8 +782,10 @@ someerror:
* Need to put word counts in the word tries, so that we can find
* a word by its number.
*/
- tree_count_words(slang->sl_fbyts, slang->sl_fidxs);
- tree_count_words(slang->sl_sbyts, slang->sl_sidxs);
+ tree_count_words(slang->sl_fbyts, slang->sl_fbyts_len,
+ slang->sl_fidxs);
+ tree_count_words(slang->sl_sbyts, slang->sl_sbyts_len,
+ slang->sl_sidxs);
nextone:
if (fd != NULL)
@@ -1603,8 +1605,11 @@ spell_read_tree(
if (len <= 0)
return 0;
- // Allocate the byte array.
- bp = alloc(len);
+ // Allocate the byte array. Zero-initialize so that any position the
+ // tree does not visit reads as 0; a stray BY_INDEX shared reference
+ // into such a slot then behaves as end-of-word in spellsuggest()
+ // instead of consuming an arbitrary heap byte as a siblingcount.
+ bp = alloc_clear(len);
if (bp == NULL)
return SP_OTHERERROR;
*bytsp = bp;
@@ -1618,9 +1623,11 @@ spell_read_tree(
*idxsp = ip;
// Recursively read the tree and store it in the array.
- idx = read_tree_node(fd, bp, ip, len, 0, prefixtree, prefixcnt);
+ idx = read_tree_node(fd, bp, ip, len, 0, prefixtree, prefixcnt, 0);
if (idx < 0)
return idx;
+ if (idx != len)
+ return SP_FORMERROR;
return 0;
}
@@ -1642,7 +1649,8 @@ read_tree_node(
int maxidx, // size of arrays
idx_T startidx, // current index in "byts" and "idxs"
int prefixtree, // TRUE for reading PREFIXTREE
- int maxprefcondnr) // maximum for <prefcondnr>
+ int maxprefcondnr, // maximum for <prefcondnr>
+ int depth) // recursiong level
{
int len;
int i;
@@ -1652,6 +1660,12 @@ read_tree_node(
int c2;
#define SHARED_MASK 0x8000000
+ // Bail out on a crafted .spl whose tree recurses beyond the maximum
+ // word length: each tree level corresponds to one byte of a word, so
+ // any well-formed file has depth <= MAXWLEN.
+ if (depth > MAXWLEN)
+ return SP_FORMERROR;
+
len = getc(fd); // <siblingcount>
if (len <= 0)
return SP_TRUNCERROR;
@@ -1737,7 +1751,7 @@ read_tree_node(
{
idxs[startidx + i] = idx;
idx = read_tree_node(fd, byts, idxs, maxidx, idx,
- prefixtree, maxprefcondnr);
+ prefixtree, maxprefcondnr, depth + 1);
if (idx < 0)
break;
}
@@ -5649,7 +5663,7 @@ sug_filltree(spellinfo_T *spin, slang_T *slang)
spin->si_blocks_cnt = 0;
// Skip over any other NUL bytes (same word with different
- // flags). But don't go over the end.
+ // flags). But don't go over the end
while (n + 1 < slang->sl_fbyts_len && byts[n + 1] == 0)
{
++n;
diff --git a/src/testdir/test_spell.vim b/src/testdir/test_spell.vim
index 58a2d5870..3b7f727d8 100644
--- a/src/testdir/test_spell.vim
+++ b/src/testdir/test_spell.vim
@@ -912,7 +912,10 @@ func Test_spellsuggest_too_deep()
" This was incrementing "depth" over MAXWLEN.
new
norm s000G00ý000000000000
- sil norm ..vzG................vvzG0 v z=
+ try
+ sil norm ..vzG................vvzG0 v z=
+ catch /E759:/
+ endtry
bwipe!
endfunc
diff --git a/src/testdir/test_spellfile.vim b/src/testdir/test_spellfile.vim
index 8f3ef4907..cf75eb4ef 100644
--- a/src/testdir/test_spellfile.vim
+++ b/src/testdir/test_spellfile.vim
@@ -372,6 +372,24 @@ func Test_spellfile_format_error()
" LWORDTREE: incorrect sibling node count
call Spellfile_Test(0zFF00000001040000000000000000, 'E759:')
+ " LWORDTREE: declared nodecount larger than the tree actually fills.
+ " Root has two siblings: 'x' (recurses into an end-of-word at idx 3..4)
+ " and BY_INDEX targeting position 9. Tree fills positions 0..4, leaving
+ " 5..9 unwritten — byts[9] would be uninitialized without the fix.
+ call Spellfile_Test(0zFF0000000A02780100000979010000000000000000000000,
'E759:')
+
+ " LWORDTREE: recursion depth past MAXWLEN. A linear chain of 254
+ " (siblingcount=1, byte='a') frames drives read_tree_node to depth
+ " MAXWLEN where the new guard rejects. The trailing (01 00) gives the
+ " chain a clean end-of-word so an *unguarded* parser would accept the
+ " file silently — that's what makes this a meaningful regression test
+ " for the depth check specifically (a deeper chain would also crash
+ " unguarded builds via stack overflow, which we don't want in CI).
+ let v = eval('0zFF00000200' .. repeat('0161', 255)
+ \ .. '0100' .. repeat('00', 8))
+
+ call Spellfile_Test(v, 'E759:')
+
" KWORDTREE: missing tree node
call Spellfile_Test(0zFF0000000000000004, 'E758:')
diff --git a/src/version.c b/src/version.c
index ce8c620a9..f0e32a02a 100644
--- a/src/version.c
+++ b/src/version.c
@@ -729,6 +729,8 @@ static char *(features[]) =
static int included_patches[] =
{ /* Add new patch number below this line */
+/**/
+ 513,
/**/
512,
/**/
--
--
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/E1wQY92-001Eav-9h%40256bit.org.
